* [PATCH v7] wifi: mwifiex: Fix OOB and integer underflow when rx packets
@ 2023-07-23 7:07 Polaris Pi
2023-07-26 12:05 ` Matthew Wang
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Polaris Pi @ 2023-07-23 7:07 UTC (permalink / raw)
To: matthewmwang, briannorris, kuba, kvalo; +Cc: linux-wireless, Polaris Pi
Make sure mwifiex_process_mgmt_packet,
mwifiex_process_sta_rx_packet and mwifiex_process_uap_rx_packet,
mwifiex_uap_queue_bridged_pkt and mwifiex_process_rx_packet
not out-of-bounds access the skb->data buffer.
Fixes: 2dbaf751b1de ("mwifiex: report received management frames to cfg80211")
Signed-off-by: Polaris Pi <pinkperfect2021@gmail.com>
---
V5: Follow chromeos comments: preserve the original flow of mwifiex_process_uap_rx_packet
V6: Simplify check in mwifiex_process_uap_rx_packet
V7: Fix drop packets issue when auotest V6, now pass manual and auto tests
---
drivers/net/wireless/marvell/mwifiex/sta_rx.c | 11 ++++++++++-
.../net/wireless/marvell/mwifiex/uap_txrx.c | 19 +++++++++++++++++++
drivers/net/wireless/marvell/mwifiex/util.c | 10 +++++++---
3 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_rx.c b/drivers/net/wireless/marvell/mwifiex/sta_rx.c
index 13659b02ba88..f2899d53a43f 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_rx.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_rx.c
@@ -86,6 +86,14 @@ int mwifiex_process_rx_packet(struct mwifiex_private *priv,
rx_pkt_len = le16_to_cpu(local_rx_pd->rx_pkt_length);
rx_pkt_hdr = (void *)local_rx_pd + rx_pkt_off;
+ if (sizeof(*rx_pkt_hdr) + rx_pkt_off > skb->len) {
+ mwifiex_dbg(priv->adapter, ERROR,
+ "wrong rx packet offset: len=%d, rx_pkt_off=%d\n",
+ skb->len, rx_pkt_off);
+ priv->stats.rx_dropped++;
+ dev_kfree_skb_any(skb);
+ }
+
if ((!memcmp(&rx_pkt_hdr->rfc1042_hdr, bridge_tunnel_header,
sizeof(bridge_tunnel_header))) ||
(!memcmp(&rx_pkt_hdr->rfc1042_hdr, rfc1042_header,
@@ -194,7 +202,8 @@ int mwifiex_process_sta_rx_packet(struct mwifiex_private *priv,
rx_pkt_hdr = (void *)local_rx_pd + rx_pkt_offset;
- if ((rx_pkt_offset + rx_pkt_length) > (u16) skb->len) {
+ if ((rx_pkt_offset + rx_pkt_length) > skb->len ||
+ sizeof(rx_pkt_hdr->eth803_hdr) + rx_pkt_offset > skb->len) {
mwifiex_dbg(adapter, ERROR,
"wrong rx packet: len=%d, rx_pkt_offset=%d, rx_pkt_length=%d\n",
skb->len, rx_pkt_offset, rx_pkt_length);
diff --git a/drivers/net/wireless/marvell/mwifiex/uap_txrx.c b/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
index e495f7eaea03..04ff051f5d18 100644
--- a/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
+++ b/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
@@ -103,6 +103,15 @@ static void mwifiex_uap_queue_bridged_pkt(struct mwifiex_private *priv,
return;
}
+ if (sizeof(*rx_pkt_hdr) +
+ le16_to_cpu(uap_rx_pd->rx_pkt_offset) > skb->len) {
+ mwifiex_dbg(adapter, ERROR,
+ "wrong rx packet offset: len=%d,rx_pkt_offset=%d\n",
+ skb->len, le16_to_cpu(uap_rx_pd->rx_pkt_offset));
+ priv->stats.rx_dropped++;
+ dev_kfree_skb_any(skb);
+ }
+
if ((!memcmp(&rx_pkt_hdr->rfc1042_hdr, bridge_tunnel_header,
sizeof(bridge_tunnel_header))) ||
(!memcmp(&rx_pkt_hdr->rfc1042_hdr, rfc1042_header,
@@ -367,6 +376,16 @@ int mwifiex_process_uap_rx_packet(struct mwifiex_private *priv,
rx_pkt_type = le16_to_cpu(uap_rx_pd->rx_pkt_type);
rx_pkt_hdr = (void *)uap_rx_pd + le16_to_cpu(uap_rx_pd->rx_pkt_offset);
+ if (le16_to_cpu(uap_rx_pd->rx_pkt_offset) +
+ sizeof(rx_pkt_hdr->eth803_hdr) > skb->len) {
+ mwifiex_dbg(adapter, ERROR,
+ "wrong rx packet for struct ethhdr: len=%d, offset=%d\n",
+ skb->len, le16_to_cpu(uap_rx_pd->rx_pkt_offset));
+ priv->stats.rx_dropped++;
+ dev_kfree_skb_any(skb);
+ return 0;
+ }
+
ether_addr_copy(ta, rx_pkt_hdr->eth803_hdr.h_source);
if ((le16_to_cpu(uap_rx_pd->rx_pkt_offset) +
diff --git a/drivers/net/wireless/marvell/mwifiex/util.c b/drivers/net/wireless/marvell/mwifiex/util.c
index 94c2d219835d..745b1d925b21 100644
--- a/drivers/net/wireless/marvell/mwifiex/util.c
+++ b/drivers/net/wireless/marvell/mwifiex/util.c
@@ -393,11 +393,15 @@ mwifiex_process_mgmt_packet(struct mwifiex_private *priv,
}
rx_pd = (struct rxpd *)skb->data;
+ pkt_len = le16_to_cpu(rx_pd->rx_pkt_length);
+ if (pkt_len < sizeof(struct ieee80211_hdr) + sizeof(pkt_len)) {
+ mwifiex_dbg(priv->adapter, ERROR, "invalid rx_pkt_length");
+ return -1;
+ }
skb_pull(skb, le16_to_cpu(rx_pd->rx_pkt_offset));
skb_pull(skb, sizeof(pkt_len));
-
- pkt_len = le16_to_cpu(rx_pd->rx_pkt_length);
+ pkt_len -= sizeof(pkt_len);
ieee_hdr = (void *)skb->data;
if (ieee80211_is_mgmt(ieee_hdr->frame_control)) {
@@ -410,7 +414,7 @@ mwifiex_process_mgmt_packet(struct mwifiex_private *priv,
skb->data + sizeof(struct ieee80211_hdr),
pkt_len - sizeof(struct ieee80211_hdr));
- pkt_len -= ETH_ALEN + sizeof(pkt_len);
+ pkt_len -= ETH_ALEN;
rx_pd->rx_pkt_length = cpu_to_le16(pkt_len);
cfg80211_rx_mgmt(&priv->wdev, priv->roc_cfg.chan.center_freq,
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v7] wifi: mwifiex: Fix OOB and integer underflow when rx packets
2023-07-23 7:07 [PATCH v7] wifi: mwifiex: Fix OOB and integer underflow when rx packets Polaris Pi
@ 2023-07-26 12:05 ` Matthew Wang
2023-07-26 21:23 ` Brian Norris
2023-08-01 14:47 ` Kalle Valo
2 siblings, 0 replies; 6+ messages in thread
From: Matthew Wang @ 2023-07-26 12:05 UTC (permalink / raw)
To: Polaris Pi; +Cc: briannorris, kuba, kvalo, linux-wireless
Reviewed-by: Matthew Wang <matthewmwang@chromium.org>
On Sun, Jul 23, 2023 at 9:07 AM Polaris Pi <pinkperfect2021@gmail.com> wrote:
>
> Make sure mwifiex_process_mgmt_packet,
> mwifiex_process_sta_rx_packet and mwifiex_process_uap_rx_packet,
> mwifiex_uap_queue_bridged_pkt and mwifiex_process_rx_packet
> not out-of-bounds access the skb->data buffer.
>
> Fixes: 2dbaf751b1de ("mwifiex: report received management frames to cfg80211")
> Signed-off-by: Polaris Pi <pinkperfect2021@gmail.com>
> ---
> V5: Follow chromeos comments: preserve the original flow of mwifiex_process_uap_rx_packet
> V6: Simplify check in mwifiex_process_uap_rx_packet
> V7: Fix drop packets issue when auotest V6, now pass manual and auto tests
> ---
> drivers/net/wireless/marvell/mwifiex/sta_rx.c | 11 ++++++++++-
> .../net/wireless/marvell/mwifiex/uap_txrx.c | 19 +++++++++++++++++++
> drivers/net/wireless/marvell/mwifiex/util.c | 10 +++++++---
> 3 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_rx.c b/drivers/net/wireless/marvell/mwifiex/sta_rx.c
> index 13659b02ba88..f2899d53a43f 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_rx.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_rx.c
> @@ -86,6 +86,14 @@ int mwifiex_process_rx_packet(struct mwifiex_private *priv,
> rx_pkt_len = le16_to_cpu(local_rx_pd->rx_pkt_length);
> rx_pkt_hdr = (void *)local_rx_pd + rx_pkt_off;
>
> + if (sizeof(*rx_pkt_hdr) + rx_pkt_off > skb->len) {
> + mwifiex_dbg(priv->adapter, ERROR,
> + "wrong rx packet offset: len=%d, rx_pkt_off=%d\n",
> + skb->len, rx_pkt_off);
> + priv->stats.rx_dropped++;
> + dev_kfree_skb_any(skb);
> + }
> +
> if ((!memcmp(&rx_pkt_hdr->rfc1042_hdr, bridge_tunnel_header,
> sizeof(bridge_tunnel_header))) ||
> (!memcmp(&rx_pkt_hdr->rfc1042_hdr, rfc1042_header,
> @@ -194,7 +202,8 @@ int mwifiex_process_sta_rx_packet(struct mwifiex_private *priv,
>
> rx_pkt_hdr = (void *)local_rx_pd + rx_pkt_offset;
>
> - if ((rx_pkt_offset + rx_pkt_length) > (u16) skb->len) {
> + if ((rx_pkt_offset + rx_pkt_length) > skb->len ||
> + sizeof(rx_pkt_hdr->eth803_hdr) + rx_pkt_offset > skb->len) {
> mwifiex_dbg(adapter, ERROR,
> "wrong rx packet: len=%d, rx_pkt_offset=%d, rx_pkt_length=%d\n",
> skb->len, rx_pkt_offset, rx_pkt_length);
> diff --git a/drivers/net/wireless/marvell/mwifiex/uap_txrx.c b/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
> index e495f7eaea03..04ff051f5d18 100644
> --- a/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
> +++ b/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
> @@ -103,6 +103,15 @@ static void mwifiex_uap_queue_bridged_pkt(struct mwifiex_private *priv,
> return;
> }
>
> + if (sizeof(*rx_pkt_hdr) +
> + le16_to_cpu(uap_rx_pd->rx_pkt_offset) > skb->len) {
> + mwifiex_dbg(adapter, ERROR,
> + "wrong rx packet offset: len=%d,rx_pkt_offset=%d\n",
> + skb->len, le16_to_cpu(uap_rx_pd->rx_pkt_offset));
> + priv->stats.rx_dropped++;
> + dev_kfree_skb_any(skb);
> + }
> +
> if ((!memcmp(&rx_pkt_hdr->rfc1042_hdr, bridge_tunnel_header,
> sizeof(bridge_tunnel_header))) ||
> (!memcmp(&rx_pkt_hdr->rfc1042_hdr, rfc1042_header,
> @@ -367,6 +376,16 @@ int mwifiex_process_uap_rx_packet(struct mwifiex_private *priv,
> rx_pkt_type = le16_to_cpu(uap_rx_pd->rx_pkt_type);
> rx_pkt_hdr = (void *)uap_rx_pd + le16_to_cpu(uap_rx_pd->rx_pkt_offset);
>
> + if (le16_to_cpu(uap_rx_pd->rx_pkt_offset) +
> + sizeof(rx_pkt_hdr->eth803_hdr) > skb->len) {
> + mwifiex_dbg(adapter, ERROR,
> + "wrong rx packet for struct ethhdr: len=%d, offset=%d\n",
> + skb->len, le16_to_cpu(uap_rx_pd->rx_pkt_offset));
> + priv->stats.rx_dropped++;
> + dev_kfree_skb_any(skb);
> + return 0;
> + }
> +
> ether_addr_copy(ta, rx_pkt_hdr->eth803_hdr.h_source);
>
> if ((le16_to_cpu(uap_rx_pd->rx_pkt_offset) +
> diff --git a/drivers/net/wireless/marvell/mwifiex/util.c b/drivers/net/wireless/marvell/mwifiex/util.c
> index 94c2d219835d..745b1d925b21 100644
> --- a/drivers/net/wireless/marvell/mwifiex/util.c
> +++ b/drivers/net/wireless/marvell/mwifiex/util.c
> @@ -393,11 +393,15 @@ mwifiex_process_mgmt_packet(struct mwifiex_private *priv,
> }
>
> rx_pd = (struct rxpd *)skb->data;
> + pkt_len = le16_to_cpu(rx_pd->rx_pkt_length);
> + if (pkt_len < sizeof(struct ieee80211_hdr) + sizeof(pkt_len)) {
> + mwifiex_dbg(priv->adapter, ERROR, "invalid rx_pkt_length");
> + return -1;
> + }
>
> skb_pull(skb, le16_to_cpu(rx_pd->rx_pkt_offset));
> skb_pull(skb, sizeof(pkt_len));
> -
> - pkt_len = le16_to_cpu(rx_pd->rx_pkt_length);
> + pkt_len -= sizeof(pkt_len);
>
> ieee_hdr = (void *)skb->data;
> if (ieee80211_is_mgmt(ieee_hdr->frame_control)) {
> @@ -410,7 +414,7 @@ mwifiex_process_mgmt_packet(struct mwifiex_private *priv,
> skb->data + sizeof(struct ieee80211_hdr),
> pkt_len - sizeof(struct ieee80211_hdr));
>
> - pkt_len -= ETH_ALEN + sizeof(pkt_len);
> + pkt_len -= ETH_ALEN;
> rx_pd->rx_pkt_length = cpu_to_le16(pkt_len);
>
> cfg80211_rx_mgmt(&priv->wdev, priv->roc_cfg.chan.center_freq,
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7] wifi: mwifiex: Fix OOB and integer underflow when rx packets
2023-07-23 7:07 [PATCH v7] wifi: mwifiex: Fix OOB and integer underflow when rx packets Polaris Pi
2023-07-26 12:05 ` Matthew Wang
@ 2023-07-26 21:23 ` Brian Norris
2023-07-27 6:10 ` Kalle Valo
2023-08-01 14:47 ` Kalle Valo
2 siblings, 1 reply; 6+ messages in thread
From: Brian Norris @ 2023-07-26 21:23 UTC (permalink / raw)
To: Polaris Pi; +Cc: matthewmwang, kuba, kvalo, linux-wireless
On Sun, Jul 23, 2023 at 07:07:41AM +0000, Polaris Pi wrote:
> Make sure mwifiex_process_mgmt_packet,
> mwifiex_process_sta_rx_packet and mwifiex_process_uap_rx_packet,
> mwifiex_uap_queue_bridged_pkt and mwifiex_process_rx_packet
> not out-of-bounds access the skb->data buffer.
>
> Fixes: 2dbaf751b1de ("mwifiex: report received management frames to cfg80211")
> Signed-off-by: Polaris Pi <pinkperfect2021@gmail.com>
> ---
> V5: Follow chromeos comments: preserve the original flow of mwifiex_process_uap_rx_packet
> V6: Simplify check in mwifiex_process_uap_rx_packet
> V7: Fix drop packets issue when auotest V6, now pass manual and auto tests
"auto tests" isn't clear to anyone not familiar with Chromium stuff.
It'd be courteous to at least make an attempt to describe what this
means (even just, "ChromeOS WiFi test suite" or something). For the
record, I believe that's approximately this?
https://chromium.googlesource.com/chromiumos/third_party/autotest/+/HEAD/docs/wificell.md
Anyway, I think the patch contents look good:
Reviewed-by: Brian Norris <briannorris@chromium.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7] wifi: mwifiex: Fix OOB and integer underflow when rx packets
2023-07-26 21:23 ` Brian Norris
@ 2023-07-27 6:10 ` Kalle Valo
2023-07-27 16:13 ` Brian Norris
0 siblings, 1 reply; 6+ messages in thread
From: Kalle Valo @ 2023-07-27 6:10 UTC (permalink / raw)
To: Brian Norris; +Cc: Polaris Pi, matthewmwang, kuba, linux-wireless
Brian Norris <briannorris@chromium.org> writes:
> On Sun, Jul 23, 2023 at 07:07:41AM +0000, Polaris Pi wrote:
>> Make sure mwifiex_process_mgmt_packet,
>> mwifiex_process_sta_rx_packet and mwifiex_process_uap_rx_packet,
>> mwifiex_uap_queue_bridged_pkt and mwifiex_process_rx_packet
>> not out-of-bounds access the skb->data buffer.
>>
>> Fixes: 2dbaf751b1de ("mwifiex: report received management frames to cfg80211")
>> Signed-off-by: Polaris Pi <pinkperfect2021@gmail.com>
>> ---
>> V5: Follow chromeos comments: preserve the original flow of mwifiex_process_uap_rx_packet
>> V6: Simplify check in mwifiex_process_uap_rx_packet
>> V7: Fix drop packets issue when auotest V6, now pass manual and auto tests
>
> "auto tests" isn't clear to anyone not familiar with Chromium stuff.
> It'd be courteous to at least make an attempt to describe what this
> means (even just, "ChromeOS WiFi test suite" or something). For the
> record, I believe that's approximately this?
>
> https://chromium.googlesource.com/chromiumos/third_party/autotest/+/HEAD/docs/wificell.md
>
> Anyway, I think the patch contents look good:
>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
I'm nitpicking but now that you (Brian) are a maintainer I would prefer
that you use Acked-by instead of Reviewed-by. Patchwork shows the
statistics (A/R/T in the web ui) and then it's easy for me to see that
the patch is ready to be applied. This is for the future, no need to
change anything here.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7] wifi: mwifiex: Fix OOB and integer underflow when rx packets
2023-07-27 6:10 ` Kalle Valo
@ 2023-07-27 16:13 ` Brian Norris
0 siblings, 0 replies; 6+ messages in thread
From: Brian Norris @ 2023-07-27 16:13 UTC (permalink / raw)
To: Kalle Valo; +Cc: Polaris Pi, matthewmwang, kuba, linux-wireless
On Wed, Jul 26, 2023 at 11:10 PM Kalle Valo <kvalo@kernel.org> wrote:
>
> Brian Norris <briannorris@chromium.org> writes:
> > Reviewed-by: Brian Norris <briannorris@chromium.org>
>
> I'm nitpicking but now that you (Brian) are a maintainer I would prefer
> that you use Acked-by instead of Reviewed-by. Patchwork shows the
> statistics (A/R/T in the web ui) and then it's easy for me to see that
> the patch is ready to be applied. This is for the future, no need to
> change anything here.
Argh, I knew that's the recommendation, and I thought I did that here,
but obviously not. Thanks for the reminder. I'm sure I'll fix my
muscle memory eventually :)
Brian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7] wifi: mwifiex: Fix OOB and integer underflow when rx packets
2023-07-23 7:07 [PATCH v7] wifi: mwifiex: Fix OOB and integer underflow when rx packets Polaris Pi
2023-07-26 12:05 ` Matthew Wang
2023-07-26 21:23 ` Brian Norris
@ 2023-08-01 14:47 ` Kalle Valo
2 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2023-08-01 14:47 UTC (permalink / raw)
To: Polaris Pi; +Cc: matthewmwang, briannorris, kuba, linux-wireless, Polaris Pi
Polaris Pi <pinkperfect2021@gmail.com> wrote:
> Make sure mwifiex_process_mgmt_packet,
> mwifiex_process_sta_rx_packet and mwifiex_process_uap_rx_packet,
> mwifiex_uap_queue_bridged_pkt and mwifiex_process_rx_packet
> not out-of-bounds access the skb->data buffer.
>
> Fixes: 2dbaf751b1de ("mwifiex: report received management frames to cfg80211")
> Signed-off-by: Polaris Pi <pinkperfect2021@gmail.com>
> Reviewed-by: Matthew Wang <matthewmwang@chromium.org>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
Patch applied to wireless-next.git, thanks.
119585281617 wifi: mwifiex: Fix OOB and integer underflow when rx packets
--
https://patchwork.kernel.org/project/linux-wireless/patch/20230723070741.1544662-1-pinkperfect2021@gmail.com/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-01 14:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-23 7:07 [PATCH v7] wifi: mwifiex: Fix OOB and integer underflow when rx packets Polaris Pi
2023-07-26 12:05 ` Matthew Wang
2023-07-26 21:23 ` Brian Norris
2023-07-27 6:10 ` Kalle Valo
2023-07-27 16:13 ` Brian Norris
2023-08-01 14:47 ` Kalle Valo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).