public inbox for linux-wireless@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] wifi: rtw88: fix OOB read from firmware RX descriptor exceeding DMA buffer
@ 2026-04-15 22:24 Tristan Madani
  2026-04-17 15:14 ` Bitterblue Smith
  0 siblings, 1 reply; 5+ messages in thread
From: Tristan Madani @ 2026-04-15 22:24 UTC (permalink / raw)
  To: Ping-Ke Shih; +Cc: Johannes Berg, linux-wireless

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) {
+			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;


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] wifi: rtw88: fix OOB read from firmware RX descriptor exceeding DMA buffer
  2026-04-15 22:24 [PATCH v2] wifi: rtw88: fix OOB read from firmware RX descriptor exceeding DMA buffer Tristan Madani
@ 2026-04-17 15:14 ` Bitterblue Smith
  2026-04-20  5:31   ` Ping-Ke Shih
  0 siblings, 1 reply; 5+ messages in thread
From: Bitterblue Smith @ 2026-04-17 15:14 UTC (permalink / raw)
  To: Tristan Madani, Ping-Ke Shih; +Cc: Johannes Berg, linux-wireless

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) {
> +			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.

Well, kind of. Maybe RTK_PCI_RX_BUF_SIZE is too small? 11454 + 24
doesn't take into account the PHY info size.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH v2] wifi: rtw88: fix OOB read from firmware RX descriptor exceeding DMA buffer
  2026-04-17 15:14 ` Bitterblue Smith
@ 2026-04-20  5:31   ` Ping-Ke Shih
  2026-04-20 22:22     ` Bitterblue Smith
  0 siblings, 1 reply; 5+ messages in thread
From: Ping-Ke Shih @ 2026-04-20  5:31 UTC (permalink / raw)
  To: Bitterblue Smith, Tristan Madani
  Cc: Johannes Berg, linux-wireless@vger.kernel.org

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.

> 
> 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.

Ping-Ke


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] wifi: rtw88: fix OOB read from firmware RX descriptor exceeding DMA buffer
  2026-04-20  5:31   ` Ping-Ke Shih
@ 2026-04-20 22:22     ` Bitterblue Smith
  2026-04-21  1:38       ` Ping-Ke Shih
  0 siblings, 1 reply; 5+ messages in thread
From: Bitterblue Smith @ 2026-04-20 22:22 UTC (permalink / raw)
  To: Ping-Ke Shih, Tristan Madani
  Cc: Johannes Berg, linux-wireless@vger.kernel.org

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
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH v2] wifi: rtw88: fix OOB read from firmware RX descriptor exceeding DMA buffer
  2026-04-20 22:22     ` Bitterblue Smith
@ 2026-04-21  1:38       ` Ping-Ke Shih
  0 siblings, 0 replies; 5+ messages in thread
From: Ping-Ke Shih @ 2026-04-21  1:38 UTC (permalink / raw)
  To: Bitterblue Smith, Tristan Madani
  Cc: Johannes Berg, linux-wireless@vger.kernel.org

Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> On 20/04/2026 08:31, Ping-Ke Shih wrote:
> > Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> >>
> >> 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.

I think the total size including PHY status must be smaller than
RTK_PCI_RX_BUF_SIZE, otherwise we should enlarge RTK_PCI_RX_BUF_SIZE.

The rtw89 can split a packet into multiple segments if RX buffer is 
smaller than a receiving packet, but it has exact overload, so we
enlarge the buffer as large as necessary. 

For rtw88, I can't find similar implementation. Maybe we can try to
enlarge the size to see if any improvement. 

Ping-Ke


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-21  1:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-15 22:24 [PATCH v2] wifi: rtw88: fix OOB read from firmware RX descriptor exceeding DMA buffer Tristan Madani
2026-04-17 15:14 ` Bitterblue Smith
2026-04-20  5:31   ` Ping-Ke Shih
2026-04-20 22:22     ` Bitterblue Smith
2026-04-21  1:38       ` Ping-Ke Shih

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox