linux-kernel - [PATCH v3] USB: gadget: Fix use-after-free during usb config switch

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: <d3393e2f-a79c-d2c1-f752-71bd21a7ddbf@huawei.com>
Date:   Tue, 15 Nov 2022 20:52:02 +0800
From:   jiantao zhang <water.zhangjiantao@...wei.com>
To:     <gregkh@...uxfoundation.org>, <stern@...land.harvard.edu>,
        <jakobkoschel@...il.com>, <geert+renesas@...der.be>,
        张建涛 <water.zhangjiantao@...wei.com>,
        <colin.i.king@...il.com>,
        薛涛 <xuetao09@...wei.com>,
        <linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>
CC:     Caiyadong <caiyadong@...wei.com>,
        xuhaiyang <xuhaiyang5@...ilicon.com>
Subject: [PATCH v3] USB: gadget: Fix use-after-free during usb config switch

In the process of switching USB config from rndis to other config,
if the hardware does not support the ->pullup callback, or the
hardware encounters a low probability fault, both of them may cause
the ->pullup callback to fail, which will then cause a system panic
(use after free).

The gadget drivers sometimes need to be unloaded regardless of the
hardware's behavior.

Analysis as follows:
=======================================================================
(1) write /config/usb_gadget/g1/UDC "none"

gether_disconnect+0x2c/0x1f8
rndis_disable+0x4c/0x74
composite_disconnect+0x74/0xb0
configfs_composite_disconnect+0x60/0x7c
usb_gadget_disconnect+0x70/0x124
usb_gadget_unregister_driver+0xc8/0x1d8
gadget_dev_desc_UDC_store+0xec/0x1e4

(2) rm /config/usb_gadget/g1/configs/b.1/f1

rndis_deregister+0x28/0x54
rndis_free+0x44/0x7c
usb_put_function+0x14/0x1c
config_usb_cfg_unlink+0xc4/0xe0
configfs_unlink+0x124/0x1c8
vfs_unlink+0x114/0x1dc

(3) rmdir /config/usb_gadget/g1/functions/rndis.gs4

panic+0x1fc/0x3d0
do_page_fault+0xa8/0x46c
do_mem_abort+0x3c/0xac
el1_sync_handler+0x40/0x78
0xffffff801138f880
rndis_close+0x28/0x34
eth_stop+0x74/0x110
dev_close_many+0x48/0x194
rollback_registered_many+0x118/0x814
unregister_netdev+0x20/0x30
gether_cleanup+0x1c/0x38
rndis_attr_release+0xc/0x14
kref_put+0x74/0xb8
configfs_rmdir+0x314/0x374

If gadget->ops->pullup() return an error, function rndis_close() will be
called, then it will causes a use-after-free problem.
=======================================================================

Fixes: 0a55187a1ec8 ("USB: gadget core: Issue ->disconnect() callback 
from usb_gadget_disconnect()")
Signed-off-by: Jiantao Zhang <water.zhangjiantao@...wei.com>
Signed-off-by: TaoXue <xuetao09@...wei.com>
---
V2 -> V3: Solved the format issues of Fixes and backtraces.
V1 -> V2: V1 will affect the original function, V2 just move the callback
after "if" statement, so that the original function will not be affected.
And fixed formatting issues.

  drivers/usb/gadget/udc/core.c | 12 ++++++------
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index c63c0c2cf649..bf9878e1a72a 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -734,13 +734,13 @@ int usb_gadget_disconnect(struct usb_gadget *gadget)
  	}
   	ret = gadget->ops->pullup(gadget, 0);
-	if (!ret) {
+	if (!ret)
  		gadget->connected = 0;
-		mutex_lock(&udc_lock);
-		if (gadget->udc->driver)
-			gadget->udc->driver->disconnect(gadget);
-		mutex_unlock(&udc_lock);
-	}
+
+	mutex_lock(&udc_lock);
+	if (gadget->udc->driver)
+		gadget->udc->driver->disconnect(gadget);
+	mutex_unlock(&udc_lock);
   out:
  	trace_usb_gadget_disconnect(gadget, ret);
-- 
2.17.1