* [PATCH] [media] em28xx-input: fix refcount bug in em28xx_usb_disconnect
@ 2021-07-06 12:15 Dongliang Mu
2021-07-06 12:34 ` Dongliang Mu
0 siblings, 1 reply; 2+ messages in thread
From: Dongliang Mu @ 2021-07-06 12:15 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Frank Schäfer
Cc: Dongliang Mu, linux-media, linux-kernel
If em28xx_ir_init fails, it would decrease the refcount of dev. However,
in the em28xx_ir_fini, when ir is NULL, it goes to ref_put and decrease
the refcount of dev. This will lead to a refcount bug.
Fix this bug by returning 0 when ir is NULL in the em28xx_ir_fini. To
simplify the refcount issue, move the kref_get after the input
extension is successfully initialized and remove the kref_put in the
error handling code.
refcount_t: underflow; use-after-free.
WARNING: CPU: 0 PID: 7 at lib/refcount.c:28 refcount_warn_saturate+0x18e/0x1a0 lib/refcount.c:28
Modules linked in:
CPU: 0 PID: 7 Comm: kworker/0:1 Not tainted 5.13.0 #3
Workqueue: usb_hub_wq hub_event
RIP: 0010:refcount_warn_saturate+0x18e/0x1a0 lib/refcount.c:28
Call Trace:
kref_put.constprop.0+0x60/0x85 include/linux/kref.h:69
em28xx_usb_disconnect.cold+0xd7/0xdc drivers/media/usb/em28xx/em28xx-cards.c:4150
usb_unbind_interface+0xbf/0x3a0 drivers/usb/core/driver.c:458
__device_release_driver drivers/base/dd.c:1201 [inline]
device_release_driver_internal+0x22a/0x230 drivers/base/dd.c:1232
bus_remove_device+0x108/0x160 drivers/base/bus.c:529
device_del+0x1fe/0x510 drivers/base/core.c:3540
usb_disable_device+0xd1/0x1d0 drivers/usb/core/message.c:1419
usb_disconnect+0x109/0x330 drivers/usb/core/hub.c:2221
hub_port_connect drivers/usb/core/hub.c:5151 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5440 [inline]
port_event drivers/usb/core/hub.c:5586 [inline]
hub_event+0xf81/0x1d40 drivers/usb/core/hub.c:5668
process_one_work+0x2c9/0x610 kernel/workqueue.c:2276
process_scheduled_works kernel/workqueue.c:2338 [inline]
worker_thread+0x333/0x5b0 kernel/workqueue.c:2424
kthread+0x188/0x1d0 kernel/kthread.c:319
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295
Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
Fixes: 47677e51e2a4 ("[media] em28xx: Only deallocate struct em28xx after finishing all extensions")
Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
---
drivers/media/usb/em28xx/em28xx-input.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-input.c b/drivers/media/usb/em28xx/em28xx-input.c
index 59529cbf9cd0..f741be9583a8 100644
--- a/drivers/media/usb/em28xx/em28xx-input.c
+++ b/drivers/media/usb/em28xx/em28xx-input.c
@@ -708,7 +708,6 @@ static int em28xx_ir_init(struct em28xx *dev)
return 0;
}
- kref_get(&dev->ref);
INIT_DELAYED_WORK(&dev->buttons_query_work, em28xx_query_buttons);
if (dev->board.buttons)
@@ -833,6 +832,9 @@ static int em28xx_ir_init(struct em28xx *dev)
dev_info(&dev->intf->dev, "Input extension successfully initialized\n");
+ /* Only increase refcount when this function is executed successfully */
+ kref_get(&dev->ref);
+
return 0;
error:
@@ -842,7 +844,6 @@ static int em28xx_ir_init(struct em28xx *dev)
kfree(ir);
ref_put:
em28xx_shutdown_buttons(dev);
- kref_put(&dev->ref, em28xx_free_device);
return err;
}
@@ -861,7 +862,7 @@ static int em28xx_ir_fini(struct em28xx *dev)
/* skip detach on non attached boards */
if (!ir)
- goto ref_put;
+ return 0;
rc_unregister_device(ir->rc);
@@ -871,7 +872,6 @@ static int em28xx_ir_fini(struct em28xx *dev)
kfree(ir);
dev->ir = NULL;
-ref_put:
kref_put(&dev->ref, em28xx_free_device);
return 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] [media] em28xx-input: fix refcount bug in em28xx_usb_disconnect
2021-07-06 12:15 [PATCH] [media] em28xx-input: fix refcount bug in em28xx_usb_disconnect Dongliang Mu
@ 2021-07-06 12:34 ` Dongliang Mu
0 siblings, 0 replies; 2+ messages in thread
From: Dongliang Mu @ 2021-07-06 12:34 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Frank Schäfer; +Cc: linux-media, linux-kernel
On Tue, Jul 6, 2021 at 8:15 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>
> If em28xx_ir_init fails, it would decrease the refcount of dev. However,
> in the em28xx_ir_fini, when ir is NULL, it goes to ref_put and decrease
> the refcount of dev. This will lead to a refcount bug.
>
> Fix this bug by returning 0 when ir is NULL in the em28xx_ir_fini. To
> simplify the refcount issue, move the kref_get after the input
> extension is successfully initialized and remove the kref_put in the
> error handling code.
>
> refcount_t: underflow; use-after-free.
> WARNING: CPU: 0 PID: 7 at lib/refcount.c:28 refcount_warn_saturate+0x18e/0x1a0 lib/refcount.c:28
> Modules linked in:
> CPU: 0 PID: 7 Comm: kworker/0:1 Not tainted 5.13.0 #3
> Workqueue: usb_hub_wq hub_event
> RIP: 0010:refcount_warn_saturate+0x18e/0x1a0 lib/refcount.c:28
> Call Trace:
> kref_put.constprop.0+0x60/0x85 include/linux/kref.h:69
> em28xx_usb_disconnect.cold+0xd7/0xdc drivers/media/usb/em28xx/em28xx-cards.c:4150
> usb_unbind_interface+0xbf/0x3a0 drivers/usb/core/driver.c:458
> __device_release_driver drivers/base/dd.c:1201 [inline]
> device_release_driver_internal+0x22a/0x230 drivers/base/dd.c:1232
> bus_remove_device+0x108/0x160 drivers/base/bus.c:529
> device_del+0x1fe/0x510 drivers/base/core.c:3540
> usb_disable_device+0xd1/0x1d0 drivers/usb/core/message.c:1419
> usb_disconnect+0x109/0x330 drivers/usb/core/hub.c:2221
> hub_port_connect drivers/usb/core/hub.c:5151 [inline]
> hub_port_connect_change drivers/usb/core/hub.c:5440 [inline]
> port_event drivers/usb/core/hub.c:5586 [inline]
> hub_event+0xf81/0x1d40 drivers/usb/core/hub.c:5668
> process_one_work+0x2c9/0x610 kernel/workqueue.c:2276
> process_scheduled_works kernel/workqueue.c:2338 [inline]
> worker_thread+0x333/0x5b0 kernel/workqueue.c:2424
> kthread+0x188/0x1d0 kernel/kthread.c:319
> ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295
>
> Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
> Fixes: 47677e51e2a4 ("[media] em28xx: Only deallocate struct em28xx after finishing all extensions")
After some more code review, I think the "Fixes" tag should be the fix:
ac5688637144 ("media: em28xx: Fix possible memory leak of em28xx struct")
static struct em28xx_ops rc_ops = {
.id = EM28XX_RC,
.name = "Em28xx Input Extension",
.init = em28xx_ir_init,
.fini = em28xx_ir_fini,
}
As init and fini are executed in pair, there is no need to decrease
the refcount in the error handling code.
In this patch, I remove the kref_put to prevent the refcount bug.
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
> drivers/media/usb/em28xx/em28xx-input.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-input.c b/drivers/media/usb/em28xx/em28xx-input.c
> index 59529cbf9cd0..f741be9583a8 100644
> --- a/drivers/media/usb/em28xx/em28xx-input.c
> +++ b/drivers/media/usb/em28xx/em28xx-input.c
> @@ -708,7 +708,6 @@ static int em28xx_ir_init(struct em28xx *dev)
> return 0;
> }
>
> - kref_get(&dev->ref);
> INIT_DELAYED_WORK(&dev->buttons_query_work, em28xx_query_buttons);
>
> if (dev->board.buttons)
> @@ -833,6 +832,9 @@ static int em28xx_ir_init(struct em28xx *dev)
>
> dev_info(&dev->intf->dev, "Input extension successfully initialized\n");
>
> + /* Only increase refcount when this function is executed successfully */
> + kref_get(&dev->ref);
> +
> return 0;
>
> error:
> @@ -842,7 +844,6 @@ static int em28xx_ir_init(struct em28xx *dev)
> kfree(ir);
> ref_put:
> em28xx_shutdown_buttons(dev);
> - kref_put(&dev->ref, em28xx_free_device);
> return err;
> }
>
> @@ -861,7 +862,7 @@ static int em28xx_ir_fini(struct em28xx *dev)
>
> /* skip detach on non attached boards */
> if (!ir)
> - goto ref_put;
> + return 0;
>
> rc_unregister_device(ir->rc);
>
> @@ -871,7 +872,6 @@ static int em28xx_ir_fini(struct em28xx *dev)
> kfree(ir);
> dev->ir = NULL;
>
> -ref_put:
> kref_put(&dev->ref, em28xx_free_device);
>
> return 0;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-07-06 12:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-06 12:15 [PATCH] [media] em28xx-input: fix refcount bug in em28xx_usb_disconnect Dongliang Mu
2021-07-06 12:34 ` Dongliang Mu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).