| Message ID | YgcSbUwiALbmoTvL@rowland.harvard.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: elo: Fix refcount leak in elo_probe() | expand |
On Sat, Feb 12, 2022 at 9:50 AM Alan Stern <stern@rowland.harvard.edu> wrote: > > Syzbot identified a refcount leak in the hid-elo driver: > > BUG: memory leak > unreferenced object 0xffff88810d49e800 (size 2048): > comm "kworker/1:1", pid 25, jiffies 4294954629 (age 16.460s) > hex dump (first 32 bytes): > ff ff ff ff 31 00 00 00 00 00 00 00 00 00 00 00 ....1........... > 00 00 00 00 00 00 00 00 00 00 00 00 03 00 00 00 ................ > backtrace: > [<ffffffff82c87a62>] kmalloc include/linux/slab.h:581 [inline] > [<ffffffff82c87a62>] kzalloc include/linux/slab.h:715 [inline] > [<ffffffff82c87a62>] usb_alloc_dev+0x32/0x450 drivers/usb/core/usb.c:582 > [<ffffffff82c91a47>] hub_port_connect drivers/usb/core/hub.c:5260 [inline] > [<ffffffff82c91a47>] hub_port_connect_change drivers/usb/core/hub.c:5502 [inline] > [<ffffffff82c91a47>] port_event drivers/usb/core/hub.c:5660 [inline] > [<ffffffff82c91a47>] hub_event+0x1097/0x21a0 drivers/usb/core/hub.c:5742 > [<ffffffff8126c3ef>] process_one_work+0x2bf/0x600 kernel/workqueue.c:2307 > [<ffffffff8126ccd9>] worker_thread+0x59/0x5b0 kernel/workqueue.c:2454 > [<ffffffff81276765>] kthread+0x125/0x160 kernel/kthread.c:377 > [<ffffffff810022ff>] ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295 > > Not shown in the bug report but present in the console log: > > [ 182.014764][ T3257] elo 0003:04E7:0030.0006: item fetching failed at offset 0/1 > [ 182.022255][ T3257] elo 0003:04E7:0030.0006: parse failed > [ 182.027904][ T3257] elo: probe of 0003:04E7:0030.0006 failed with error -22 > [ 182.214767][ T3257] usb 1-1: USB disconnect, device number 7 > [ 188.090199][ T3604] kmemleak: 3 new suspected memory leaks (see /sys/kernel/debug/kmemleak) > BUG: memory leak > > which points to hid-elo as the buggy driver. > > The leak is caused by elo_probe() failing to release the reference it > holds to the struct usb_device in its failure pathway. In the end the > driver doesn't need to take this reference at all, because the Hi Alan, My patch "[PATCH] hid: elo: fix memory leak in elo_probe" is merged several weeks ago. However, I fix this bug by modifying the error handling code in elo_probe. If you think the refcount is not necessary, maybe a new patch to remove the refcount is better. > elo_priv structure is always deallocated synchronously when the driver > unbinds from the interface. > > Therefore this patch fixes the reference leak by not taking the > reference in the first place. > > Reported-and-tested-by: syzbot+8caaaec4e7a55d75e243@syzkaller.appspotmail.com > Signed-off-by: Alan Stern <stern@rowland.harvard.edu> > CC: <stable@vger.kernel.org> > > --- > > > [as1971] > > > drivers/hid/hid-elo.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > Index: usb-devel/drivers/hid/hid-elo.c > =================================================================== > --- usb-devel.orig/drivers/hid/hid-elo.c > +++ usb-devel/drivers/hid/hid-elo.c > @@ -239,7 +239,7 @@ static int elo_probe(struct hid_device * > > INIT_DELAYED_WORK(&priv->work, elo_work); > udev = interface_to_usbdev(to_usb_interface(hdev->dev.parent)); > - priv->usbdev = usb_get_dev(udev); > + priv->usbdev = udev; > > hid_set_drvdata(hdev, priv); > > @@ -270,8 +270,6 @@ static void elo_remove(struct hid_device > { > struct elo_priv *priv = hid_get_drvdata(hdev); > > - usb_put_dev(priv->usbdev); > - > hid_hw_stop(hdev); > cancel_delayed_work_sync(&priv->work); > kfree(priv); > > -- > You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group. > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/YgcSbUwiALbmoTvL%40rowland.harvard.edu.
On Mon, Feb 14, 2022 at 03:34:42PM +0800, Dongliang Mu wrote: > On Sat, Feb 12, 2022 at 9:50 AM Alan Stern <stern@rowland.harvard.edu> wrote: > > > > Syzbot identified a refcount leak in the hid-elo driver: > > > > BUG: memory leak > > unreferenced object 0xffff88810d49e800 (size 2048): > > comm "kworker/1:1", pid 25, jiffies 4294954629 (age 16.460s) > > hex dump (first 32 bytes): > > ff ff ff ff 31 00 00 00 00 00 00 00 00 00 00 00 ....1........... > > 00 00 00 00 00 00 00 00 00 00 00 00 03 00 00 00 ................ > > backtrace: > > [<ffffffff82c87a62>] kmalloc include/linux/slab.h:581 [inline] > > [<ffffffff82c87a62>] kzalloc include/linux/slab.h:715 [inline] > > [<ffffffff82c87a62>] usb_alloc_dev+0x32/0x450 drivers/usb/core/usb.c:582 > > [<ffffffff82c91a47>] hub_port_connect drivers/usb/core/hub.c:5260 [inline] > > [<ffffffff82c91a47>] hub_port_connect_change drivers/usb/core/hub.c:5502 [inline] > > [<ffffffff82c91a47>] port_event drivers/usb/core/hub.c:5660 [inline] > > [<ffffffff82c91a47>] hub_event+0x1097/0x21a0 drivers/usb/core/hub.c:5742 > > [<ffffffff8126c3ef>] process_one_work+0x2bf/0x600 kernel/workqueue.c:2307 > > [<ffffffff8126ccd9>] worker_thread+0x59/0x5b0 kernel/workqueue.c:2454 > > [<ffffffff81276765>] kthread+0x125/0x160 kernel/kthread.c:377 > > [<ffffffff810022ff>] ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295 > > > > Not shown in the bug report but present in the console log: > > > > [ 182.014764][ T3257] elo 0003:04E7:0030.0006: item fetching failed at offset 0/1 > > [ 182.022255][ T3257] elo 0003:04E7:0030.0006: parse failed > > [ 182.027904][ T3257] elo: probe of 0003:04E7:0030.0006 failed with error -22 > > [ 182.214767][ T3257] usb 1-1: USB disconnect, device number 7 > > [ 188.090199][ T3604] kmemleak: 3 new suspected memory leaks (see /sys/kernel/debug/kmemleak) > > BUG: memory leak > > > > which points to hid-elo as the buggy driver. > > > > The leak is caused by elo_probe() failing to release the reference it > > holds to the struct usb_device in its failure pathway. In the end the > > driver doesn't need to take this reference at all, because the > > Hi Alan, > > My patch "[PATCH] hid: elo: fix memory leak in elo_probe" is merged > several weeks ago. Really? It still isn't in Linus's tree as of 5.17-rc4. I would expect a bug fix to go upstream as soon as possible. > However, I fix this bug by modifying the error handling code in > elo_probe. If you think the refcount is not necessary, maybe a new > patch to remove the refcount is better. The refcount was added less than a year ago by Salah Triki in commit fbf42729d0e9 ("HID: elo: update the reference count of the usb device structure"), but the commit message doesn't explain why it is necessary. There certainly isn't any obvious reason for it; the driver doesn't release any references after elo_remove() returns and we know that the usb_device structure won't be deallocated before the driver gets unbound. Alan Stern
On Fri, Feb 11, 2022 at 08:50:37PM -0500, Alan Stern wrote: > Syzbot identified a refcount leak in the hid-elo driver: > > BUG: memory leak > unreferenced object 0xffff88810d49e800 (size 2048): > comm "kworker/1:1", pid 25, jiffies 4294954629 (age 16.460s) > hex dump (first 32 bytes): > ff ff ff ff 31 00 00 00 00 00 00 00 00 00 00 00 ....1........... > 00 00 00 00 00 00 00 00 00 00 00 00 03 00 00 00 ................ > backtrace: > [<ffffffff82c87a62>] kmalloc include/linux/slab.h:581 [inline] > [<ffffffff82c87a62>] kzalloc include/linux/slab.h:715 [inline] > [<ffffffff82c87a62>] usb_alloc_dev+0x32/0x450 drivers/usb/core/usb.c:582 > [<ffffffff82c91a47>] hub_port_connect drivers/usb/core/hub.c:5260 [inline] > [<ffffffff82c91a47>] hub_port_connect_change drivers/usb/core/hub.c:5502 [inline] > [<ffffffff82c91a47>] port_event drivers/usb/core/hub.c:5660 [inline] > [<ffffffff82c91a47>] hub_event+0x1097/0x21a0 drivers/usb/core/hub.c:5742 > [<ffffffff8126c3ef>] process_one_work+0x2bf/0x600 kernel/workqueue.c:2307 > [<ffffffff8126ccd9>] worker_thread+0x59/0x5b0 kernel/workqueue.c:2454 > [<ffffffff81276765>] kthread+0x125/0x160 kernel/kthread.c:377 > [<ffffffff810022ff>] ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295 > > Not shown in the bug report but present in the console log: > > [ 182.014764][ T3257] elo 0003:04E7:0030.0006: item fetching failed at offset 0/1 > [ 182.022255][ T3257] elo 0003:04E7:0030.0006: parse failed > [ 182.027904][ T3257] elo: probe of 0003:04E7:0030.0006 failed with error -22 > [ 182.214767][ T3257] usb 1-1: USB disconnect, device number 7 > [ 188.090199][ T3604] kmemleak: 3 new suspected memory leaks (see /sys/kernel/debug/kmemleak) > BUG: memory leak > > which points to hid-elo as the buggy driver. > > The leak is caused by elo_probe() failing to release the reference it > holds to the struct usb_device in its failure pathway. In the end the > driver doesn't need to take this reference at all, because the > elo_priv structure is always deallocated synchronously when the driver > unbinds from the interface. > > Therefore this patch fixes the reference leak by not taking the > reference in the first place. > > Reported-and-tested-by: syzbot+8caaaec4e7a55d75e243@syzkaller.appspotmail.com > Signed-off-by: Alan Stern <stern@rowland.harvard.edu> > CC: <stable@vger.kernel.org> > Alan, this bug was fixed a different way in 817b8b9c5396 ("HID: elo: fix memory leak in elo_probe") so now the two fixes lead to a use after free. Your patch is more elegant so we should revert 817b8b9c5396. regards, dan carpenter
On Mon, Feb 14, 2022 at 09:41:32AM -0500, Alan Stern wrote: > On Mon, Feb 14, 2022 at 03:34:42PM +0800, Dongliang Mu wrote: > > On Sat, Feb 12, 2022 at 9:50 AM Alan Stern <stern@rowland.harvard.edu> wrote: > > > > > > Syzbot identified a refcount leak in the hid-elo driver: > > > > > > BUG: memory leak > > > unreferenced object 0xffff88810d49e800 (size 2048): > > > comm "kworker/1:1", pid 25, jiffies 4294954629 (age 16.460s) > > > hex dump (first 32 bytes): > > > ff ff ff ff 31 00 00 00 00 00 00 00 00 00 00 00 ....1........... > > > 00 00 00 00 00 00 00 00 00 00 00 00 03 00 00 00 ................ > > > backtrace: > > > [<ffffffff82c87a62>] kmalloc include/linux/slab.h:581 [inline] > > > [<ffffffff82c87a62>] kzalloc include/linux/slab.h:715 [inline] > > > [<ffffffff82c87a62>] usb_alloc_dev+0x32/0x450 drivers/usb/core/usb.c:582 > > > [<ffffffff82c91a47>] hub_port_connect drivers/usb/core/hub.c:5260 [inline] > > > [<ffffffff82c91a47>] hub_port_connect_change drivers/usb/core/hub.c:5502 [inline] > > > [<ffffffff82c91a47>] port_event drivers/usb/core/hub.c:5660 [inline] > > > [<ffffffff82c91a47>] hub_event+0x1097/0x21a0 drivers/usb/core/hub.c:5742 > > > [<ffffffff8126c3ef>] process_one_work+0x2bf/0x600 kernel/workqueue.c:2307 > > > [<ffffffff8126ccd9>] worker_thread+0x59/0x5b0 kernel/workqueue.c:2454 > > > [<ffffffff81276765>] kthread+0x125/0x160 kernel/kthread.c:377 > > > [<ffffffff810022ff>] ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295 > > > > > > Not shown in the bug report but present in the console log: > > > > > > [ 182.014764][ T3257] elo 0003:04E7:0030.0006: item fetching failed at offset 0/1 > > > [ 182.022255][ T3257] elo 0003:04E7:0030.0006: parse failed > > > [ 182.027904][ T3257] elo: probe of 0003:04E7:0030.0006 failed with error -22 > > > [ 182.214767][ T3257] usb 1-1: USB disconnect, device number 7 > > > [ 188.090199][ T3604] kmemleak: 3 new suspected memory leaks (see /sys/kernel/debug/kmemleak) > > > BUG: memory leak > > > > > > which points to hid-elo as the buggy driver. > > > > > > The leak is caused by elo_probe() failing to release the reference it > > > holds to the struct usb_device in its failure pathway. In the end the > > > driver doesn't need to take this reference at all, because the > > > > Hi Alan, > > > > My patch "[PATCH] hid: elo: fix memory leak in elo_probe" is merged > > several weeks ago. > > Really? It still isn't in Linus's tree as of 5.17-rc4. I would expect > a bug fix to go upstream as soon as possible. > > > However, I fix this bug by modifying the error handling code in > > elo_probe. If you think the refcount is not necessary, maybe a new > > patch to remove the refcount is better. > > The refcount was added less than a year ago by Salah Triki in commit > fbf42729d0e9 ("HID: elo: update the reference count of the usb device > structure"), but the commit message doesn't explain why it is > necessary. There certainly isn't any obvious reason for it; the driver > doesn't release any references after elo_remove() returns and we know > that the usb_device structure won't be deallocated before the driver > gets unbound. Salah sent a bunch of these. The reasoning was explained in this email. https://www.spinics.net/lists/kernel/msg4026672.html When he resent the patch, Greg said that taking the reference wasn't needed so the patch wasn't applied. (Also it had the same reference leak so that's a second reason it wasn't applied). https://lkml.org/lkml/2021/8/4/324 So someone should go through and revert all of Salah's bogus usb_get_dev() patches. regards, dan carpenter
On Thu, Feb 17, 2022 at 11:04:59AM +0300, Dan Carpenter wrote: > > Salah sent a bunch of these. The reasoning was explained in this email. > > https://www.spinics.net/lists/kernel/msg4026672.html > > When he resent the patch, Greg said that taking the reference wasn't > needed so the patch wasn't applied. (Also it had the same reference > leak so that's a second reason it wasn't applied). > > https://lkml.org/lkml/2021/8/4/324 > > So someone should go through and revert all of Salah's bogus usb_get_dev() > patches. Never mind, this is the only usb_get_dev() which was merged. regards, dan carpenter
On Thu, 17 Feb 2022, Dan Carpenter wrote: > > The refcount was added less than a year ago by Salah Triki in commit > > fbf42729d0e9 ("HID: elo: update the reference count of the usb device > > structure"), but the commit message doesn't explain why it is > > necessary. There certainly isn't any obvious reason for it; the driver > > doesn't release any references after elo_remove() returns and we know > > that the usb_device structure won't be deallocated before the driver > > gets unbound. > > Salah sent a bunch of these. The reasoning was explained in this email. > > https://www.spinics.net/lists/kernel/msg4026672.html > > When he resent the patch, Greg said that taking the reference wasn't > needed so the patch wasn't applied. (Also it had the same reference > leak so that's a second reason it wasn't applied). Sorry for late response, I've been away for a week. I have now queued revert of all this mess and will be sending it to Linus for 5.17 still. Thanks everybody for reporting.
On Thu, Feb 17, 2022 at 11:04:59AM +0300, Dan Carpenter wrote: > Salah sent a bunch of these. The reasoning was explained in this email. > > https://www.spinics.net/lists/kernel/msg4026672.html > > When he resent the patch, Greg said that taking the reference wasn't > needed so the patch wasn't applied. (Also it had the same reference > leak so that's a second reason it wasn't applied). Indeed, the kerneldoc for usb_get_intf() does say that each reference held by a driver must be refcounted. And there's nothing wrong with doing that, _provided_ you do it correctly. But if you know the extra refcount will never be needed (because the reference will be dropped before the usb_interface in question is removed), fiddling with the reference count is unnecessary. I guess whether or not to do it could be considered a matter of taste. On the other hand, it wouldn't hurt to update the kerneldoc for usb_get_intf() (and usb_get_dev() also). We could point out that if a driver does not access the usb_interface structure after its disconnect routine returns, incrementing the refcount isn't mandatory. Greg, any opinion on this? Alan Stern
On Thu, Feb 17, 2022 at 10:25:02AM -0500, Alan Stern wrote: > On Thu, Feb 17, 2022 at 11:04:59AM +0300, Dan Carpenter wrote: > > Salah sent a bunch of these. The reasoning was explained in this email. > > > > https://www.spinics.net/lists/kernel/msg4026672.html > > > > When he resent the patch, Greg said that taking the reference wasn't > > needed so the patch wasn't applied. (Also it had the same reference > > leak so that's a second reason it wasn't applied). > > Indeed, the kerneldoc for usb_get_intf() does say that each reference > held by a driver must be refcounted. And there's nothing wrong with > doing that, _provided_ you do it correctly. > > But if you know the extra refcount will never be needed (because the > reference will be dropped before the usb_interface in question is > removed), fiddling with the reference count is unnecessary. I guess > whether or not to do it could be considered a matter of taste. > > On the other hand, it wouldn't hurt to update the kerneldoc for > usb_get_intf() (and usb_get_dev() also). We could point out that if a > driver does not access the usb_interface structure after its disconnect > routine returns, incrementing the refcount isn't mandatory. That would be good to add to prevent this type of confusion in the future. thanks, greg k-h
On Fri, Feb 25, 2022 at 5:15 PM Greg KH <greg@kroah.com> wrote: > > On Thu, Feb 17, 2022 at 10:25:02AM -0500, Alan Stern wrote: > > On Thu, Feb 17, 2022 at 11:04:59AM +0300, Dan Carpenter wrote: > > > Salah sent a bunch of these. The reasoning was explained in this email. > > > > > > https://www.spinics.net/lists/kernel/msg4026672.html > > > > > > When he resent the patch, Greg said that taking the reference wasn't > > > needed so the patch wasn't applied. (Also it had the same reference > > > leak so that's a second reason it wasn't applied). > > > > Indeed, the kerneldoc for usb_get_intf() does say that each reference > > held by a driver must be refcounted. And there's nothing wrong with > > doing that, _provided_ you do it correctly. > > > > But if you know the extra refcount will never be needed (because the > > reference will be dropped before the usb_interface in question is > > removed), fiddling with the reference count is unnecessary. I guess > > whether or not to do it could be considered a matter of taste. > > > > On the other hand, it wouldn't hurt to update the kerneldoc for > > usb_get_intf() (and usb_get_dev() also). We could point out that if a > > driver does not access the usb_interface structure after its disconnect > > routine returns, incrementing the refcount isn't mandatory. > > That would be good to add to prevent this type of confusion in the > future. Hi Jiri Kosina, from my understanding, my previous patch and patch from Alan Stern can all fix the underlying issue. But as Dan said, the patch from Alan is more elegant. However, the revert commit from you said, my commit introduces memory leak, which confuses me. HID: elo: Revert USB reference counting Commit 817b8b9 ("HID: elo: fix memory leak in elo_probe") introduced memory leak on error path, but more importantly the whole USB reference counting is not needed at all in the first place ...... If it is really my patch that introduces the memory leak, please let me know. best regards, Dongliang Mu > > thanks, > > greg k-h
On Sat, Mar 12, 2022 at 05:39:05PM +0800, Dongliang Mu wrote: > On Fri, Feb 25, 2022 at 5:15 PM Greg KH <greg@kroah.com> wrote: > > > > On Thu, Feb 17, 2022 at 10:25:02AM -0500, Alan Stern wrote: > > > On Thu, Feb 17, 2022 at 11:04:59AM +0300, Dan Carpenter wrote: > > > > Salah sent a bunch of these. The reasoning was explained in this email. > > > > > > > > https://www.spinics.net/lists/kernel/msg4026672.html > > > > > > > > When he resent the patch, Greg said that taking the reference wasn't > > > > needed so the patch wasn't applied. (Also it had the same reference > > > > leak so that's a second reason it wasn't applied). > > > > > > Indeed, the kerneldoc for usb_get_intf() does say that each reference > > > held by a driver must be refcounted. And there's nothing wrong with > > > doing that, _provided_ you do it correctly. > > > > > > But if you know the extra refcount will never be needed (because the > > > reference will be dropped before the usb_interface in question is > > > removed), fiddling with the reference count is unnecessary. I guess > > > whether or not to do it could be considered a matter of taste. > > > > > > On the other hand, it wouldn't hurt to update the kerneldoc for > > > usb_get_intf() (and usb_get_dev() also). We could point out that if a > > > driver does not access the usb_interface structure after its disconnect > > > routine returns, incrementing the refcount isn't mandatory. > > > > That would be good to add to prevent this type of confusion in the > > future. > > Hi Jiri Kosina, > > from my understanding, my previous patch and patch from Alan Stern can > all fix the underlying issue. But as Dan said, the patch from Alan is > more elegant. > > However, the revert commit from you said, my commit introduces memory > leak, which confuses me. > > HID: elo: Revert USB reference counting > > Commit 817b8b9 ("HID: elo: fix memory leak in elo_probe") introduced > memory leak on error path, but more importantly the whole USB reference > counting is not needed at all in the first place ...... > > If it is really my patch that introduces the memory leak, please let me know. Jiri named the wrong commit in his Changelog. The memory leak was introduced by commit fbf42729d0e9 ("HID: elo: update the reference count of the usb device structure"). not by your commit 817b8b9c5396 ("HID: elo: fix memory leak in elo_probe"). Alan Stern
Index: usb-devel/drivers/hid/hid-elo.c =================================================================== --- usb-devel.orig/drivers/hid/hid-elo.c +++ usb-devel/drivers/hid/hid-elo.c @@ -239,7 +239,7 @@ static int elo_probe(struct hid_device * INIT_DELAYED_WORK(&priv->work, elo_work); udev = interface_to_usbdev(to_usb_interface(hdev->dev.parent)); - priv->usbdev = usb_get_dev(udev); + priv->usbdev = udev; hid_set_drvdata(hdev, priv); @@ -270,8 +270,6 @@ static void elo_remove(struct hid_device { struct elo_priv *priv = hid_get_drvdata(hdev); - usb_put_dev(priv->usbdev); - hid_hw_stop(hdev); cancel_delayed_work_sync(&priv->work); kfree(priv);
Syzbot identified a refcount leak in the hid-elo driver: BUG: memory leak unreferenced object 0xffff88810d49e800 (size 2048): comm "kworker/1:1", pid 25, jiffies 4294954629 (age 16.460s) hex dump (first 32 bytes): ff ff ff ff 31 00 00 00 00 00 00 00 00 00 00 00 ....1........... 00 00 00 00 00 00 00 00 00 00 00 00 03 00 00 00 ................ backtrace: [<ffffffff82c87a62>] kmalloc include/linux/slab.h:581 [inline] [<ffffffff82c87a62>] kzalloc include/linux/slab.h:715 [inline] [<ffffffff82c87a62>] usb_alloc_dev+0x32/0x450 drivers/usb/core/usb.c:582 [<ffffffff82c91a47>] hub_port_connect drivers/usb/core/hub.c:5260 [inline] [<ffffffff82c91a47>] hub_port_connect_change drivers/usb/core/hub.c:5502 [inline] [<ffffffff82c91a47>] port_event drivers/usb/core/hub.c:5660 [inline] [<ffffffff82c91a47>] hub_event+0x1097/0x21a0 drivers/usb/core/hub.c:5742 [<ffffffff8126c3ef>] process_one_work+0x2bf/0x600 kernel/workqueue.c:2307 [<ffffffff8126ccd9>] worker_thread+0x59/0x5b0 kernel/workqueue.c:2454 [<ffffffff81276765>] kthread+0x125/0x160 kernel/kthread.c:377 [<ffffffff810022ff>] ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295 Not shown in the bug report but present in the console log: [ 182.014764][ T3257] elo 0003:04E7:0030.0006: item fetching failed at offset 0/1 [ 182.022255][ T3257] elo 0003:04E7:0030.0006: parse failed [ 182.027904][ T3257] elo: probe of 0003:04E7:0030.0006 failed with error -22 [ 182.214767][ T3257] usb 1-1: USB disconnect, device number 7 [ 188.090199][ T3604] kmemleak: 3 new suspected memory leaks (see /sys/kernel/debug/kmemleak) BUG: memory leak which points to hid-elo as the buggy driver. The leak is caused by elo_probe() failing to release the reference it holds to the struct usb_device in its failure pathway. In the end the driver doesn't need to take this reference at all, because the elo_priv structure is always deallocated synchronously when the driver unbinds from the interface. Therefore this patch fixes the reference leak by not taking the reference in the first place. Reported-and-tested-by: syzbot+8caaaec4e7a55d75e243@syzkaller.appspotmail.com Signed-off-by: Alan Stern <stern@rowland.harvard.edu> CC: <stable@vger.kernel.org> --- [as1971] drivers/hid/hid-elo.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)