Linux USB
 help / color / mirror / Atom feed
From: jiantao zhang <water.zhangjiantao@huawei.com>
To: gregkh@linuxfoundation.org, stern@rowland.harvard.edu,
	jakobkoschel@gmail.com, geert+renesas@glider.be,
	张建涛 <water.zhangjiantao@huawei.com>,
	colin.i.king@gmail.com, 薛涛 <xuetao09@huawei.com>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: 蔡亚东 <caiyadong@huawei.com>, 徐海洋 <xuhaiyang5@hisilicon.com>,
	suzhuangluan@hisilicon.com
Subject: [PATCH v2] USB: gadget: Fix use-after-free during usb config switch
Date: Sat, 12 Nov 2022 11:12:46 +0800	[thread overview]
Message-ID: <27f1534f-a985-06e9-b94f-ba60167e74d2@huawei.com> (raw)
In-Reply-To: <20221112030433.4945-1-xuetao09@huawei.com>

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" (init.usb.configfs.rc:2)

gether_disconnect+0x2c/0x1f8 (dev->port_usb = NULL)
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

In function usb_gadget_disconnect(),The ->disconnect() callback will
not be called when gadget->ops->pullup() return an error, therefore,
pointer dev->port will not be set to NULL. If pointer dev->port_usb
is not null, it will cause an exception of use-after-free in step3.

(2) rm /config/usb_gadget/g1/configs/b.1/f1 (init.usb.configfs.rc:8)
(f1 -> ../../../../usb_gadget/g1/functions/rndis.gs4)

rndis_deregister+0x28/0x54 (kfree(params))
rndis_free+0x44/0x7c (kfree(rndis))
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
(init.usb.configfs.rc:11)

Call trace:
panic+0x1fc/0x3d0
die+0x29c/0x2a8
do_page_fault+0xa8/0x46c
do_mem_abort+0x3c/0xac
el1_sync_handler+0x40/0x78
0xffffff801138f880 (params->resp_avail is an illegal func pointer)
rndis_close+0x28/0x34 (->rndis_indicate_status_msg->params->resp_avail)
eth_stop+0x74/0x110 (if dev->port_usb != NULL, call rndis_close)
__dev_close_many+0x134/0x194
dev_close_many+0x48/0x194
rollback_registered_many+0x118/0x814
unregister_netdevice_queue+0xe0/0x168
unregister_netdev+0x20/0x30
gether_cleanup+0x1c/0x38
rndis_free_inst+0x2c/0x58
rndis_attr_release+0xc/0x14
kref_put+0x74/0xb8
config_item_put+0x14/0x1c
configfs_rmdir+0x314/0x374

In step3,function pointer params->resp_avail() is a wild pointer
becase pointer params has been freed in step2.

Free mem stack(in step2):
usb_put_function -> rndis_free -> rndis_deregister -> kfree(params)

use-after-free stack(in step3):
eth_stop -> rndis_close -> rndis_signal_disconnect ->
rndis_indicate_status_msg -> params->resp_avail()

In function eth_stop(), if pointer dev->port_usb is NULL, function
rndis_close() will not be called.
If gadget->ops->pullup() return an error in step1,dev->port_usb will
not be set to null. So, a panic will be caused in step3.
=======================================================================

Fixes:<0a55187a1ec8c> (USB: gadget core: Issue ->disconnect()
callback from usb_gadget_disconnect())
Signed-off-by: Jiantao Zhang <water.zhangjiantao@huawei.com>
Signed-off-by: TaoXue <xuetao09@huawei.com>
---
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

.


       reply	other threads:[~2022-11-12  3:13 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20221112030433.4945-1-xuetao09@huawei.com>
2022-11-12  3:12 ` jiantao zhang [this message]
2022-11-12  7:03   ` [PATCH v2] USB: gadget: Fix use-after-free during usb config switch Greg KH
2022-11-12  8:14 ` jiantao zhang
2022-11-13  9:12   ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=27f1534f-a985-06e9-b94f-ba60167e74d2@huawei.com \
    --to=water.zhangjiantao@huawei.com \
    --cc=caiyadong@huawei.com \
    --cc=colin.i.king@gmail.com \
    --cc=geert+renesas@glider.be \
    --cc=gregkh@linuxfoundation.org \
    --cc=jakobkoschel@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    --cc=suzhuangluan@hisilicon.com \
    --cc=xuetao09@huawei.com \
    --cc=xuhaiyang5@hisilicon.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox