Re: [PATCH] staging: rtl8723bs: fix ie_length bound check in rtw_cfg80211_inform_bss
From: Dan Carpenter
Date: Mon Sep 21 2026 - 11:44:49 EST
On Sun, Sep 20, 2026 at 02:28:49PM +0000, Adi Prasan wrote:
> The buffer bound check in rtw_cfg80211_inform_bss() only verifies
> that bssinf_len (ie_length + header size) does not exceed
> MAX_BSSINFO_LEN (1000 bytes), but network.ies[] is only MAX_IE_SZ
> (768) bytes. This allows ie_length values up to ~976 bytes to pass
> the check while a subsequent memcpy() from network.ies still reads
> only 768 valid bytes, and other paths that write to network.ies
> consistently cap ie_length to MAX_IE_SZ.
>
> Add an explicit check against MAX_IE_SZ so the bound matches the
> actual size of network.ies.
>
> Signed-off-by: Adi Prasan <itsadi2409@xxxxxxxxx>
This needs a Fixes tag.
The original code seems like a bounds check on the destination.
Your code adds a separate bounds check on the read buffer.
Why do we even have the MAX_BSSINFO_LEN limit? What's that based
on? 1000 seems like a very suspicious number to me. It's a normal
enough number for humans, but it's a strange number when we're adding
up struct sizes. Do we ever need the whole buffer? (These questions
are basically rephrasing the same question. I'm assuming everyone
just feeds them to AI, and I'm trying to learn who to do prompt
engineering).
It wouldn't surprise me if there was a different read check on the
source buffer.
The other question for me is:
304 memcpy(pbuf, pnetwork->network.ies, pnetwork->network.ie_length);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
We copy the network.ies entries to pbuf
305 len += pnetwork->network.ie_length;
306
307 *((__le64 *)pbuf) = cpu_to_le64(notify_timestamp);
^^^^^^^^^^^^^^^^^
And then scribble over the first entry. That doesn't make sense.
Should the timestamp go before or after the entries? Review the
git log and other implementations of the the realtek wireless drivers
to check.
regards,
dan carpenter