From: Johan Hovold <johan@kernel.org> Date: 2021-10-27 08:12:23
This series adds the missing endpoint sanity checks to the wireless USB
drivers that were doing packet-size calculations without first making
sure that the USB descriptors were sane.
Johan
Changes in v2:
- tighten the sanity checks for the mwifiex firmware-download mode to
handle also a missing bulk-out endpoint (noticed by Brian Norris)
Johan Hovold (3):
ath10k: fix division by zero in send path
ath6kl: fix division by zero in send path
mwifiex: fix division by zero in fw download path
drivers/net/wireless/ath/ath10k/usb.c | 5 +++++
drivers/net/wireless/ath/ath6kl/usb.c | 5 +++++
drivers/net/wireless/marvell/mwifiex/usb.c | 16 ++++++++++++++++
3 files changed, 26 insertions(+)
--
2.32.0
From: Johan Hovold <johan@kernel.org> Date: 2021-10-27 08:12:23
Add the missing endpoint max-packet sanity check to probe() to avoid
division by zero in ath10k_usb_hif_tx_sg() in case a malicious device
has broken descriptors (or when doing descriptor fuzz testing).
Note that USB core will reject URBs submitted for endpoints with zero
wMaxPacketSize but that drivers doing packet-size calculations still
need to handle this (cf. commit 2548288b4fb0 ("USB: Fix: Don't skip
endpoint descriptors with maxpacket=0")).
Fixes: 9cbee358687e ("ath6kl: add full USB support")
Cc: stable@vger.kernel.org # 3.5
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/wireless/ath/ath6kl/usb.c | 5 +++++
1 file changed, 5 insertions(+)
From: Johan Hovold <johan@kernel.org> Date: 2021-10-27 08:12:32
Add the missing endpoint max-packet sanity check to probe() to avoid
division by zero in ath10k_usb_hif_tx_sg() in case a malicious device
has broken descriptors (or when doing descriptor fuzz testing).
Note that USB core will reject URBs submitted for endpoints with zero
wMaxPacketSize but that drivers doing packet-size calculations still
need to handle this (cf. commit 2548288b4fb0 ("USB: Fix: Don't skip
endpoint descriptors with maxpacket=0")).
Fixes: 4db66499df91 ("ath10k: add initial USB support")
Cc: stable@vger.kernel.org # 4.14
Cc: Erik Stromdahl <redacted>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/wireless/ath/ath10k/usb.c | 5 +++++
1 file changed, 5 insertions(+)
From: Johan Hovold <johan@kernel.org> Date: 2021-10-27 08:12:33
Add the missing endpoint sanity checks to probe() to avoid division by
zero in mwifiex_write_data_sync() in case a malicious device has broken
descriptors (or when doing descriptor fuzz testing).
Only add checks for the firmware-download boot stage, which require both
command endpoints, for now. The driver looks like it will handle a
missing endpoint during normal operation without oopsing, albeit not
very gracefully as it will try to submit URBs to the default pipe and
fail.
Note that USB core will reject URBs submitted for endpoints with zero
wMaxPacketSize but that drivers doing packet-size calculations still
need to handle this (cf. commit 2548288b4fb0 ("USB: Fix: Don't skip
endpoint descriptors with maxpacket=0")).
Fixes: 4daffe354366 ("mwifiex: add support for Marvell USB8797 chipset")
Cc: stable@vger.kernel.org # 3.5
Cc: Amitkumar Karwar <redacted>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/wireless/marvell/mwifiex/usb.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
^^ These two conditions are applicable to USB8XXX_FW_READY too, right?
Right, but I didn't want to add an incomplete set of constraints.
I couldn't find any documentation (e.g. lsusb -v) for what the
descriptors are supposed to look like, but judging from the code,
something like
if (!card->rx_cmd_ep || !card->tx_cmd_ep)
return -ENODEV;
if (!card->rx_data_ep || !card->port[0].tx_data_ep)
return -ENODEV;
should do. But I'm not sure about the second tx endpoint,
card->port[1].tx_data_ep, for which support was added later and which
the driver appears to be able to manage without.
Either way it has nothing to do with the division-by-zero and should be
added separately.
quoted
+ if (card->bulk_out_maxpktsize == 0)
+ return -ENODEV;
+ break;
+ case USB8XXX_FW_READY:
+ /* Assume the driver can handle missing endpoints for now. */
+ break;
+ default:
+ WARN_ON(1);
+ return -ENODEV;
+ }
From: Kalle Valo <hidden> Date: 2021-10-28 07:34:43
Johan Hovold [off-list ref] wrote:
Add the missing endpoint max-packet sanity check to probe() to avoid
division by zero in ath10k_usb_hif_tx_sg() in case a malicious device
has broken descriptors (or when doing descriptor fuzz testing).
Note that USB core will reject URBs submitted for endpoints with zero
wMaxPacketSize but that drivers doing packet-size calculations still
need to handle this (cf. commit 2548288b4fb0 ("USB: Fix: Don't skip
endpoint descriptors with maxpacket=0")).
Fixes: 4db66499df91 ("ath10k: add initial USB support")
Cc: stable@vger.kernel.org # 4.14
Cc: Erik Stromdahl <redacted>
Signed-off-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Kalle Valo <redacted>
From: Kalle Valo <hidden> Date: 2021-10-28 13:28:41
Johan Hovold [off-list ref] wrote:
Add the missing endpoint sanity checks to probe() to avoid division by
zero in mwifiex_write_data_sync() in case a malicious device has broken
descriptors (or when doing descriptor fuzz testing).
Only add checks for the firmware-download boot stage, which require both
command endpoints, for now. The driver looks like it will handle a
missing endpoint during normal operation without oopsing, albeit not
very gracefully as it will try to submit URBs to the default pipe and
fail.
Note that USB core will reject URBs submitted for endpoints with zero
wMaxPacketSize but that drivers doing packet-size calculations still
need to handle this (cf. commit 2548288b4fb0 ("USB: Fix: Don't skip
endpoint descriptors with maxpacket=0")).
Fixes: 4daffe354366 ("mwifiex: add support for Marvell USB8797 chipset")
Cc: stable@vger.kernel.org # 3.5
Cc: Amitkumar Karwar <redacted>
Signed-off-by: Johan Hovold <johan@kernel.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>