From: Greg Kroah-Hartman Date: Sun, 4 Oct 2020 09:52:44 +0000 (+0200) Subject: 5.8-stable patches X-Git-Tag: v4.19.150~40 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c4fd197133a62b7bb23dd178170f522c0f207909;p=thirdparty%2Fkernel%2Fstable-queue.git 5.8-stable patches added patches: revert-usbip-implement-a-match-function-to-fix-usbip.patch usb-gadget-f_ncm-fix-ndp16-datagram-validation.patch usbcore-driver-accommodate-usbip.patch usbcore-driver-fix-incorrect-downcast.patch usbcore-driver-fix-specific-driver-selection.patch --- diff --git a/queue-5.8/revert-usbip-implement-a-match-function-to-fix-usbip.patch b/queue-5.8/revert-usbip-implement-a-match-function-to-fix-usbip.patch new file mode 100644 index 00000000000..73e81ad8e8b --- /dev/null +++ b/queue-5.8/revert-usbip-implement-a-match-function-to-fix-usbip.patch @@ -0,0 +1,80 @@ +From d6407613c1e2ef90213dee388aa16b6e1bd08cbc Mon Sep 17 00:00:00 2001 +From: "M. Vefa Bicakci" +Date: Tue, 22 Sep 2020 14:07:00 +0300 +Subject: Revert "usbip: Implement a match function to fix usbip" + +From: M. Vefa Bicakci + +commit d6407613c1e2ef90213dee388aa16b6e1bd08cbc upstream. + +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". + +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: +Reported-by: Andrey Konovalov +Tested-by: Andrey Konovalov +Acked-by: Shuah Khan +Signed-off-by: M. Vefa Bicakci +Link: https://lore.kernel.org/r/20200922110703.720960-2-m.v.b@runbox.com +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/usb/usbip/stub_dev.c | 6 ------ + 1 file changed, 6 deletions(-) + +--- a/drivers/usb/usbip/stub_dev.c ++++ b/drivers/usb/usbip/stub_dev.c +@@ -461,11 +461,6 @@ static void stub_disconnect(struct usb_d + 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, diff --git a/queue-5.8/series b/queue-5.8/series index 78a4656bcc3..5182bc39a3d 100644 --- a/queue-5.8/series +++ b/queue-5.8/series @@ -1,3 +1,8 @@ io_uring-always-delete-double-poll-wait-entry-on-match.patch btrfs-fix-filesystem-corruption-after-a-device-replace.patch mmc-sdhci-workaround-broken-command-queuing-on-intel-glk-based-irbis-models.patch +usb-gadget-f_ncm-fix-ndp16-datagram-validation.patch +revert-usbip-implement-a-match-function-to-fix-usbip.patch +usbcore-driver-fix-specific-driver-selection.patch +usbcore-driver-fix-incorrect-downcast.patch +usbcore-driver-accommodate-usbip.patch diff --git a/queue-5.8/usb-gadget-f_ncm-fix-ndp16-datagram-validation.patch b/queue-5.8/usb-gadget-f_ncm-fix-ndp16-datagram-validation.patch new file mode 100644 index 00000000000..30ce13f3ff7 --- /dev/null +++ b/queue-5.8/usb-gadget-f_ncm-fix-ndp16-datagram-validation.patch @@ -0,0 +1,124 @@ +From 2b405533c2560d7878199c57d95a39151351df72 Mon Sep 17 00:00:00 2001 +From: Bryan O'Donoghue +Date: Sun, 20 Sep 2020 18:01:58 +0100 +Subject: USB: gadget: f_ncm: Fix NDP16 datagram validation + +From: Bryan O'Donoghue + +commit 2b405533c2560d7878199c57d95a39151351df72 upstream. + +commit 2b74b0a04d3e ("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: 2b74b0a04d3e ("USB: gadget: f_ncm: add bounds checks to ncm_unwrap_ntb()") +Cc: Ilja Van Sprundel +Cc: Brooke Basile +Cc: stable +Signed-off-by: Bryan O'Donoghue +Link: https://lore.kernel.org/r/20200920170158.1217068-1-bryan.odonoghue@linaro.org +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/usb/gadget/function/f_ncm.c | 30 ++---------------------------- + 1 file changed, 2 insertions(+), 28 deletions(-) + +--- a/drivers/usb/gadget/function/f_ncm.c ++++ b/drivers/usb/gadget/function/f_ncm.c +@@ -1189,7 +1189,6 @@ static int ncm_unwrap_ntb(struct gether + const struct ndp_parser_opts *opts = ncm->parser_opts; + unsigned crc_len = ncm->is_crc ? sizeof(uint32_t) : 0; + int dgram_counter; +- bool ndp_after_header; + + /* dwSignature */ + if (get_unaligned_le32(tmp) != opts->nth_sign) { +@@ -1216,7 +1215,6 @@ static int ncm_unwrap_ntb(struct gether + } + + ndp_index = get_ncm(&tmp, opts->ndp_index); +- ndp_after_header = false; + + /* Run through all the NDP's in the NTB */ + do { +@@ -1232,8 +1230,6 @@ static int ncm_unwrap_ntb(struct gether + ndp_index); + goto err; + } +- if (ndp_index == opts->nth_size) +- ndp_after_header = true; + + /* + * walk through NDP +@@ -1312,37 +1308,13 @@ static int ncm_unwrap_ntb(struct gether + index2 = get_ncm(&tmp, opts->dgram_item_len); + dg_len2 = get_ncm(&tmp, opts->dgram_item_len); + +- if (index2 == 0 || dg_len2 == 0) +- break; +- + /* wDatagramIndex[1] */ +- if (ndp_after_header) { +- if (index2 < opts->nth_size + opts->ndp_size) { +- INFO(port->func.config->cdev, +- "Bad index: %#X\n", index2); +- goto err; +- } +- } else { +- if (index2 < opts->nth_size + opts->dpe_size) { +- INFO(port->func.config->cdev, +- "Bad index: %#X\n", index2); +- goto err; +- } +- } + if (index2 > block_len - opts->dpe_size) { + INFO(port->func.config->cdev, + "Bad index: %#X\n", index2); + goto err; + } + +- /* wDatagramLength[1] */ +- if ((dg_len2 < 14 + crc_len) || +- (dg_len2 > frame_max)) { +- INFO(port->func.config->cdev, +- "Bad dgram length: %#X\n", dg_len); +- goto err; +- } +- + /* + * Copy the data into a new skb. + * This ensures the truesize is correct +@@ -1359,6 +1331,8 @@ static int ncm_unwrap_ntb(struct gether + ndp_len -= 2 * (opts->dgram_item_len * 2); + + dgram_counter++; ++ if (index2 == 0 || dg_len2 == 0) ++ break; + } while (ndp_len > 2 * (opts->dgram_item_len * 2)); + } while (ndp_index); + diff --git a/queue-5.8/usbcore-driver-accommodate-usbip.patch b/queue-5.8/usbcore-driver-accommodate-usbip.patch new file mode 100644 index 00000000000..82c105eda58 --- /dev/null +++ b/queue-5.8/usbcore-driver-accommodate-usbip.patch @@ -0,0 +1,121 @@ +From 3fce39601a1a34d940cf62858ee01ed9dac5d459 Mon Sep 17 00:00:00 2001 +From: "M. Vefa Bicakci" +Date: Tue, 22 Sep 2020 14:07:03 +0300 +Subject: usbcore/driver: Accommodate usbip + +From: M. Vefa Bicakci + +commit 3fce39601a1a34d940cf62858ee01ed9dac5d459 upstream. + +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") +Cc: # 5.8 +Cc: Bastien Nocera +Cc: Valentina Manea +Cc: Shuah Khan +Cc: Greg Kroah-Hartman +Cc: Alan Stern +Cc: +Tested-by: Andrey Konovalov +Acked-by: Shuah Khan +Signed-off-by: M. Vefa Bicakci +Link: https://lore.kernel.org/r/20200922110703.720960-5-m.v.b@runbox.com +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/usb/core/driver.c | 37 +++++++++++++++++++++++++++++++------ + 1 file changed, 31 insertions(+), 6 deletions(-) + +--- a/drivers/usb/core/driver.c ++++ b/drivers/usb/core/driver.c +@@ -269,8 +269,30 @@ static int usb_probe_device(struct devic + 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 devic + 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; diff --git a/queue-5.8/usbcore-driver-fix-incorrect-downcast.patch b/queue-5.8/usbcore-driver-fix-incorrect-downcast.patch new file mode 100644 index 00000000000..fa5275bcd7b --- /dev/null +++ b/queue-5.8/usbcore-driver-fix-incorrect-downcast.patch @@ -0,0 +1,72 @@ +From 4df30e7603432704380b12fe40a604ee7f66746d Mon Sep 17 00:00:00 2001 +From: "M. Vefa Bicakci" +Date: Tue, 22 Sep 2020 14:07:02 +0300 +Subject: usbcore/driver: Fix incorrect downcast + +From: M. Vefa Bicakci + +commit 4df30e7603432704380b12fe40a604ee7f66746d upstream. + +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: d5643d2249b2 ("USB: Fix device driver race") +Cc: # 5.8 +Cc: Greg Kroah-Hartman +Cc: Alan Stern +Cc: Bastien Nocera +Cc: Shuah Khan +Cc: Valentina Manea +Cc: +Tested-by: Andrey Konovalov +Signed-off-by: M. Vefa Bicakci +Link: https://lore.kernel.org/r/20200922110703.720960-4-m.v.b@runbox.com +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/usb/core/driver.c | 11 ++--------- + 1 file changed, 2 insertions(+), 9 deletions(-) + +--- a/drivers/usb/core/driver.c ++++ b/drivers/usb/core/driver.c +@@ -905,21 +905,14 @@ static int usb_uevent(struct device *dev + 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); diff --git a/queue-5.8/usbcore-driver-fix-specific-driver-selection.patch b/queue-5.8/usbcore-driver-fix-specific-driver-selection.patch new file mode 100644 index 00000000000..1ce97e0c373 --- /dev/null +++ b/queue-5.8/usbcore-driver-fix-specific-driver-selection.patch @@ -0,0 +1,67 @@ +From aea850cd35ae3d266fe6f93fb9edb25e4a555230 Mon Sep 17 00:00:00 2001 +From: "M. Vefa Bicakci" +Date: Tue, 22 Sep 2020 14:07:01 +0300 +Subject: usbcore/driver: Fix specific driver selection + +From: M. Vefa Bicakci + +commit aea850cd35ae3d266fe6f93fb9edb25e4a555230 upstream. + +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: d5643d2249b2 ("USB: Fix device driver race") +Cc: # 5.8 +Cc: Greg Kroah-Hartman +Cc: Alan Stern +Cc: Bastien Nocera +Cc: Shuah Khan +Cc: Valentina Manea +Cc: +Tested-by: Andrey Konovalov +Signed-off-by: M. Vefa Bicakci +Link: https://lore.kernel.org/r/20200922110703.720960-3-m.v.b@runbox.com +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/usb/core/driver.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +--- a/drivers/usb/core/driver.c ++++ b/drivers/usb/core/driver.c +@@ -924,7 +924,7 @@ static int __usb_bus_reprobe_drivers(str + + 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);