Re: [PATCH v2] wifi: rtw88: fix OOB read from firmware RX descriptor exceeding DMA buffer

[Bitterblue Smith <rtl8821cerfe2@gmail.com>]

On 20/04/2026 08:31, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>>
>> On 16/04/2026 01:24, Tristan Madani wrote:
>>> From: Tristan Madani <tristan@talencesecurity.com>
>>>
>>> In rtw_pci_rx_napi(), new_len is computed as the sum of pkt_len (14-bit
>>> descriptor field, max 16383) and pkt_offset (drv_info_sz + shift, both
>>> firmware-controlled). The result can exceed RTK_PCI_RX_BUF_SIZE (11478),
>>> causing an out-of-bounds read from the pre-allocated DMA buffer when
>>> skb_put_data copies new_len bytes. The USB transport already validates
>>> this (rtw_usb_rx_data_put checks against RTW_USB_MAX_RECVBUF_SZ); the
>>> PCIe path does not.
>>>
>>> Add a check that new_len does not exceed the DMA buffer size.
>>>
>>> Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
>>> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
>>> ---
>>> Note: v2 resubmission -- original sent via Gmail had HTML rendering
>>> issues. This version uses git send-email for plain-text formatting.
>>>
>>> Changes in v2:
>>>   - v2: clarify field widths and maximum new_len derivation in commit
>>>     message, per Ping-Ke Shih's feedback.
>>>
>>> drivers/net/wireless/realtek/rtw88/pci.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
>>> index XXXXXXX..XXXXXXX 100644
>>> --- a/drivers/net/wireless/realtek/rtw88/pci.c
>>> +++ b/drivers/net/wireless/realtek/rtw88/pci.c
>>> @@ -1078,6 +1078,11 @@ static int rtw_pci_rx_napi(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci,
>>>               new_len = pkt_stat.pkt_len + pkt_offset;
>>> +             if (new_len > RTK_PCI_RX_BUF_SIZE) {
> 
> Since this is data (hot) path, I'd prefer unlikely(new_len > RTK_PCI_RX_BUF_SIZE).
> 
>>> +                     rtw_dbg(rtwdev, RTW_DBG_RX,
>>> +                             "oversized RX packet: %u\n", new_len);
>>> +                     goto next_rp;
>>> +             }
>>>               new = dev_alloc_skb(new_len);
>>>               if (WARN_ONCE(!new, "rx routine starvation\n"))
>>>                       goto next_rp;
>>>
>>>
>>
>> I'm working on a patch which will implement the same validation
>> in rtw_rx_query_rx_desc(), along with two other checks. I got a
>> report about too short packets from RTL8814AU, so USB devices
>> can also benefit from checking pkt_len. It will make this patch
>> redundant.
> 
> Bitterblue, if you can take the change of this patch into your
> patch, I'd skip this patch. Please let me know your thought.
> 

No, I was mistaken. My patch doesn't do exactly the same thing as
this patch. It just checks if pkt_len is more than 11454. This
patch is also needed. Sorry for the noise.

>>
>> Well, kind of. Maybe RTK_PCI_RX_BUF_SIZE is too small? 11454 + 24
>> doesn't take into account the PHY info size.
> 
> In rtw_pci_sync_rx_desc_device(), driver does
>     buf_desc->buf_size = cpu_to_le16(RTK_PCI_RX_BUF_SIZE);
> 
> This is to tell hardware the size of RX DMA buffer. I think hardware
> can't DMA data over this size.
> 

Indeed, I don't think the hardware will write more than
RTK_PCI_RX_BUF_SIZE bytes. But I wonder if some bytes won't be lost
(or the entire packet?) if it ever receives a frame of 11454 bytes
and wants to attach a PHY status? Then it would need a buffer of
11454 + 24 + 32 bytes. I don't know if this ever happens.

> Ping-Ke
>