1. 29 Sep, 2020 1 commit
  2. 25 Sep, 2020 4 commits
    • M. Vefa Bicakci's avatar
      usbcore/driver: Accommodate usbip · 3fce3960
      M. Vefa Bicakci authored
      Commit 88b7381a ("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: 88b7381a ("USB: Select better matching USB drivers when available")
      Cc: <stable@vger.kernel.org> # 5.8
      Cc: Bastien Nocera <hadess@hadess.net>
      Cc: Valentina Manea <valentina.manea.m@gmail.com>
      Cc: Shuah Khan <shuah@kernel.org>
      Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
      Cc: Alan Stern <stern@rowland.harvard.edu>
      Cc: <syzkaller@googlegroups.com>
      Tested-by: default avatarAndrey Konovalov <andreyknvl@google.com>
      Acked-by: default avatarShuah Khan <skhan@linuxfoundation.org>
      Signed-off-by: default avatarM. Vefa Bicakci <m.v.b@runbox.com>
      Link: https://lore.kernel.org/r/20200922110703.720960-5-m.v.b@runbox.comSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      3fce3960
    • M. Vefa Bicakci's avatar
      usbcore/driver: Fix incorrect downcast · 4df30e76
      M. Vefa Bicakci authored
      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: d5643d22 ("USB: Fix device driver race")
      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: Shuah Khan <shuah@kernel.org>
      Cc: Valentina Manea <valentina.manea.m@gmail.com>
      Cc: <syzkaller@googlegroups.com>
      Tested-by: default avatarAndrey Konovalov <andreyknvl@google.com>
      Signed-off-by: default avatarM. Vefa Bicakci <m.v.b@runbox.com>
      Link: https://lore.kernel.org/r/20200922110703.720960-4-m.v.b@runbox.comSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      4df30e76
    • M. Vefa Bicakci's avatar
      usbcore/driver: Fix specific driver selection · aea850cd
      M. Vefa Bicakci authored
      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: d5643d22 ("USB: Fix device driver race")
      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: Shuah Khan <shuah@kernel.org>
      Cc: Valentina Manea <valentina.manea.m@gmail.com>
      Cc: <syzkaller@googlegroups.com>
      Tested-by: default avatarAndrey Konovalov <andreyknvl@google.com>
      Signed-off-by: default avatarM. Vefa Bicakci <m.v.b@runbox.com>
      Link: https://lore.kernel.org/r/20200922110703.720960-3-m.v.b@runbox.comSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      aea850cd
    • M. Vefa Bicakci's avatar
      Revert "usbip: Implement a match function to fix usbip" · d6407613
      M. Vefa Bicakci authored
      This commit reverts commit 7a2f2974 ("usbip: Implement a match
      function to fix usbip").
      
      In summary, commit d5643d22 ("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".
      
      Cc: <stable@vger.kernel.org> # 5.8: 41160802ab8e: USB: Simplify USB ID table match
      Cc: <stable@vger.kernel.org> # 5.8
      Cc: Bastien Nocera <hadess@hadess.net>
      Cc: Valentina Manea <valentina.manea.m@gmail.com>
      Cc: Shuah Khan <shuah@kernel.org>
      Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
      Cc: Alan Stern <stern@rowland.harvard.edu>
      Cc: <syzkaller@googlegroups.com>
      Reported-by: default avatarAndrey Konovalov <andreyknvl@google.com>
      Tested-by: default avatarAndrey Konovalov <andreyknvl@google.com>
      Acked-by: default avatarShuah Khan <skhan@linuxfoundation.org>
      Signed-off-by: default avatarM. Vefa Bicakci <m.v.b@runbox.com>
      Link: https://lore.kernel.org/r/20200922110703.720960-2-m.v.b@runbox.comSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      d6407613
  3. 22 Sep, 2020 1 commit
    • Bryan O'Donoghue's avatar
      USB: gadget: f_ncm: Fix NDP16 datagram validation · 2b405533
      Bryan O'Donoghue authored
      commit 2b74b0a0 ("USB: gadget: f_ncm: add bounds checks to ncm_unwrap_ntb()")
      adds important bounds checking however it unfortunately also introduces  a
      bug with respect to section 3.3.1 of the NCM specification.
      
      wDatagramIndex[1] : "Byte index, in little endian, of the second datagram
      described by this NDP16. If zero, then this marks the end of the sequence
      of datagrams in this NDP16."
      
      wDatagramLength[1]: "Byte length, in little endian, of the second datagram
      described by this NDP16. If zero, then this marks the end of the sequence
      of datagrams in this NDP16."
      
      wDatagramIndex[1] and wDatagramLength[1] respectively then may be zero but
      that does not mean we should throw away the data referenced by
      wDatagramIndex[0] and wDatagramLength[0] as is currently the case.
      
      Breaking the loop on (index2 == 0 || dg_len2 == 0) should come at the end
      as was previously the case and checks for index2 and dg_len2 should be
      removed since zero is valid.
      
      I'm not sure how much testing the above patch received but for me right now
      after enumeration ping doesn't work. Reverting the commit restores ping,
      scp, etc.
      
      The extra validation associated with wDatagramIndex[0] and
      wDatagramLength[0] appears to be valid so, this change removes the incorrect
      restriction on wDatagramIndex[1] and wDatagramLength[1] restoring data
      processing between host and device.
      
      Fixes: 2b74b0a0 ("USB: gadget: f_ncm: add bounds checks to ncm_unwrap_ntb()")
      Cc: Ilja Van Sprundel <ivansprundel@ioactive.com>
      Cc: Brooke Basile <brookebasile@gmail.com>
      Cc: stable <stable@kernel.org>
      Signed-off-by: default avatarBryan O'Donoghue <bryan.odonoghue@linaro.org>
      Link: https://lore.kernel.org/r/20200920170158.1217068-1-bryan.odonoghue@linaro.orgSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      2b405533
  4. 21 Sep, 2020 2 commits
  5. 20 Sep, 2020 20 commits
  6. 19 Sep, 2020 12 commits