| Message ID | 20230906102940.1120269-1-treapking@chromium.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: mwifiex: Fix oob check condition in mwifiex_process_rx_packet | expand |
> - if ((!memcmp(&rx_pkt_hdr->rfc1042_hdr, bridge_tunnel_header, > - sizeof(bridge_tunnel_header))) || > - (!memcmp(&rx_pkt_hdr->rfc1042_hdr, rfc1042_header, > - sizeof(rfc1042_header)) && > - ntohs(rx_pkt_hdr->rfc1042_hdr.snap_type) != ETH_P_AARP && > - ntohs(rx_pkt_hdr->rfc1042_hdr.snap_type) != ETH_P_IPX)) { > + if (sizeof(rx_pkt_hdr) + rx_pkt_off <= skb->len && sizeof(*rx_pkt_hdr)?
Hi Matthew, On Thu, Sep 7, 2023 at 5:10 PM Matthew Wang <matthewmwang@chromium.org> wrote: > > > - if ((!memcmp(&rx_pkt_hdr->rfc1042_hdr, bridge_tunnel_header, > > - sizeof(bridge_tunnel_header))) || > > - (!memcmp(&rx_pkt_hdr->rfc1042_hdr, rfc1042_header, > > - sizeof(rfc1042_header)) && > > - ntohs(rx_pkt_hdr->rfc1042_hdr.snap_type) != ETH_P_AARP && > > - ntohs(rx_pkt_hdr->rfc1042_hdr.snap_type) != ETH_P_IPX)) { > > + if (sizeof(rx_pkt_hdr) + rx_pkt_off <= skb->len && > > sizeof(*rx_pkt_hdr)? Thanks for catching this. I'll upload a v2 for this. Best, Pin-yen
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_rx.c b/drivers/net/wireless/marvell/mwifiex/sta_rx.c index 65420ad67416..ebb65f3c086c 100644 --- a/drivers/net/wireless/marvell/mwifiex/sta_rx.c +++ b/drivers/net/wireless/marvell/mwifiex/sta_rx.c @@ -86,7 +86,8 @@ 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) { + if (sizeof(rx_pkt_hdr->eth803_hdr) + sizeof(rfc1042_header) + + 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); @@ -95,12 +96,13 @@ int mwifiex_process_rx_packet(struct mwifiex_private *priv, return -1; } - if ((!memcmp(&rx_pkt_hdr->rfc1042_hdr, bridge_tunnel_header, - sizeof(bridge_tunnel_header))) || - (!memcmp(&rx_pkt_hdr->rfc1042_hdr, rfc1042_header, - sizeof(rfc1042_header)) && - ntohs(rx_pkt_hdr->rfc1042_hdr.snap_type) != ETH_P_AARP && - ntohs(rx_pkt_hdr->rfc1042_hdr.snap_type) != ETH_P_IPX)) { + if (sizeof(rx_pkt_hdr) + rx_pkt_off <= skb->len && + ((!memcmp(&rx_pkt_hdr->rfc1042_hdr, bridge_tunnel_header, + sizeof(bridge_tunnel_header))) || + (!memcmp(&rx_pkt_hdr->rfc1042_hdr, rfc1042_header, + sizeof(rfc1042_header)) && + ntohs(rx_pkt_hdr->rfc1042_hdr.snap_type) != ETH_P_AARP && + ntohs(rx_pkt_hdr->rfc1042_hdr.snap_type) != ETH_P_IPX))) { /* * Replace the 803 header and rfc1042 header (llc/snap) with an * EthernetII header, keep the src/dst and snap_type
Only skip the code path trying to access the rfc1042 headers when the buffer is too small, so the driver can still process packets without rfc1042 headers. Fixes: 119585281617 ("wifi: mwifiex: Fix OOB and integer underflow when rx packets") Signed-off-by: Pin-yen Lin <treapking@chromium.org> --- drivers/net/wireless/marvell/mwifiex/sta_rx.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)