From patchwork Tue Sep 22 11:07:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "M. Vefa Bicakci" X-Patchwork-Id: 297547 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E5B3AC4727E for ; Tue, 22 Sep 2020 11:07:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9A36920B1F for ; Tue, 22 Sep 2020 11:07:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726582AbgIVLHf (ORCPT ); Tue, 22 Sep 2020 07:07:35 -0400 Received: from aibo.runbox.com ([91.220.196.211]:51628 "EHLO aibo.runbox.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726563AbgIVLHf (ORCPT ); Tue, 22 Sep 2020 07:07:35 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=runbox.com; s=selector2; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To :Message-Id:Date:Subject:Cc:To:From; bh=psqE07dXlcAdf8xwU1osA4X0VG70gu6FTObLRO7oA7Y=; b=ZMQwj+445i4FxUocxpvqWlXVw3 BEqiV3gKbzDiSMSGPvNX+5HwDGeF+LTd9ReJRv2pwLNETTiC/PXg6l5IPoaPrmGdJIv/R7MHw/Pl2 ggBPhn+o8KSX6qPJ4zl0++G+tgulw8W/nzvj3W+czioT4XJ/s+6Ews1KRUdiPUJuhUgqe1ag4CxhA hGHaPQpeTJssU7gAle9SOwfqqD9M38ZJxY8h4Kywm4drc121YK9X6kUAOUjoF2esbbdOYfK8nc/Tn o1Q/Ri4G0K0KoxXZxB0MMQUo1GrrGdAeVtpaKM3Q4Asd6BijmByveExGSg0+3IsyxzHzjrAH5RRLo ar2nmPDQ==; Received: from [10.9.9.73] (helo=submission02.runbox) by mailtransmit03.runbox with esmtp (Exim 4.86_2) (envelope-from ) id 1kKg8z-0005vk-Ce; Tue, 22 Sep 2020 13:07:33 +0200 Received: by submission02.runbox with esmtpsa [Authenticated alias (536975)] (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) id 1kKg8k-0000Pn-3h; Tue, 22 Sep 2020 13:07:18 +0200 From: "M. Vefa Bicakci" To: linux-usb@vger.kernel.org Cc: "M. Vefa Bicakci" , stable@vger.kernel.org, Bastien Nocera , Valentina Manea , Shuah Khan , Greg Kroah-Hartman , Alan Stern , syzkaller@googlegroups.com Subject: [PATCH v3 1/4] Revert "usbip: Implement a match function to fix usbip" Date: Tue, 22 Sep 2020 14:07:00 +0300 Message-Id: <20200922110703.720960-2-m.v.b@runbox.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200922110703.720960-1-m.v.b@runbox.com> References: <20200922110703.720960-1-m.v.b@runbox.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org This commit reverts commit 7a2f2974f265 ("usbip: Implement a match function to fix usbip"). In summary, commit d5643d2249b2 ("USB: Fix device driver race") inadvertently broke usbip functionality, which I resolved in an incorrect manner by introducing a match function to usbip, usbip_match(), that unconditionally returns true. However, the usbip_match function, as is, causes usbip to take over virtual devices used by syzkaller for USB fuzzing, which is a regression reported by Andrey Konovalov. Furthermore, in conjunction with the fix of another bug, handled by another patch titled "usbcore/driver: Fix specific driver selection" in this patch set, the usbip_match function causes unexpected USB subsystem behaviour when the usbip_host driver is loaded. The unexpected behaviour can be qualified as follows: - If commit 41160802ab8e ("USB: Simplify USB ID table match") is included in the kernel, then all USB devices are bound to the usbip_host driver, which appears to the user as if all USB devices were disconnected. - If the same commit (41160802ab8e) is not in the kernel (as is the case with v5.8.10) then all USB devices are re-probed and re-bound to their original device drivers, which appears to the user as a disconnection and re-connection of USB devices. Please note that this commit will make usbip non-operational again, until yet another patch in this patch set is merged, titled "usbcore/driver: Accommodate usbip". Reported-by: Andrey Konovalov Link: https://lore.kernel.org/linux-usb/CAAeHK+zOrHnxjRFs=OE8T=O9208B9HP_oo8RZpyVOZ9AJ54pAA@mail.gmail.com/ Cc: # 5.8: 41160802ab8e: USB: Simplify USB ID table match Cc: # 5.8 Cc: Bastien Nocera Cc: Valentina Manea Cc: Shuah Khan Cc: Greg Kroah-Hartman Cc: Alan Stern Cc: Signed-off-by: M. Vefa Bicakci Acked-by: Shuah Khan --- v3: New patch in the patch set. Note for stable tree maintainers: I have marked the following commit as a dependency of this patch, because that commit resolves a bug that the next commit in this patch set uncovers, where if a driver does not have an id_table, then its match function is not considered for execution at all. commit 41160802ab8e ("USB: Simplify USB ID table match") --- drivers/usb/usbip/stub_dev.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/usb/usbip/stub_dev.c b/drivers/usb/usbip/stub_dev.c index 9d7d642022d1..2305d425e6c9 100644 --- a/drivers/usb/usbip/stub_dev.c +++ b/drivers/usb/usbip/stub_dev.c @@ -461,11 +461,6 @@ static void stub_disconnect(struct usb_device *udev) return; } -static bool usbip_match(struct usb_device *udev) -{ - return true; -} - #ifdef CONFIG_PM /* These functions need usb_port_suspend and usb_port_resume, @@ -491,7 +486,6 @@ struct usb_device_driver stub_driver = { .name = "usbip-host", .probe = stub_probe, .disconnect = stub_disconnect, - .match = usbip_match, #ifdef CONFIG_PM .suspend = stub_suspend, .resume = stub_resume, From patchwork Tue Sep 22 11:07:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "M. Vefa Bicakci" X-Patchwork-Id: 258457 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1AD9FC2D0E2 for ; Tue, 22 Sep 2020 11:07:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B8ADD23A1B for ; Tue, 22 Sep 2020 11:07:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726589AbgIVLHg (ORCPT ); Tue, 22 Sep 2020 07:07:36 -0400 Received: from aibo.runbox.com ([91.220.196.211]:51622 "EHLO aibo.runbox.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726340AbgIVLHg (ORCPT ); Tue, 22 Sep 2020 07:07:36 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=runbox.com; s=selector2; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To :Message-Id:Date:Subject:Cc:To:From; bh=J9tHTJNHwgpZ+uiXbuJyprpeAr/tHIoXWflEBxP+l6w=; b=gSVHNrVRxOhK9vtDhSrARqze4+ vG88XGkfylxvPet48bO/a/YoP4yLI11JrIZ+t7ilbQq8QjdDulPH6LKZMuH+qYsl2TBPIkmVo9Xn/ IOr12TBkh2iGbklEytSBd2LHfc0WdShYO2FK1vsSITI0SV/uONNIkDLFnPnQyPYJW1JtLGdK9DI8T SSqVoWXTrR5TJRZ/hMUNZspj/ftO/DWZOn1iDUzxwO4ieNJsZztByLHHWkMtrb5+MyqcK8k8SdHhc FkW57ONhvTS/W7Rbs/v8YrZMgon6Ug92F06+kz5InuV/mlYF3W9ay3zMX+ZKV6OLpxracq0EzB+gX OsZUKDNA==; Received: from [10.9.9.73] (helo=submission02.runbox) by mailtransmit03.runbox with esmtp (Exim 4.86_2) (envelope-from ) id 1kKg8z-0005ve-1s; Tue, 22 Sep 2020 13:07:33 +0200 Received: by submission02.runbox with esmtpsa [Authenticated alias (536975)] (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) id 1kKg8q-0000Pz-3x; Tue, 22 Sep 2020 13:07:24 +0200 From: "M. Vefa Bicakci" To: linux-usb@vger.kernel.org Cc: "M. Vefa Bicakci" , stable@vger.kernel.org, Greg Kroah-Hartman , Alan Stern , Bastien Nocera , Shuah Khan , Valentina Manea , syzkaller@googlegroups.com Subject: [PATCH v3 2/4] usbcore/driver: Fix specific driver selection Date: Tue, 22 Sep 2020 14:07:01 +0300 Message-Id: <20200922110703.720960-3-m.v.b@runbox.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200922110703.720960-1-m.v.b@runbox.com> References: <20200922110703.720960-1-m.v.b@runbox.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org This commit resolves a bug in the selection/discovery of more specific USB device drivers for devices that are currently bound to generic USB device drivers. The bug is in the logic that determines whether a device currently bound to a generic USB device driver should be re-probed by a more specific USB device driver or not. The code in __usb_bus_reprobe_drivers() used to have the following lines: if (usb_device_match_id(udev, new_udriver->id_table) == NULL && (!new_udriver->match || new_udriver->match(udev) != 0)) return 0; ret = device_reprobe(dev); As the reader will notice, the code checks whether the USB device in consideration matches the identifier table (id_table) of a specific USB device_driver (new_udriver), followed by a similar check, but this time with the USB device driver's match function. However, the match function's return value is not checked correctly. When match() returns zero, it means that the specific USB device driver is *not* applicable to the USB device in question, but the code then goes on to reprobe the device with the new USB device driver under consideration. All this to say, the logic is inverted. This bug was found by code inspection and instrumentation while investigating the root cause of the issue reported by Andrey Konovalov, where usbip took over syzkaller's virtual USB devices in an undesired manner. The report is linked below. Fixes: d5643d2249 ("USB: Fix device driver race") Link: https://lore.kernel.org/linux-usb/CAAeHK+zOrHnxjRFs=OE8T=O9208B9HP_oo8RZpyVOZ9AJ54pAA@mail.gmail.com/ Cc: # 5.8 Cc: Greg Kroah-Hartman Cc: Alan Stern Cc: Bastien Nocera Cc: Shuah Khan Cc: Valentina Manea Cc: Signed-off-by: M. Vefa Bicakci --- v3: No functional changes; only commit message changes. v2: Following Bastien Nocera's suggestion, this is a new patch, split from the patch published at: https://lore.kernel.org/linux-usb/20200917095959.174378-1-m.v.b@runbox.com/ --- drivers/usb/core/driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index c976ea9f9582..950044a6e77f 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -924,7 +924,7 @@ static int __usb_bus_reprobe_drivers(struct device *dev, void *data) udev = to_usb_device(dev); if (usb_device_match_id(udev, new_udriver->id_table) == NULL && - (!new_udriver->match || new_udriver->match(udev) != 0)) + (!new_udriver->match || new_udriver->match(udev) == 0)) return 0; ret = device_reprobe(dev); From patchwork Tue Sep 22 11:07:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "M. Vefa Bicakci" X-Patchwork-Id: 297546 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4EECDC2D0E2 for ; Tue, 22 Sep 2020 11:07:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 062B5239D2 for ; Tue, 22 Sep 2020 11:07:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726577AbgIVLHf (ORCPT ); Tue, 22 Sep 2020 07:07:35 -0400 Received: from aibo.runbox.com ([91.220.196.211]:40220 "EHLO aibo.runbox.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726533AbgIVLHf (ORCPT ); Tue, 22 Sep 2020 07:07:35 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=runbox.com; s=selector2; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To :Message-Id:Date:Subject:Cc:To:From; bh=8csiEGr24be+WDvlGFb8o+D++Lm8+TY5ZC6fpXtGTsg=; b=OdOavOZgMqBPM0+1EPG9BX0Rpv dslBV3UscSPvJt384cqAIJDV8vBvGDL3NQrfCnKRdYU3jqc2P1PlSCp+pszhdmrz24dqFqsKHxn6p 7ICdp/6oinZ/81NPfUqMNtWXT1P5KWRcACxAn5uBZCL6mnSaCo/SV/6X53eQ72aghP+JGw0Gfsppe EXPB+Ta6l4kwk4B3L6QGQVuD5i/qwkJk8fkPZHNURuAnnMhXtvBXpGjS52O4VkSBI4cKv8rvyzC4k 1/0K0KlJkpVJrP9+7wylwZh915mMMLTtDonMuKdLoLGZDiHwy+2PUDCSp17uedequvHB9yRIk1l+F yOnF19fg==; Received: from [10.9.9.73] (helo=submission02.runbox) by mailtransmit02.runbox with esmtp (Exim 4.86_2) (envelope-from ) id 1kKg8z-00053H-6V; Tue, 22 Sep 2020 13:07:33 +0200 Received: by submission02.runbox with esmtpsa [Authenticated alias (536975)] (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) id 1kKg8w-0000QD-MX; Tue, 22 Sep 2020 13:07:31 +0200 From: "M. Vefa Bicakci" To: linux-usb@vger.kernel.org Cc: "M. Vefa Bicakci" , stable@vger.kernel.org, Greg Kroah-Hartman , Alan Stern , Bastien Nocera , Shuah Khan , Valentina Manea , syzkaller@googlegroups.com Subject: [PATCH v3 3/4] usbcore/driver: Fix incorrect downcast Date: Tue, 22 Sep 2020 14:07:02 +0300 Message-Id: <20200922110703.720960-4-m.v.b@runbox.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200922110703.720960-1-m.v.b@runbox.com> References: <20200922110703.720960-1-m.v.b@runbox.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org This commit resolves a minor bug in the selection/discovery of more specific USB device drivers for devices that are currently bound to generic USB device drivers. The bug is related to the way a candidate USB device driver is compared against the generic USB device driver. The code in is_dev_usb_generic_driver() assumes that the device driver in question is a USB device driver by calling to_usb_device_driver(dev->driver) to downcast; however I have observed that this assumption is not always true, through code instrumentation. This commit avoids the incorrect downcast altogether by comparing the USB device's driver (i.e., dev->driver) to the generic USB device driver directly. This method was suggested by Alan Stern. This bug was found while investigating Andrey Konovalov's report indicating usbip device driver misbehaviour with the recently merged generic USB device driver selection feature. The report is linked below. Fixes: d5643d2249 ("USB: Fix device driver race") Link: https://lore.kernel.org/linux-usb/CAAeHK+zOrHnxjRFs=OE8T=O9208B9HP_oo8RZpyVOZ9AJ54pAA@mail.gmail.com/ Cc: # 5.8 Cc: Greg Kroah-Hartman Cc: Alan Stern Cc: Bastien Nocera Cc: Shuah Khan Cc: Valentina Manea Cc: Signed-off-by: M. Vefa Bicakci --- v3: Simplify the device driver pointer comparison to avoid incorrect downcast, as suggested by Alan Stern. Minor edits to the commit message. v2: Following Bastien Nocera's code review comment, use is_usb_device() to verify that a device is bound to a USB device driver (as opposed to, e.g., a USB interface driver) to avoid incorrect downcast. --- drivers/usb/core/driver.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 950044a6e77f..7d90cbe063ec 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -905,21 +905,14 @@ static int usb_uevent(struct device *dev, struct kobj_uevent_env *env) return 0; } -static bool is_dev_usb_generic_driver(struct device *dev) -{ - struct usb_device_driver *udd = dev->driver ? - to_usb_device_driver(dev->driver) : NULL; - - return udd == &usb_generic_driver; -} - static int __usb_bus_reprobe_drivers(struct device *dev, void *data) { struct usb_device_driver *new_udriver = data; struct usb_device *udev; int ret; - if (!is_dev_usb_generic_driver(dev)) + /* Don't reprobe if current driver isn't usb_generic_driver */ + if (dev->driver != &usb_generic_driver.drvwrap.driver) return 0; udev = to_usb_device(dev); From patchwork Tue Sep 22 11:07:03 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "M. Vefa Bicakci" X-Patchwork-Id: 258456 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A9244C4727E for ; Tue, 22 Sep 2020 11:07:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 62C17208A9 for ; Tue, 22 Sep 2020 11:07:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726593AbgIVLHk (ORCPT ); Tue, 22 Sep 2020 07:07:40 -0400 Received: from aibo.runbox.com ([91.220.196.211]:40240 "EHLO aibo.runbox.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726340AbgIVLHk (ORCPT ); Tue, 22 Sep 2020 07:07:40 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=runbox.com; s=selector2; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To :Message-Id:Date:Subject:Cc:To:From; bh=1K5YriAZ+QeTiFeBXlTzxJqiLhAXdhR8YIm2hSleeAY=; b=DPOZ/vBvVeE5VkFqICVEVzL/1f DQes4jcQp2KInC8Ztk26cjPIhAFpKkdJvHyVb7Dh+/4obzamxrZfkheHkAxs7YCbfQ7eKh2uDGtkv s2+TePfYfqeiYwPD+45fZrQSNEqsIh0PTWJOunXMxo22+9hS4iRdiRqRz4wbEcWNnQV+3VVBrJfGO EVnSdItwxWWuFneoSuCK7iYEK9PBVX/SNFX3+xA3+I14ZtWNfEGLgQia+1cFdfPnHvf6mJ9rKfEzR 22a4K/5ojfbKmg0AGPCkDhgU+yqeb8M1UW0BHH2ptPxXiMS4O0uwP1DXV5jZaCdKNgGhn9HLK5F4f Gx8VJ7Zw==; Received: from [10.9.9.74] (helo=submission03.runbox) by mailtransmit02.runbox with esmtp (Exim 4.86_2) (envelope-from ) id 1kKg93-00053W-Dk; Tue, 22 Sep 2020 13:07:37 +0200 Received: by submission03.runbox with esmtpsa [Authenticated alias (536975)] (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) id 1kKg92-00062a-VZ; Tue, 22 Sep 2020 13:07:37 +0200 From: "M. Vefa Bicakci" To: linux-usb@vger.kernel.org Cc: "M. Vefa Bicakci" , stable@vger.kernel.org, Bastien Nocera , Valentina Manea , Shuah Khan , Greg Kroah-Hartman , Alan Stern , syzkaller@googlegroups.com Subject: [PATCH v3 4/4] usbcore/driver: Accommodate usbip Date: Tue, 22 Sep 2020 14:07:03 +0300 Message-Id: <20200922110703.720960-5-m.v.b@runbox.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200922110703.720960-1-m.v.b@runbox.com> References: <20200922110703.720960-1-m.v.b@runbox.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org Commit 88b7381a939d ("USB: Select better matching USB drivers when available") inadvertently broke usbip functionality. The commit in question allows USB device drivers to be explicitly matched with USB devices via the use of driver-provided identifier tables and match functions, which is useful for a specialised device driver to be chosen for a device that can also be handled by another, more generic, device driver. Prior, the USB device section of usb_device_match() had an unconditional "return 1" statement, which allowed user-space to bind USB devices to the usbip_host device driver, if desired. However, the aforementioned commit changed the default/fallback return value to zero. This breaks device drivers such as usbip_host, so this commit restores the legacy behaviour, but only if a device driver does not have an id_table and a match() function. In addition, if usb_device_match is called for a device driver and device pair where the device does not match the id_table of the device driver in question, then the device driver will be disqualified for the device. This allows avoiding the default case of "return 1", which prevents undesirable probe() calls to a driver even though its id_table did not match the device. Finally, this commit changes the specialised-driver-to-generic-driver transition code so that when a device driver returns -ENODEV, a more generic device driver is only considered if the current device driver does not have an id_table and a match() function. This ensures that "generic" drivers such as usbip_host will not be considered specialised device drivers and will not cause the device to be locked in to the generic device driver, when a more specialised device driver could be tried. All of these changes restore usbip functionality without regressions, ensure that the specialised/generic device driver selection logic works as expected with the usb and apple-mfi-fastcharge drivers, and do not negatively affect the use of devices provided by dummy_hcd. Fixes: 88b7381a939d ("USB: Select better matching USB drivers when available") Link: https://lore.kernel.org/linux-usb/CAAeHK+zOrHnxjRFs=OE8T=O9208B9HP_oo8RZpyVOZ9AJ54pAA@mail.gmail.com/ Cc: # 5.8 Cc: Bastien Nocera Cc: Valentina Manea Cc: Shuah Khan Cc: Greg Kroah-Hartman Cc: Alan Stern Cc: Signed-off-by: M. Vefa Bicakci Acked-by: Shuah Khan --- v3: New patch in this patch set. For reviewers' awareness, another option for usb_device_match would be as follows. This allows the driver's match() function to act as a fallback in case the driver has an id_table, but the id_table does not include the device. if (!udrv->id_table && !udrv->match) { /* Allow usbip and similar drivers to bind to * arbitrary devices; let their probe functions * decide. */ return 1; } if (udrv->id_table && usb_device_match_id(udev, udrv->id_table) != NULL) return 1; if (udrv->match && udrv->match(udev)) return 1; return 0; --- drivers/usb/core/driver.c | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 7d90cbe063ec..98b7449c11f3 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -269,8 +269,30 @@ static int usb_probe_device(struct device *dev) if (error) return error; + /* Probe the USB device with the driver in hand, but only + * defer to a generic driver in case the current USB + * device driver has an id_table or a match function; i.e., + * when the device driver was explicitly matched against + * a device. + * + * If the device driver does not have either of these, + * then we assume that it can bind to any device and is + * not truly a more specialized/non-generic driver, so a + * return value of -ENODEV should not force the device + * to be handled by the generic USB driver, as there + * can still be another, more specialized, device driver. + * + * This accommodates the usbip driver. + * + * TODO: What if, in the future, there are multiple + * specialized USB device drivers for a particular device? + * In such cases, there is a need to try all matching + * specialised device drivers prior to setting the + * use_generic_driver bit. + */ error = udriver->probe(udev); - if (error == -ENODEV && udriver != &usb_generic_driver) { + if (error == -ENODEV && udriver != &usb_generic_driver && + (udriver->id_table || udriver->match)) { udev->use_generic_driver = 1; return -EPROBE_DEFER; } @@ -831,14 +853,17 @@ static int usb_device_match(struct device *dev, struct device_driver *drv) udev = to_usb_device(dev); udrv = to_usb_device_driver(drv); - if (udrv->id_table && - usb_device_match_id(udev, udrv->id_table) != NULL) { - return 1; - } + if (udrv->id_table) + return usb_device_match_id(udev, udrv->id_table) != NULL; if (udrv->match) return udrv->match(udev); - return 0; + + /* If the device driver under consideration does not have a + * id_table or a match function, then let the driver's probe + * function decide. + */ + return 1; } else if (is_usb_interface(dev)) { struct usb_interface *intf;