Linux USB
 help / color / mirror / Atom feed
From: "M. Vefa Bicakci" <m.v.b@runbox.com>
To: linux-usb@vger.kernel.org
Cc: "M. Vefa Bicakci" <m.v.b@runbox.com>,
	Andrey Konovalov <andreyknvl@google.com>,
	stable@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Alan Stern <stern@rowland.harvard.edu>,
	Bastien Nocera <hadess@hadess.net>,
	syzkaller@googlegroups.com
Subject: [PATCH 1/2] usbcore/driver: Fix specific driver selection
Date: Thu, 17 Sep 2020 12:59:58 +0300	[thread overview]
Message-ID: <20200917095959.174378-1-m.v.b@runbox.com> (raw)
In-Reply-To: <359d080c-5cbb-250a-0ebd-aaba5f5c530d@runbox.com>

This commit resolves two minor bugs in the selection/discovery of more
specific USB device drivers for devices that are currently bound to
generic USB device drivers.

The first 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() used to unconditionally use
to_usb_device_driver() on each device driver, without verifying that
the device driver in question is a USB device driver (as opposed to a
USB interface driver).

The second bug is related to 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 commit resolves both of the bugs, which were found by code
inspection and instrumentation after Andrey Konovalov's report
indicating USB/IP subsystem's misbehaviour with the generic USB device
driver matching code.

Reported-by: Andrey Konovalov <andreyknvl@google.com>
Fixes: d5643d2249 ("USB: Fix device driver race")
Link: https://lore.kernel.org/linux-usb/CAAeHK+zOrHnxjRFs=OE8T=O9208B9HP_oo8RZpyVOZ9AJ54pAA@mail.gmail.com/
Cc: <stable@vger.kernel.org> # 5.8
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Bastien Nocera <hadess@hadess.net>
Cc: <syzkaller@googlegroups.com>
Signed-off-by: M. Vefa Bicakci <m.v.b@runbox.com>
---
 drivers/usb/core/driver.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index c976ea9f9582..509bb0d5fa4f 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -907,10 +907,18 @@ static int usb_uevent(struct device *dev, struct kobj_uevent_env *env)
 
 static bool is_dev_usb_generic_driver(struct device *dev)
 {
-	struct usb_device_driver *udd = dev->driver ?
-		to_usb_device_driver(dev->driver) : NULL;
+	/* A non-existing driver can never be equal to &usb_generic_driver. */
+	if (!dev->driver)
+		return 0;
+
+	/* Check whether the driver is a USB interface driver, which is not
+	 * a USB device driver, and hence cannot be &usb_generic_driver.
+	 * (Plus, to_usb_device_driver is only valid for USB device drivers.)
+	 */
+	if (!is_usb_device_driver(dev->driver))
+		return 0;
 
-	return udd == &usb_generic_driver;
+	return to_usb_device_driver(dev->driver) == &usb_generic_driver;
 }
 
 static int __usb_bus_reprobe_drivers(struct device *dev, void *data)
@@ -924,7 +932,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);

base-commit: 871e6496207c6aa94134448779c77631a11bfa2e
-- 
2.26.2


  reply	other threads:[~2020-09-17 10:23 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-16 13:33 USB driver ID matching broken Andrey Konovalov
2020-09-16 14:15 ` Greg Kroah-Hartman
2020-09-16 14:39   ` Bastien Nocera
2020-09-16 15:58     ` M. Vefa Bicakci
2020-09-17  9:59       ` M. Vefa Bicakci [this message]
2020-09-17  9:59         ` [PATCH 2/2] usbip: Make the driver's match function specific M. Vefa Bicakci
2020-09-17 10:23         ` [PATCH 1/2] usbcore/driver: Fix specific driver selection Bastien Nocera
2020-09-17 10:39           ` M. Vefa Bicakci
2020-09-17 10:49             ` Bastien Nocera
2020-09-17 14:41               ` [PATCH 1/3] " M. Vefa Bicakci
2020-09-17 14:41                 ` [PATCH 2/3] usbcore/driver: Fix incorrect downcast M. Vefa Bicakci
2020-09-17 15:01                   ` Alan Stern
2020-09-18  9:26                     ` M. Vefa Bicakci
2020-09-17 14:41                 ` [PATCH 3/3] usbip: Make the driver's match function specific M. Vefa Bicakci
2020-09-17 15:21                   ` Shuah Khan
2020-09-18  9:26                     ` M. Vefa Bicakci
2020-09-18 14:31                       ` M. Vefa Bicakci
2020-09-18 15:49                         ` Shuah Khan
2020-09-19 13:54                           ` M. Vefa Bicakci
2020-09-21 17:03                             ` M. Vefa Bicakci
2020-09-18 14:31                 ` [PATCH 1/3] usbcore/driver: Fix specific driver selection M. Vefa Bicakci
2020-09-18 14:52                   ` Alan Stern
2020-09-19 13:52                     ` M. Vefa Bicakci

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=20200917095959.174378-1-m.v.b@runbox.com \
    --to=m.v.b@runbox.com \
    --cc=andreyknvl@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hadess@hadess.net \
    --cc=linux-usb@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    --cc=syzkaller@googlegroups.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