linux-kernel - [PATCH] [media] em28xx-input: fix refcount bug in em28xx_usb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210706121505.641603-1-mudongliangabcd@gmail.com>
Date:   Tue,  6 Jul 2021 20:15:05 +0800
From:   Dongliang Mu <mudongliangabcd@...il.com>
To:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        Frank Schäfer <fschaefer.oss@...glemail.com>
Cc:     Dongliang Mu <mudongliangabcd@...il.com>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH] [media] em28xx-input: fix refcount bug in em28xx_usb_disconnect

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@...il.com>
Fixes: 47677e51e2a4 ("[media] em28xx: Only deallocate struct em28xx after finishing all extensions")
Signed-off-by: Dongliang Mu <mudongliangabcd@...il.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