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
