Re: [PATCH v3] staging: rtl8723bs: validate monitor transmit frame lengths

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Fri, Jul 17, 2026 at 10:24:37AM -0300, Mariano Baragiola wrote:
> rtw_cfg80211_monitor_if_xmit_entry() removes the radiotap header and
> then reads the 802.11 frame control field without checking that a base
> 802.11 header remains.
> 
> The data path also pulls the calculated 802.11, QoS and SNAP header
> span before confirming that the skb contains it. A truncated frame can
> therefore cause out-of-bounds reads or leave insufficient data for the
> Ethernet address writes.
> 
> Reject frames that do not contain the base 802.11 header and data
> frames that do not contain their complete calculated header span.
> 
> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> Signed-off-by: Mariano Baragiola <mbaragiola@linux.com>
> ---
> Changes in v3:
> - Add the below-separator changelog as requested by Greg K-H patch-bot.
> 
> Changes in v2:
> - Add the Fixes tag as requested by Dan Carpenter.
> 
>  drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
> index 6a97afd89dc7..bed8192ab44f 100644
> --- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
> +++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
> @@ -2033,6 +2033,8 @@ static netdev_tx_t rtw_cfg80211_monitor_if_xmit_entry(struct sk_buff *skb, struc
>  
>  	/* Skip the ratio tap header */
>  	skb_pull(skb, rtap_len);
> +	if (unlikely(skb->len < dot11_hdr_len))
> +		goto fail;

Only ever use likely/unlikely if you can measure the difference of it
not being there.  Otherwise the cpu will almost always get it right
instead.

Is that the case here?


>  
>  	dot11_hdr = (struct ieee80211_hdr *)skb->data;
>  	frame_control = le16_to_cpu(dot11_hdr->frame_control);
> @@ -2045,6 +2047,8 @@ static netdev_tx_t rtw_cfg80211_monitor_if_xmit_entry(struct sk_buff *skb, struc
>  			qos_len = 2;
>  		if ((frame_control & 0x0300) == 0x0300)
>  			dot11_hdr_len += 6;
> +		if (unlikely(skb->len < dot11_hdr_len + qos_len + snap_len))
> +			goto fail;

Same here, is this actually measurable?

thanks,

greg k-h