* [PATCH v6] wifi: mwifiex: Fix OOB and integer underflow when rx packets
@ 2023-07-14 22:48 Polaris Pi
2023-07-15 20:22 ` Matthew Wang
2023-07-17 14:02 ` Polaris Pi
0 siblings, 2 replies; 5+ messages in thread
From: Polaris Pi @ 2023-07-14 22:48 UTC (permalink / raw)
To: kuba, matthewmwang, kuabhs, amitkarwar, kvalo, ganapathi017,
sharvari.harisangam, huxinming820
Cc: linux-wireless, linux-kernel, Polaris Pi
Make sure mwifiex_process_mgmt_packet and its callers
mwifiex_process_sta_rx_packet and mwifiex_process_uap_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
---
drivers/net/wireless/marvell/mwifiex/sta_rx.c | 3 ++-
drivers/net/wireless/marvell/mwifiex/uap_txrx.c | 10 ++++++++++
drivers/net/wireless/marvell/mwifiex/util.c | 5 +++++
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_rx.c b/drivers/net/wireless/marvell/mwifiex/sta_rx.c
index 13659b02ba88..88aaec645291 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_rx.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_rx.c
@@ -194,7 +194,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) > (u16)skb->len ||
+ skb->len - rx_pkt_offset < sizeof(*rx_pkt_hdr)) {
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..f0711b73ba3e 100644
--- a/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
+++ b/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
@@ -367,6 +367,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) > skb->len) {
+ mwifiex_dbg(adapter, ERROR,
+ "wrong rx packet offset: 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..31e1a82883e4 100644
--- a/drivers/net/wireless/marvell/mwifiex/util.c
+++ b/drivers/net/wireless/marvell/mwifiex/util.c
@@ -399,6 +399,11 @@ mwifiex_process_mgmt_packet(struct mwifiex_private *priv,
pkt_len = le16_to_cpu(rx_pd->rx_pkt_length);
+ if (pkt_len < sizeof(struct ieee80211_hdr) || skb->len < pkt_len) {
+ mwifiex_dbg(priv->adapter, ERROR, "invalid rx_pkt_length");
+ return -1;
+ }
+
ieee_hdr = (void *)skb->data;
if (ieee80211_is_mgmt(ieee_hdr->frame_control)) {
if (mwifiex_parse_mgmt_packet(priv, (u8 *)ieee_hdr,
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v6] wifi: mwifiex: Fix OOB and integer underflow when rx packets
2023-07-14 22:48 [PATCH v6] wifi: mwifiex: Fix OOB and integer underflow when rx packets Polaris Pi
@ 2023-07-15 20:22 ` Matthew Wang
2023-07-17 14:02 ` Polaris Pi
1 sibling, 0 replies; 5+ messages in thread
From: Matthew Wang @ 2023-07-15 20:22 UTC (permalink / raw)
To: Polaris Pi
Cc: kuba, kuabhs, amitkarwar, kvalo, ganapathi017,
sharvari.harisangam, huxinming820, linux-wireless, linux-kernel
Reviewed-by: Matthew Wang <matthewmwang@chromium.org>
On Sat, Jul 15, 2023 at 12:48 AM Polaris Pi <pinkperfect2021@gmail.com> wrote:
>
> Make sure mwifiex_process_mgmt_packet and its callers
> mwifiex_process_sta_rx_packet and mwifiex_process_uap_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
> ---
> drivers/net/wireless/marvell/mwifiex/sta_rx.c | 3 ++-
> drivers/net/wireless/marvell/mwifiex/uap_txrx.c | 10 ++++++++++
> drivers/net/wireless/marvell/mwifiex/util.c | 5 +++++
> 3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_rx.c b/drivers/net/wireless/marvell/mwifiex/sta_rx.c
> index 13659b02ba88..88aaec645291 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_rx.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_rx.c
> @@ -194,7 +194,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) > (u16)skb->len ||
> + skb->len - rx_pkt_offset < sizeof(*rx_pkt_hdr)) {
> 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..f0711b73ba3e 100644
> --- a/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
> +++ b/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
> @@ -367,6 +367,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) > skb->len) {
> + mwifiex_dbg(adapter, ERROR,
> + "wrong rx packet offset: 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..31e1a82883e4 100644
> --- a/drivers/net/wireless/marvell/mwifiex/util.c
> +++ b/drivers/net/wireless/marvell/mwifiex/util.c
> @@ -399,6 +399,11 @@ mwifiex_process_mgmt_packet(struct mwifiex_private *priv,
>
> pkt_len = le16_to_cpu(rx_pd->rx_pkt_length);
>
> + if (pkt_len < sizeof(struct ieee80211_hdr) || skb->len < pkt_len) {
> + mwifiex_dbg(priv->adapter, ERROR, "invalid rx_pkt_length");
> + return -1;
> + }
> +
> ieee_hdr = (void *)skb->data;
> if (ieee80211_is_mgmt(ieee_hdr->frame_control)) {
> if (mwifiex_parse_mgmt_packet(priv, (u8 *)ieee_hdr,
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v6] wifi: mwifiex: Fix OOB and integer underflow when rx packets
2023-07-14 22:48 [PATCH v6] wifi: mwifiex: Fix OOB and integer underflow when rx packets Polaris Pi
2023-07-15 20:22 ` Matthew Wang
@ 2023-07-17 14:02 ` Polaris Pi
2023-07-17 21:39 ` Jakub Kicinski
1 sibling, 1 reply; 5+ messages in thread
From: Polaris Pi @ 2023-07-17 14:02 UTC (permalink / raw)
To: kuba; +Cc: pinkperfect2021, kvalo, linux-wireless
Hi Jakub,
This patch has been reviewed by chromeos wifi team, and review tag has been added by their tech leader.
Could you please review it again?
Thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v6] wifi: mwifiex: Fix OOB and integer underflow when rx packets
2023-07-17 14:02 ` Polaris Pi
@ 2023-07-17 21:39 ` Jakub Kicinski
2023-07-20 21:39 ` Matthew Wang
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2023-07-17 21:39 UTC (permalink / raw)
To: Polaris Pi; +Cc: kvalo, linux-wireless
On Mon, 17 Jul 2023 14:02:32 +0000 Polaris Pi wrote:
> Hi Jakub,
>
> This patch has been reviewed by chromeos wifi team, and review tag
> has been added by their tech leader.
> Could you please review it again?
Looks fine, I'll leave it to the wireless maintainer to process
once they are back from their vacation. It doesn't look very urgent
to me.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v6] wifi: mwifiex: Fix OOB and integer underflow when rx packets
2023-07-17 21:39 ` Jakub Kicinski
@ 2023-07-20 21:39 ` Matthew Wang
0 siblings, 0 replies; 5+ messages in thread
From: Matthew Wang @ 2023-07-20 21:39 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Polaris Pi, kvalo, linux-wireless
> diff --git a/drivers/net/wireless/marvell/mwifiex/util.c b/drivers/net/wireless/marvell/mwifiex/util.c
> index 94c2d219835d..31e1a82883e4 100644
> --- a/drivers/net/wireless/marvell/mwifiex/util.c
> +++ b/drivers/net/wireless/marvell/mwifiex/util.c
> @@ -399,6 +399,11 @@ mwifiex_process_mgmt_packet(struct mwifiex_private *priv,
>
> pkt_len = le16_to_cpu(rx_pd->rx_pkt_length);
>
> + if (pkt_len < sizeof(struct ieee80211_hdr) || skb->len < pkt_len) {
I've tested this patch a bit on a ChromeOS device and I've noticed
empirically that skb->len is often (always?) two less than pkt_len,
implying that pkt_len actually includes the rx_pkt_length field as
well (note that pkt_len gets adjusted by ETH_ALEN + sizeof(pkt_len)
below), so we end up hitting this condition reliably in certain
situations. This probably means the memmove below is not entirely
correct, but either way I don't think this patch is correct on its
own.
Consider my Reviewed-by tag removed until this gets resolved.
> + mwifiex_dbg(priv->adapter, ERROR, "invalid rx_pkt_length");
> + return -1;
> + }
> +
> ieee_hdr = (void *)skb->data;
> if (ieee80211_is_mgmt(ieee_hdr->frame_control)) {
> if (mwifiex_parse_mgmt_packet(priv, (u8 *)ieee_hdr,
> --
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-07-20 21:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-14 22:48 [PATCH v6] wifi: mwifiex: Fix OOB and integer underflow when rx packets Polaris Pi
2023-07-15 20:22 ` Matthew Wang
2023-07-17 14:02 ` Polaris Pi
2023-07-17 21:39 ` Jakub Kicinski
2023-07-20 21:39 ` Matthew Wang
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).