From: Sasha Levin <sashal@kernel.org> To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Sasha Levin <sashal@kernel.org>, steve.glendinning@shawell.net, linux-fbdev@vger.kernel.org, Helge Deller <deller@gmx.de>, dri-devel@lists.freedesktop.org, Dongliang Mu <dzm91@hust.edu.cn>, syzkaller <syzkaller@googlegroups.com> Subject: [PATCH AUTOSEL 6.0 18/18] fbdev: smscufx: fix error handling code in ufx_usb_probe Date: Fri, 23 Dec 2022 20:30:34 -0500 [thread overview] Message-ID: <20221224013034.392810-18-sashal@kernel.org> (raw) In-Reply-To: <20221224013034.392810-1-sashal@kernel.org> From: Dongliang Mu <dzm91@hust.edu.cn> [ Upstream commit b76449ee75e21acfe9fa4c653d8598f191ed7d68 ] The current error handling code in ufx_usb_probe have many unmatching issues, e.g., missing ufx_free_usb_list, destroy_modedb label should only include framebuffer_release, fb_dealloc_cmap only matches fb_alloc_cmap. My local syzkaller reports a memory leak bug: memory leak in ufx_usb_probe BUG: memory leak unreferenced object 0xffff88802f879580 (size 128): comm "kworker/0:7", pid 17416, jiffies 4295067474 (age 46.710s) hex dump (first 32 bytes): 80 21 7c 2e 80 88 ff ff 18 d0 d0 0c 80 88 ff ff .!|............. 00 d0 d0 0c 80 88 ff ff e0 ff ff ff 0f 00 00 00 ................ backtrace: [<ffffffff814c99a0>] kmalloc_trace+0x20/0x90 mm/slab_common.c:1045 [<ffffffff824d219c>] kmalloc include/linux/slab.h:553 [inline] [<ffffffff824d219c>] kzalloc include/linux/slab.h:689 [inline] [<ffffffff824d219c>] ufx_alloc_urb_list drivers/video/fbdev/smscufx.c:1873 [inline] [<ffffffff824d219c>] ufx_usb_probe+0x11c/0x15a0 drivers/video/fbdev/smscufx.c:1655 [<ffffffff82d17927>] usb_probe_interface+0x177/0x370 drivers/usb/core/driver.c:396 [<ffffffff82712f0d>] call_driver_probe drivers/base/dd.c:560 [inline] [<ffffffff82712f0d>] really_probe+0x12d/0x390 drivers/base/dd.c:639 [<ffffffff8271322f>] __driver_probe_device+0xbf/0x140 drivers/base/dd.c:778 [<ffffffff827132da>] driver_probe_device+0x2a/0x120 drivers/base/dd.c:808 [<ffffffff82713c27>] __device_attach_driver+0xf7/0x150 drivers/base/dd.c:936 [<ffffffff82710137>] bus_for_each_drv+0xb7/0x100 drivers/base/bus.c:427 [<ffffffff827136b5>] __device_attach+0x105/0x2d0 drivers/base/dd.c:1008 [<ffffffff82711d36>] bus_probe_device+0xc6/0xe0 drivers/base/bus.c:487 [<ffffffff8270e242>] device_add+0x642/0xdc0 drivers/base/core.c:3517 [<ffffffff82d14d5f>] usb_set_configuration+0x8ef/0xb80 drivers/usb/core/message.c:2170 [<ffffffff82d2576c>] usb_generic_driver_probe+0x8c/0xc0 drivers/usb/core/generic.c:238 [<ffffffff82d16ffc>] usb_probe_device+0x5c/0x140 drivers/usb/core/driver.c:293 [<ffffffff82712f0d>] call_driver_probe drivers/base/dd.c:560 [inline] [<ffffffff82712f0d>] really_probe+0x12d/0x390 drivers/base/dd.c:639 [<ffffffff8271322f>] __driver_probe_device+0xbf/0x140 drivers/base/dd.c:778 Fix this bug by rewriting the error handling code in ufx_usb_probe. Reported-by: syzkaller <syzkaller@googlegroups.com> Tested-by: Dongliang Mu <dzm91@hust.edu.cn> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn> Signed-off-by: Helge Deller <deller@gmx.de> Signed-off-by: Sasha Levin <sashal@kernel.org> --- drivers/video/fbdev/smscufx.c | 46 +++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c index 9343b7a4ac89..2ad6e98ce10d 100644 --- a/drivers/video/fbdev/smscufx.c +++ b/drivers/video/fbdev/smscufx.c @@ -1622,7 +1622,7 @@ static int ufx_usb_probe(struct usb_interface *interface, struct usb_device *usbdev; struct ufx_data *dev; struct fb_info *info; - int retval; + int retval = -ENOMEM; u32 id_rev, fpga_rev; /* usb initialization */ @@ -1654,15 +1654,17 @@ static int ufx_usb_probe(struct usb_interface *interface, if (!ufx_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) { dev_err(dev->gdev, "ufx_alloc_urb_list failed\n"); - goto e_nomem; + goto put_ref; } /* We don't register a new USB class. Our client interface is fbdev */ /* allocates framebuffer driver structure, not framebuffer memory */ info = framebuffer_alloc(0, &usbdev->dev); - if (!info) - goto e_nomem; + if (!info) { + dev_err(dev->gdev, "framebuffer_alloc failed\n"); + goto free_urb_list; + } dev->info = info; info->par = dev; @@ -1705,22 +1707,34 @@ static int ufx_usb_probe(struct usb_interface *interface, check_warn_goto_error(retval, "unable to find common mode for display and adapter"); retval = ufx_reg_set_bits(dev, 0x4000, 0x00000001); - check_warn_goto_error(retval, "error %d enabling graphics engine", retval); + if (retval < 0) { + dev_err(dev->gdev, "error %d enabling graphics engine", retval); + goto setup_modes; + } /* ready to begin using device */ atomic_set(&dev->usb_active, 1); dev_dbg(dev->gdev, "checking var"); retval = ufx_ops_check_var(&info->var, info); - check_warn_goto_error(retval, "error %d ufx_ops_check_var", retval); + if (retval < 0) { + dev_err(dev->gdev, "error %d ufx_ops_check_var", retval); + goto reset_active; + } dev_dbg(dev->gdev, "setting par"); retval = ufx_ops_set_par(info); - check_warn_goto_error(retval, "error %d ufx_ops_set_par", retval); + if (retval < 0) { + dev_err(dev->gdev, "error %d ufx_ops_set_par", retval); + goto reset_active; + } dev_dbg(dev->gdev, "registering framebuffer"); retval = register_framebuffer(info); - check_warn_goto_error(retval, "error %d register_framebuffer", retval); + if (retval < 0) { + dev_err(dev->gdev, "error %d register_framebuffer", retval); + goto reset_active; + } dev_info(dev->gdev, "SMSC UDX USB device /dev/fb%d attached. %dx%d resolution." " Using %dK framebuffer memory\n", info->node, @@ -1728,21 +1742,23 @@ static int ufx_usb_probe(struct usb_interface *interface, return 0; -error: - fb_dealloc_cmap(&info->cmap); -destroy_modedb: +reset_active: + atomic_set(&dev->usb_active, 0); +setup_modes: fb_destroy_modedb(info->monspecs.modedb); vfree(info->screen_base); fb_destroy_modelist(&info->modelist); +error: + fb_dealloc_cmap(&info->cmap); +destroy_modedb: framebuffer_release(info); +free_urb_list: + if (dev->urbs.count > 0) + ufx_free_urb_list(dev); put_ref: kref_put(&dev->kref, ufx_free); /* ref for framebuffer */ kref_put(&dev->kref, ufx_free); /* last ref from kref_init */ return retval; - -e_nomem: - retval = -ENOMEM; - goto put_ref; } static void ufx_usb_disconnect(struct usb_interface *interface) -- 2.35.1
WARNING: multiple messages have this Message-ID (diff)
From: Sasha Levin <sashal@kernel.org> To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Dongliang Mu <dzm91@hust.edu.cn>, syzkaller <syzkaller@googlegroups.com>, Helge Deller <deller@gmx.de>, Sasha Levin <sashal@kernel.org>, steve.glendinning@shawell.net, linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: [PATCH AUTOSEL 6.0 18/18] fbdev: smscufx: fix error handling code in ufx_usb_probe Date: Fri, 23 Dec 2022 20:30:34 -0500 [thread overview] Message-ID: <20221224013034.392810-18-sashal@kernel.org> (raw) In-Reply-To: <20221224013034.392810-1-sashal@kernel.org> From: Dongliang Mu <dzm91@hust.edu.cn> [ Upstream commit b76449ee75e21acfe9fa4c653d8598f191ed7d68 ] The current error handling code in ufx_usb_probe have many unmatching issues, e.g., missing ufx_free_usb_list, destroy_modedb label should only include framebuffer_release, fb_dealloc_cmap only matches fb_alloc_cmap. My local syzkaller reports a memory leak bug: memory leak in ufx_usb_probe BUG: memory leak unreferenced object 0xffff88802f879580 (size 128): comm "kworker/0:7", pid 17416, jiffies 4295067474 (age 46.710s) hex dump (first 32 bytes): 80 21 7c 2e 80 88 ff ff 18 d0 d0 0c 80 88 ff ff .!|............. 00 d0 d0 0c 80 88 ff ff e0 ff ff ff 0f 00 00 00 ................ backtrace: [<ffffffff814c99a0>] kmalloc_trace+0x20/0x90 mm/slab_common.c:1045 [<ffffffff824d219c>] kmalloc include/linux/slab.h:553 [inline] [<ffffffff824d219c>] kzalloc include/linux/slab.h:689 [inline] [<ffffffff824d219c>] ufx_alloc_urb_list drivers/video/fbdev/smscufx.c:1873 [inline] [<ffffffff824d219c>] ufx_usb_probe+0x11c/0x15a0 drivers/video/fbdev/smscufx.c:1655 [<ffffffff82d17927>] usb_probe_interface+0x177/0x370 drivers/usb/core/driver.c:396 [<ffffffff82712f0d>] call_driver_probe drivers/base/dd.c:560 [inline] [<ffffffff82712f0d>] really_probe+0x12d/0x390 drivers/base/dd.c:639 [<ffffffff8271322f>] __driver_probe_device+0xbf/0x140 drivers/base/dd.c:778 [<ffffffff827132da>] driver_probe_device+0x2a/0x120 drivers/base/dd.c:808 [<ffffffff82713c27>] __device_attach_driver+0xf7/0x150 drivers/base/dd.c:936 [<ffffffff82710137>] bus_for_each_drv+0xb7/0x100 drivers/base/bus.c:427 [<ffffffff827136b5>] __device_attach+0x105/0x2d0 drivers/base/dd.c:1008 [<ffffffff82711d36>] bus_probe_device+0xc6/0xe0 drivers/base/bus.c:487 [<ffffffff8270e242>] device_add+0x642/0xdc0 drivers/base/core.c:3517 [<ffffffff82d14d5f>] usb_set_configuration+0x8ef/0xb80 drivers/usb/core/message.c:2170 [<ffffffff82d2576c>] usb_generic_driver_probe+0x8c/0xc0 drivers/usb/core/generic.c:238 [<ffffffff82d16ffc>] usb_probe_device+0x5c/0x140 drivers/usb/core/driver.c:293 [<ffffffff82712f0d>] call_driver_probe drivers/base/dd.c:560 [inline] [<ffffffff82712f0d>] really_probe+0x12d/0x390 drivers/base/dd.c:639 [<ffffffff8271322f>] __driver_probe_device+0xbf/0x140 drivers/base/dd.c:778 Fix this bug by rewriting the error handling code in ufx_usb_probe. Reported-by: syzkaller <syzkaller@googlegroups.com> Tested-by: Dongliang Mu <dzm91@hust.edu.cn> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn> Signed-off-by: Helge Deller <deller@gmx.de> Signed-off-by: Sasha Levin <sashal@kernel.org> --- drivers/video/fbdev/smscufx.c | 46 +++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c index 9343b7a4ac89..2ad6e98ce10d 100644 --- a/drivers/video/fbdev/smscufx.c +++ b/drivers/video/fbdev/smscufx.c @@ -1622,7 +1622,7 @@ static int ufx_usb_probe(struct usb_interface *interface, struct usb_device *usbdev; struct ufx_data *dev; struct fb_info *info; - int retval; + int retval = -ENOMEM; u32 id_rev, fpga_rev; /* usb initialization */ @@ -1654,15 +1654,17 @@ static int ufx_usb_probe(struct usb_interface *interface, if (!ufx_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) { dev_err(dev->gdev, "ufx_alloc_urb_list failed\n"); - goto e_nomem; + goto put_ref; } /* We don't register a new USB class. Our client interface is fbdev */ /* allocates framebuffer driver structure, not framebuffer memory */ info = framebuffer_alloc(0, &usbdev->dev); - if (!info) - goto e_nomem; + if (!info) { + dev_err(dev->gdev, "framebuffer_alloc failed\n"); + goto free_urb_list; + } dev->info = info; info->par = dev; @@ -1705,22 +1707,34 @@ static int ufx_usb_probe(struct usb_interface *interface, check_warn_goto_error(retval, "unable to find common mode for display and adapter"); retval = ufx_reg_set_bits(dev, 0x4000, 0x00000001); - check_warn_goto_error(retval, "error %d enabling graphics engine", retval); + if (retval < 0) { + dev_err(dev->gdev, "error %d enabling graphics engine", retval); + goto setup_modes; + } /* ready to begin using device */ atomic_set(&dev->usb_active, 1); dev_dbg(dev->gdev, "checking var"); retval = ufx_ops_check_var(&info->var, info); - check_warn_goto_error(retval, "error %d ufx_ops_check_var", retval); + if (retval < 0) { + dev_err(dev->gdev, "error %d ufx_ops_check_var", retval); + goto reset_active; + } dev_dbg(dev->gdev, "setting par"); retval = ufx_ops_set_par(info); - check_warn_goto_error(retval, "error %d ufx_ops_set_par", retval); + if (retval < 0) { + dev_err(dev->gdev, "error %d ufx_ops_set_par", retval); + goto reset_active; + } dev_dbg(dev->gdev, "registering framebuffer"); retval = register_framebuffer(info); - check_warn_goto_error(retval, "error %d register_framebuffer", retval); + if (retval < 0) { + dev_err(dev->gdev, "error %d register_framebuffer", retval); + goto reset_active; + } dev_info(dev->gdev, "SMSC UDX USB device /dev/fb%d attached. %dx%d resolution." " Using %dK framebuffer memory\n", info->node, @@ -1728,21 +1742,23 @@ static int ufx_usb_probe(struct usb_interface *interface, return 0; -error: - fb_dealloc_cmap(&info->cmap); -destroy_modedb: +reset_active: + atomic_set(&dev->usb_active, 0); +setup_modes: fb_destroy_modedb(info->monspecs.modedb); vfree(info->screen_base); fb_destroy_modelist(&info->modelist); +error: + fb_dealloc_cmap(&info->cmap); +destroy_modedb: framebuffer_release(info); +free_urb_list: + if (dev->urbs.count > 0) + ufx_free_urb_list(dev); put_ref: kref_put(&dev->kref, ufx_free); /* ref for framebuffer */ kref_put(&dev->kref, ufx_free); /* last ref from kref_init */ return retval; - -e_nomem: - retval = -ENOMEM; - goto put_ref; } static void ufx_usb_disconnect(struct usb_interface *interface) -- 2.35.1
next prev parent reply other threads:[~2022-12-24 1:31 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-12-24 1:30 [PATCH AUTOSEL 6.0 01/18] kset: fix memory leak when kset_register() returns error Sasha Levin 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 02/18] USB: core: Change configuration warnings to notices Sasha Levin 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 03/18] usb: core: stop USB enumeration if too many retries Sasha Levin 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 04/18] usb: gadget: aspeed: fix buffer overflow Sasha Levin 2022-12-24 1:30 ` Sasha Levin 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 05/18] usb: gadget: u_ether: Do not make UDC parent of the net device Sasha Levin 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 06/18] usb: gadget: f_ecm: Always set current gadget in ecm_bind() Sasha Levin 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 07/18] chardev: Fix potential memory leak when cdev_add() failed Sasha Levin 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 08/18] usb/usbip: Fix v_recv_cmd_submit() to use PIPE_BULK define Sasha Levin 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 09/18] char: xillybus: Prevent use-after-free due to race condition Sasha Levin 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 10/18] habanalabs: zero ts registration buff when allocated Sasha Levin 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 11/18] char: xillybus: Fix trivial bug with mutex Sasha Levin 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 12/18] iio: filter: admv8818: close potential out-of-bounds read in __admv8818_read_[h|l]pf_freq() Sasha Levin 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 13/18] xhci: disable U3 suspended ports in S4 hibernate poweroff_late stage Sasha Levin 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 14/18] ACPICA: Fix operand resolution Sasha Levin 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 15/18] ksmbd: Fix resource leak in smb2_lock() Sasha Levin 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 16/18] writeback: Add asserts for adding freed inode to lists Sasha Levin 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 17/18] exfat: fix overflow in sector and cluster conversion Sasha Levin 2022-12-24 1:30 ` Sasha Levin [this message] 2022-12-24 1:30 ` [PATCH AUTOSEL 6.0 18/18] fbdev: smscufx: fix error handling code in ufx_usb_probe Sasha Levin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20221224013034.392810-18-sashal@kernel.org \ --to=sashal@kernel.org \ --cc=deller@gmx.de \ --cc=dri-devel@lists.freedesktop.org \ --cc=dzm91@hust.edu.cn \ --cc=linux-fbdev@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=stable@vger.kernel.org \ --cc=steve.glendinning@shawell.net \ --cc=syzkaller@googlegroups.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.