RE: [PATCH rtw-next] wifi: rtw88: sdio: reject oversized RX packets

[Ping-Ke Shih <pkshih@realtek.com>]

Luka Gejak <luka.gejak@linux.dev> wrote:
> --- a/drivers/net/wireless/realtek/rtw88/sdio.c
> +++ b/drivers/net/wireless/realtek/rtw88/sdio.c
> @@ -1389,6 +1389,14 @@ static void rtw_sdio_rxfifo_recv(struct rtw_dev *rtwdev, u32 rx_len)
>                 curr_pkt_len = ALIGN(pkt_offset + pkt_stat.pkt_len,
>                                      RTW_SDIO_DATA_PTR_ALIGN);
> 
> +               if (unlikely(pkt_offset + pkt_stat.pkt_len > rx_len)) {

Can you use real hardware to test this patch to see if there is actual case
falling into this branch? (see below)

> +                       rtw_dbg(rtwdev, RTW_DBG_SDIO,
> +                               "oversized RX packet: %u, %u bytes left\n",
> +                               pkt_offset + pkt_stat.pkt_len, rx_len);
> +                       dev_kfree_skb_any(skb);
> +                       return;
> +               }
> +
>                 if ((curr_pkt_len + pkt_desc_sz) >= rx_len) {

Because I wonder if this condition is right? In my perspective, the
curr_pkt_len = pkt_offset          + pkt_stat.pkt_len + align_bytes
             = pkt_desc_sz + ...   + pkt_stat.pkt_len + align_bytes

Why does the if-condition plus pkt_desc_sz twice and compare with rx_len?

However, it should enter to handle/free the last aggregated packet. 
Can you enlighten me how it looks like that?

>                         /* Use the original skb (with it's adjusted offset)
>                          * when processing the last (or even the only) entry to
> --
> 2.55.0