From patchwork Mon Nov 7 08:56:06 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiantao Zhang X-Patchwork-Id: 13034108 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E089DC43217 for ; Mon, 7 Nov 2022 08:56:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231211AbiKGI4Z (ORCPT ); Mon, 7 Nov 2022 03:56:25 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55418 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231592AbiKGI4M (ORCPT ); Mon, 7 Nov 2022 03:56:12 -0500 Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CEC85165AC; Mon, 7 Nov 2022 00:56:08 -0800 (PST) Received: from kwepemi500005.china.huawei.com (unknown [172.30.72.55]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4N5Q7N5kXkzRp6C; Mon, 7 Nov 2022 16:56:00 +0800 (CST) Received: from kwepemi500002.china.huawei.com (7.221.188.171) by kwepemi500005.china.huawei.com (7.221.188.179) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Mon, 7 Nov 2022 16:56:06 +0800 Received: from kwepemi500002.china.huawei.com ([7.221.188.171]) by kwepemi500002.china.huawei.com ([7.221.188.171]) with mapi id 15.01.2375.031; Mon, 7 Nov 2022 16:56:06 +0800 From: Jiantao Zhang To: "Xuetao (kirin)" , "gregkh@linuxfoundation.org" , "stern@rowland.harvard.edu" , "jakobkoschel@gmail.com" , "geert+renesas@glider.be" , "colin.i.king@gmail.com" , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" CC: Caiyadong , xuhaiyang , Suzhuangluan Subject: [PATCH] USB: gadget: Fix CFI failure during usb config switch. Thread-Topic: [PATCH] USB: gadget: Fix CFI failure during usb config switch. Thread-Index: AdjyhqKcM1K7gDGYQF24dNHpxaSVqg== Date: Mon, 7 Nov 2022 08:56:06 +0000 Message-ID: Accept-Language: zh-CN, en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.136.108.160] MIME-Version: 1.0 X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org This reverts commit 0a55187a1ec8c In the process of switching USB config from rndis to other config, if function gadget->ops->pullup() return an error,it will inevitably cause a CFI failure(Linux version 5.10.43). 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(),gadget->udc->driver->disconnect() will not be called when gadget->ops->pullup() return an error, therefore, pointer dev->port will not be set to NULL. (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 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 (init.usb.configfs.rc:11) CFI failure (target: [] 0000000068f50078): CPU: 2 PID: 1 Comm: init VIP: 00 Tainted: G W O 5.10.43 #1 Call trace: __cfi_slowpath+0x300/0x3bc rndis_signal_disconnect+0x1e0/0x204 rndis_close+0x24/0x2c eth_stop+0xd0/0x234 (if dev->port_usb != NULL, call rndis_close) __dev_close_many+0x204/0x2d4 dev_close_many+0x48/0x2c8 rollback_registered_many+0x184/0xdac unregister_netdevice_queue+0xf8/0x24c rndis_free_inst+0x78/0xc8 rndis_attr_release+0x3c/0x84 config_item_release+0x6c/0x180 configfs_rmdir+0x7e0/0xca0 Since the rndis memory has been freed in step2, function rndis_close cannot be called here. In function eth_stop(), if pointer dev->port_usb is NULL, function rndis_close() will not be called. So, if gadget->ops->pullup() return an error in step1, a CFI failure will be caused here. ====================================================== Through above analysis, i think gadget->udc->driver->disconnect() need to be called even if gadget->udc->driver->disconnect() return an error. Signed-off-by: Jiantao Zhang Signed-off-by: TaoXue --- drivers/usb/gadget/udc/core.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) } 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); - } out: trace_usb_gadget_disconnect(gadget, ret); @@ -1532,6 +1524,11 @@ static void gadget_unbind_driver(struct device *dev) kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE); usb_gadget_disconnect(gadget); + + mutex_lock(&udc_lock); + udc->driver->disconnect(udc->gadget); + mutex_unlock(&udc_lock); + usb_gadget_disable_async_callbacks(udc); if (gadget->irq) synchronize_irq(gadget->irq); @@ -1626,6 +1623,11 @@ static ssize_t soft_connect_store(struct device *dev, usb_gadget_connect(udc->gadget); } else if (sysfs_streq(buf, "disconnect")) { usb_gadget_disconnect(udc->gadget); + + mutex_lock(&udc_lock); + udc->driver->disconnect(udc->gadget); + mutex_unlock(&udc_lock); + usb_gadget_udc_stop(udc); } else { dev_err(dev, "unsupported command '%s'\n", buf); -- 2.17.1 diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c index c63c0c2cf649..b502b2ac4824 100644 --- a/drivers/usb/gadget/udc/core.c +++ b/drivers/usb/gadget/udc/core.c @@ -707,9 +707,6 @@ EXPORT_SYMBOL_GPL(usb_gadget_connect); * as a disconnect (when a VBUS session is active). Not all systems * support software pullup controls. * - * Following a successful disconnect, invoke the ->disconnect() callback - * for the current gadget driver so that UDC drivers don't need to. - * * Returns zero on success, else negative errno. */ int usb_gadget_disconnect(struct usb_gadget *gadget) @@ -734,13 +731,8 @@ int usb_gadget_disconnect(struct usb_gadget *gadget)