Re: [PATCH rtw-next] wifi: rtw88: Add more validation for the RX descriptor

[Bitterblue Smith <rtl8821cerfe2@gmail.com>]

On 18/05/2026 11:14, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>> Some RTL8821CE cards can return frames with corrupted RX descriptor,
>> causing warnings and crashes if they are passed to the upper layers.
> 
> Not sure if this is the reason Larry uploaded a copy of vendor driver
> to his repository [1]. 
> 

He added it for someone whose wifi card sometimes wasn't powering on:

https://github.com/lwfinger/rtw88/issues/98#issuecomment-1263962943

> Recently, we received vulnerability report of rtw_mp_efuse_set() in
> vendor driver. I'd like to know if people are still using the vendor
> driver [1]. If not, is it possible to remove it? If people still need it,
> I will share the fix made by our internal later.
> 
> [1] https://github.com/lwfinger/rtw88/tree/master/alt_rtl8821ce
> 

We haven't been updating it for the kernel API changes, so I think we
can remove it.

>>
>> The PHY status size field is 4 bits wide, but in rtw88 its value should
>> only be 0 or 4. Checking this catches most of the corrupt frames.
>>
>> If a PHY status is present, the PHY status size should not be 0.
>>
>> The frame size should not be less than or equal to 4 and should not
>> exceed 11454.
>>
>> Discard the frame if any of these checks fail.
>>
>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221286
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> 
> Acked-by: Ping-Ke Shih <pkshih@realtek.com>
> 
> [...]
> 
>> diff --git a/drivers/net/wireless/realtek/rtw88/rx.c b/drivers/net/wireless/realtek/rtw88/rx.c
>> index d9e11343d498..65f6db3d7fcb 100644
>> --- a/drivers/net/wireless/realtek/rtw88/rx.c
>> +++ b/drivers/net/wireless/realtek/rtw88/rx.c
>> @@ -3,6 +3,7 @@
>>   */
>>
>>  #include "main.h"
>> +#include "mac.h"
>>  #include "rx.h"
>>  #include "ps.h"
>>  #include "debug.h"
>> @@ -261,9 +262,9 @@ static void rtw_rx_fill_rx_status(struct rtw_dev *rtwdev,
>>         }
>>  }
>>
>> -void rtw_rx_query_rx_desc(struct rtw_dev *rtwdev, void *rx_desc8,
>> -                         struct rtw_rx_pkt_stat *pkt_stat,
>> -                         struct ieee80211_rx_status *rx_status)
>> +int rtw_rx_query_rx_desc(struct rtw_dev *rtwdev, void *rx_desc8,
>> +                        struct rtw_rx_pkt_stat *pkt_stat,
>> +                        struct ieee80211_rx_status *rx_status)
>>  {
>>         u32 desc_sz = rtwdev->chip->rx_pkt_desc_sz;
>>         struct rtw_rx_desc *rx_desc = rx_desc8;
>> @@ -303,12 +304,25 @@ void rtw_rx_query_rx_desc(struct rtw_dev *rtwdev, void *rx_desc8,
>>                 pkt_stat->bw = RTW_CHANNEL_WIDTH_20;
> 
> Do you think if we should return -EINVAL for this case too?
> 

Yes. What do we do with the debug message? Should the other
conditions also have a debug message?

>>         }
>>
>> +       if (unlikely(pkt_stat->drv_info_sz &&
>> +                    pkt_stat->drv_info_sz != PHY_STATUS_SIZE))
>> +               return -EINVAL;
>> +
>> +       if (unlikely(pkt_stat->phy_status && !pkt_stat->drv_info_sz))
>> +               return -EINVAL;
>> +
>> +       if (unlikely(pkt_stat->pkt_len > IEEE80211_MAX_MPDU_LEN_VHT_11454))
>> +               return -EINVAL;
>> +
>>         /* drv_info_sz is in unit of 8-bytes */
>>         pkt_stat->drv_info_sz *= 8;
>>
>>         /* c2h cmd pkt's rx/phy status is not interested */
>>         if (pkt_stat->is_c2h)
>> -               return;
>> +               return 0;
>> +
>> +       if (unlikely(pkt_stat->pkt_len <= FCS_LEN))
>> +               return -EINVAL;
>>
>>         phy_status = rx_desc8 + desc_sz + pkt_stat->shift;
>>         hdr = phy_status + pkt_stat->drv_info_sz;
> 
> [...]
>