[lvc-project] [PATCH] wifi: mwifiex: avoid possible NULL skb pointer dereference

[Dmitry Antipov]

On 8/8/23 23:11, Brian Norris wrote:

> This feels like it should be 'rx_dropped', since we're dropping it
> before we done any real "RX" (let alone getting to any forward/outbound
> operation). I doubt it makes a big difference overall, but it seems like
> the right thing to do.

This is somewhat confusing for me indeed. In 'mwifiex_uap_queue_bridged_pkt()',
both 'rx_dropped' and 'tx_dropped' may be incremented, for a different reasons
(unexpected skb layout and error (re)allocating new skb, respectively).

And I have some doubts on 119585281617 ("wifi: mwifiex: Fix OOB and integer
underflow when rx packets"). Looking through 'mwifiex_uap_queue_bridged_pkt()'
again, it seems that 'return' is missing:

	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);
                /* HERE */
	}

	if ((!memcmp(&rx_pkt_hdr->rfc1042_hdr, bridge_tunnel_header,

because 'rx_pkt_hdr' points to 'skb->data' plus some offset (see above),
so reading freed memory with 'memcmp()' causes an undefined behavior.
And likewise for 'mwifiex_process_rx_packet()' (but not for
'mwifiex_process_uap_rx_packet()' where 'return 0' looks correct).

Dmitry