Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

fbdev: smscufx: fix error handling code in ufx_usb_probe

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>

authored by

Dongliang Mu and committed by
Helge Deller
b76449ee 5886b130

+31 -15
+31 -15
drivers/video/fbdev/smscufx.c
··· 1622 1622 struct usb_device *usbdev; 1623 1623 struct ufx_data *dev; 1624 1624 struct fb_info *info; 1625 - int retval; 1625 + int retval = -ENOMEM; 1626 1626 u32 id_rev, fpga_rev; 1627 1627 1628 1628 /* usb initialization */ ··· 1654 1654 1655 1655 if (!ufx_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) { 1656 1656 dev_err(dev->gdev, "ufx_alloc_urb_list failed\n"); 1657 - goto e_nomem; 1657 + goto put_ref; 1658 1658 } 1659 1659 1660 1660 /* We don't register a new USB class. Our client interface is fbdev */ 1661 1661 1662 1662 /* allocates framebuffer driver structure, not framebuffer memory */ 1663 1663 info = framebuffer_alloc(0, &usbdev->dev); 1664 - if (!info) 1665 - goto e_nomem; 1664 + if (!info) { 1665 + dev_err(dev->gdev, "framebuffer_alloc failed\n"); 1666 + goto free_urb_list; 1667 + } 1666 1668 1667 1669 dev->info = info; 1668 1670 info->par = dev; ··· 1707 1705 check_warn_goto_error(retval, "unable to find common mode for display and adapter"); 1708 1706 1709 1707 retval = ufx_reg_set_bits(dev, 0x4000, 0x00000001); 1710 - check_warn_goto_error(retval, "error %d enabling graphics engine", retval); 1708 + if (retval < 0) { 1709 + dev_err(dev->gdev, "error %d enabling graphics engine", retval); 1710 + goto setup_modes; 1711 + } 1711 1712 1712 1713 /* ready to begin using device */ 1713 1714 atomic_set(&dev->usb_active, 1); 1714 1715 1715 1716 dev_dbg(dev->gdev, "checking var"); 1716 1717 retval = ufx_ops_check_var(&info->var, info); 1717 - check_warn_goto_error(retval, "error %d ufx_ops_check_var", retval); 1718 + if (retval < 0) { 1719 + dev_err(dev->gdev, "error %d ufx_ops_check_var", retval); 1720 + goto reset_active; 1721 + } 1718 1722 1719 1723 dev_dbg(dev->gdev, "setting par"); 1720 1724 retval = ufx_ops_set_par(info); 1721 - check_warn_goto_error(retval, "error %d ufx_ops_set_par", retval); 1725 + if (retval < 0) { 1726 + dev_err(dev->gdev, "error %d ufx_ops_set_par", retval); 1727 + goto reset_active; 1728 + } 1722 1729 1723 1730 dev_dbg(dev->gdev, "registering framebuffer"); 1724 1731 retval = register_framebuffer(info); 1725 - check_warn_goto_error(retval, "error %d register_framebuffer", retval); 1732 + if (retval < 0) { 1733 + dev_err(dev->gdev, "error %d register_framebuffer", retval); 1734 + goto reset_active; 1735 + } 1726 1736 1727 1737 dev_info(dev->gdev, "SMSC UDX USB device /dev/fb%d attached. %dx%d resolution." 1728 1738 " Using %dK framebuffer memory\n", info->node, ··· 1742 1728 1743 1729 return 0; 1744 1730 1745 - error: 1746 - fb_dealloc_cmap(&info->cmap); 1747 - destroy_modedb: 1731 + reset_active: 1732 + atomic_set(&dev->usb_active, 0); 1733 + setup_modes: 1748 1734 fb_destroy_modedb(info->monspecs.modedb); 1749 1735 vfree(info->screen_base); 1750 1736 fb_destroy_modelist(&info->modelist); 1737 + error: 1738 + fb_dealloc_cmap(&info->cmap); 1739 + destroy_modedb: 1751 1740 framebuffer_release(info); 1741 + free_urb_list: 1742 + if (dev->urbs.count > 0) 1743 + ufx_free_urb_list(dev); 1752 1744 put_ref: 1753 1745 kref_put(&dev->kref, ufx_free); /* ref for framebuffer */ 1754 1746 kref_put(&dev->kref, ufx_free); /* last ref from kref_init */ 1755 1747 return retval; 1756 - 1757 - e_nomem: 1758 - retval = -ENOMEM; 1759 - goto put_ref; 1760 1748 } 1761 1749 1762 1750 static void ufx_usb_disconnect(struct usb_interface *interface)