* [PATCH 1/3] ath10k: fix division by zero in send path
@ 2021-10-26 9:52 Johan Hovold
2021-10-26 9:52 ` [PATCH 2/3] ath6kl: " Johan Hovold
2021-10-26 9:52 ` [PATCH 3/3] mwifiex: fix division by zero in fw download path Johan Hovold
0 siblings, 2 replies; 6+ messages in thread
From: Johan Hovold @ 2021-10-26 9:52 UTC (permalink / raw)
To: Kalle Valo
Cc: Amitkumar Karwar, Ganapathi Bhat, Sharvari Harisangam,
Xinming Hu, linux-wireless, netdev, linux-usb, linux-kernel,
Johan Hovold, stable, Erik Stromdahl
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 <erik.stromdahl@gmail.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/wireless/ath/ath10k/usb.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/usb.c b/drivers/net/wireless/ath/ath10k/usb.c
index 6d831b098cbb..3d98f19c6ec8 100644
--- a/drivers/net/wireless/ath/ath10k/usb.c
+++ b/drivers/net/wireless/ath/ath10k/usb.c
@@ -853,6 +853,11 @@ static int ath10k_usb_setup_pipe_resources(struct ath10k *ar,
le16_to_cpu(endpoint->wMaxPacketSize),
endpoint->bInterval);
}
+
+ /* Ignore broken descriptors. */
+ if (usb_endpoint_maxp(endpoint) == 0)
+ continue;
+
urbcount = 0;
pipe_num =
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] ath6kl: fix division by zero in send path
2021-10-26 9:52 [PATCH 1/3] ath10k: fix division by zero in send path Johan Hovold
@ 2021-10-26 9:52 ` Johan Hovold
2021-10-26 9:52 ` [PATCH 3/3] mwifiex: fix division by zero in fw download path Johan Hovold
1 sibling, 0 replies; 6+ messages in thread
From: Johan Hovold @ 2021-10-26 9:52 UTC (permalink / raw)
To: Kalle Valo
Cc: Amitkumar Karwar, Ganapathi Bhat, Sharvari Harisangam,
Xinming Hu, linux-wireless, netdev, linux-usb, linux-kernel,
Johan Hovold, stable
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(+)
diff --git a/drivers/net/wireless/ath/ath6kl/usb.c b/drivers/net/wireless/ath/ath6kl/usb.c
index bd367b79a4d3..aba70f35e574 100644
--- a/drivers/net/wireless/ath/ath6kl/usb.c
+++ b/drivers/net/wireless/ath/ath6kl/usb.c
@@ -340,6 +340,11 @@ static int ath6kl_usb_setup_pipe_resources(struct ath6kl_usb *ar_usb)
le16_to_cpu(endpoint->wMaxPacketSize),
endpoint->bInterval);
}
+
+ /* Ignore broken descriptors. */
+ if (usb_endpoint_maxp(endpoint) == 0)
+ continue;
+
urbcount = 0;
pipe_num =
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] mwifiex: fix division by zero in fw download path
2021-10-26 9:52 [PATCH 1/3] ath10k: fix division by zero in send path Johan Hovold
2021-10-26 9:52 ` [PATCH 2/3] ath6kl: " Johan Hovold
@ 2021-10-26 9:52 ` Johan Hovold
2021-10-26 17:35 ` Brian Norris
1 sibling, 1 reply; 6+ messages in thread
From: Johan Hovold @ 2021-10-26 9:52 UTC (permalink / raw)
To: Kalle Valo
Cc: Amitkumar Karwar, Ganapathi Bhat, Sharvari Harisangam,
Xinming Hu, linux-wireless, netdev, linux-usb, linux-kernel,
Johan Hovold, stable, Amitkumar Karwar
Add the missing endpoint max-packet sanity check 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).
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 <akarwar@marvell.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/wireless/marvell/mwifiex/usb.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 426e39d4ccf0..2826654907d9 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -502,6 +502,9 @@ static int mwifiex_usb_probe(struct usb_interface *intf,
atomic_set(&card->tx_cmd_urb_pending, 0);
card->bulk_out_maxpktsize =
le16_to_cpu(epd->wMaxPacketSize);
+ /* Reject broken descriptors. */
+ if (card->bulk_out_maxpktsize == 0)
+ return -ENODEV;
}
}
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] mwifiex: fix division by zero in fw download path
2021-10-26 9:52 ` [PATCH 3/3] mwifiex: fix division by zero in fw download path Johan Hovold
@ 2021-10-26 17:35 ` Brian Norris
2021-10-27 7:40 ` Johan Hovold
0 siblings, 1 reply; 6+ messages in thread
From: Brian Norris @ 2021-10-26 17:35 UTC (permalink / raw)
To: Johan Hovold
Cc: Kalle Valo, Amitkumar Karwar, Ganapathi Bhat,
Sharvari Harisangam, Xinming Hu, linux-wireless, netdev,
linux-usb, linux-kernel, stable, Amitkumar Karwar
On Tue, Oct 26, 2021 at 2:53 AM Johan Hovold <johan@kernel.org> wrote:
>
> Add the missing endpoint max-packet sanity check 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).
>
> 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 <akarwar@marvell.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
Seems like you're missing a changelog and a version number, since
you've already sent previous versions of this patch.
> drivers/net/wireless/marvell/mwifiex/usb.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
> index 426e39d4ccf0..2826654907d9 100644
> --- a/drivers/net/wireless/marvell/mwifiex/usb.c
> +++ b/drivers/net/wireless/marvell/mwifiex/usb.c
> @@ -502,6 +502,9 @@ static int mwifiex_usb_probe(struct usb_interface *intf,
> atomic_set(&card->tx_cmd_urb_pending, 0);
> card->bulk_out_maxpktsize =
> le16_to_cpu(epd->wMaxPacketSize);
> + /* Reject broken descriptors. */
> + if (card->bulk_out_maxpktsize == 0)
> + return -ENODEV;
If we're really talking about malicious devices, I'm still not 100%
sure this is sufficient -- what if the device doesn't advertise the
right endpoints? Might we get through the surrounding loop without
ever even reaching this code? Seems like the right thing to do would
be to pull the validation outside the loop.
Brian
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] mwifiex: fix division by zero in fw download path
2021-10-26 17:35 ` Brian Norris
@ 2021-10-27 7:40 ` Johan Hovold
2021-10-27 18:23 ` Brian Norris
0 siblings, 1 reply; 6+ messages in thread
From: Johan Hovold @ 2021-10-27 7:40 UTC (permalink / raw)
To: Brian Norris
Cc: Kalle Valo, Amitkumar Karwar, Ganapathi Bhat,
Sharvari Harisangam, Xinming Hu, linux-wireless, netdev,
linux-usb, linux-kernel, stable, Amitkumar Karwar
On Tue, Oct 26, 2021 at 10:35:37AM -0700, Brian Norris wrote:
> On Tue, Oct 26, 2021 at 2:53 AM Johan Hovold <johan@kernel.org> wrote:
> >
> > Add the missing endpoint max-packet sanity check 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).
> >
> > 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 <akarwar@marvell.com>
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > ---
>
> Seems like you're missing a changelog and a version number, since
> you've already sent previous versions of this patch.
Seems like you're confusing me with someone else.
> > drivers/net/wireless/marvell/mwifiex/usb.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
> > index 426e39d4ccf0..2826654907d9 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/usb.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/usb.c
> > @@ -502,6 +502,9 @@ static int mwifiex_usb_probe(struct usb_interface *intf,
> > atomic_set(&card->tx_cmd_urb_pending, 0);
> > card->bulk_out_maxpktsize =
> > le16_to_cpu(epd->wMaxPacketSize);
> > + /* Reject broken descriptors. */
> > + if (card->bulk_out_maxpktsize == 0)
> > + return -ENODEV;
>
> If we're really talking about malicious devices, I'm still not 100%
> sure this is sufficient -- what if the device doesn't advertise the
> right endpoints? Might we get through the surrounding loop without
> ever even reaching this code? Seems like the right thing to do would
> be to pull the validation outside the loop.
But you're right about this. The driver looks up its resources but still
proceeds if they're not there (and will eventually try to submit URBs
for the default pipe).
I'll send a v2.
Johan
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] mwifiex: fix division by zero in fw download path
2021-10-27 7:40 ` Johan Hovold
@ 2021-10-27 18:23 ` Brian Norris
0 siblings, 0 replies; 6+ messages in thread
From: Brian Norris @ 2021-10-27 18:23 UTC (permalink / raw)
To: Johan Hovold
Cc: Kalle Valo, Amitkumar Karwar, Ganapathi Bhat,
Sharvari Harisangam, Xinming Hu, linux-wireless, netdev,
linux-usb, linux-kernel, stable, Amitkumar Karwar
On Wed, Oct 27, 2021 at 12:40 AM Johan Hovold <johan@kernel.org> wrote:
> On Tue, Oct 26, 2021 at 10:35:37AM -0700, Brian Norris wrote:
> > Seems like you're missing a changelog and a version number, since
> > you've already sent previous versions of this patch.
>
> Seems like you're confusing me with someone else.
Oops, you're correct :( It was only a week or two ago someone else was
trying to patch this, and I didn't remember the "From" correctly.
Sorry!
> I'll send a v2.
Looks better, thanks.
Brian
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-27 18:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-26 9:52 [PATCH 1/3] ath10k: fix division by zero in send path Johan Hovold
2021-10-26 9:52 ` [PATCH 2/3] ath6kl: " Johan Hovold
2021-10-26 9:52 ` [PATCH 3/3] mwifiex: fix division by zero in fw download path Johan Hovold
2021-10-26 17:35 ` Brian Norris
2021-10-27 7:40 ` Johan Hovold
2021-10-27 18:23 ` Brian Norris
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.