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@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
index 3468d4114f60..27e7b8442d7b 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -219,7 +219,7 @@ struct cfg80211_bss *rtw_cfg80211_inform_bss(struct adapter *padapter, struct wl
 	struct mlme_priv *pmlmepriv = &(padapter->mlmepriv);
 
 	bssinf_len = pnetwork->network.ie_length + sizeof(struct ieee80211_hdr_3addr);
-	if (bssinf_len > MAX_BSSINFO_LEN)
+	if (bssinf_len > MAX_BSSINFO_LEN || pnetwork->network.ie_length > MAX_IE_SZ)
 		goto exit;
 
 	{
-- 
2.43.0

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@gmail.com>

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

Hi Dan,

Went and checked for the things you suggested.

Fixes tag: git blame shows this check hasn't been touched since the
original import, 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi
driver"). Added that in v2.

On MAX_BSSINFO_LEN: I couldn't find any rationale for 1000 anywhere
in the history - it's exactly as it was in the 2017 import, no
comment, no commit explaining it. Header (24) + MAX_IE_SZ (768) =
792, so there's already ~200 bytes of slack in the allocation beyond
what ies[] can actually hold. Looks like an arbitrary/conservative
number carried over from wherever this was ported from, not derived
from any struct size in this tree. My patch doesn't touch the
allocation, just tightens the check to match what ies[] can hold.

On the timestamp write - I don't think it's corrupting IE data,
though I get why it looks that way. network.ies[] isn't a pure IE
list despite the name - its declaration comment says "timestamp,
beacon interval, and capability information", and collect_bss_info()
confirms it: it memcpy's straight from the raw frame body right
after the header, so ies[0:8] is the captured TSF, ies[8:10] is
beacon_interval, ies[10:12] is capab_info, and actual variable IEs
start at offset 12 (matches _FIXED_IE_LENGTH_ used elsewhere in this
file). So the memcpy() followed by the timestamp write isn't
scribbling an IE entry - it's replacing the captured TSF (bytes 0-7)
with notify_timestamp = ktime_to_us(ktime_get_boottime()), while
beacon_interval/capab_info/IEs from the original capture stay
untouched. Order doesn't affect the result since it's the same 8
bytes either way.

That said, I'm not certain cfg80211 is fine getting a local boottime
value here instead of the AP's real TSF - if that's actually wrong
I'd like to understand why, I don't have full context on what
cfg80211_inform_bss_frame does with that field internally.

Thanks,
Adi

On Mon, Sep 21, 2026 at 05:03:39PM +0000, Adi Prasan wrote:
> Hi Dan,
> 
> Went and checked for the things you suggested.
> 
> Fixes tag: git blame shows this check hasn't been touched since the
> original import, 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi
> driver"). Added that in v2.
> 
> On MAX_BSSINFO_LEN: I couldn't find any rationale for 1000 anywhere
> in the history - it's exactly as it was in the 2017 import, no
> comment, no commit explaining it. Header (24) + MAX_IE_SZ (768) =
> 792, so there's already ~200 bytes of slack in the allocation beyond
> what ies[] can actually hold. Looks like an arbitrary/conservative
> number carried over from wherever this was ported from, not derived
> from any struct size in this tree. My patch doesn't touch the
> allocation, just tightens the check to match what ies[] can hold.

Try to find a better limit.

> 
> On the timestamp write - I don't think it's corrupting IE data,

Sorry, never mind.  I was looking at the wrong struct...

regards,
dan carpenter

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.

How was this found and tested?

thanks,

greg k-h

Hi Greg,

I ran smatch over drivers/staging and it flagged this
function - the check here allows ie_length up to ~976 bytes (1000
minus the 24-byte header), but network.ies[] is only MAX_IE_SZ (768)
bytes, so the memcpy() a few lines down can read past the end of
that array.

I went and checked every place that sets ie_length before it reaches
here - collect_bss_info() in rtw_mlme_ext.c, and the two
H2C_PARAMETERS_ERROR checks nearby - and all of them already clamp it
to MAX_IE_SZ. So this isn't reachable through any current caller, it
was just the local check not matching the actual buffer size. Wanted
to fix it directly rather than rely on every caller continuing to
enforce that cap.

For testing I have build-tested with make M=drivers/staging/rtl8723bs, clean
checkpatch. I don't have the actual hardware to test at runtime, and
since this only tightens a bound that's already unreachable in
practice, there's no behavior change for any existing valid input.

Thanks,
Adi

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@gmail.com>
Fixes: 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi driver")
---
 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
index 3468d4114f60..27e7b8442d7b 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -219,7 +219,7 @@ struct cfg80211_bss *rtw_cfg80211_inform_bss(struct adapter *padapter, struct wl
 	struct mlme_priv *pmlmepriv = &(padapter->mlmepriv);
 
 	bssinf_len = pnetwork->network.ie_length + sizeof(struct ieee80211_hdr_3addr);
-	if (bssinf_len > MAX_BSSINFO_LEN)
+	if (bssinf_len > MAX_BSSINFO_LEN || pnetwork->network.ie_length > MAX_IE_SZ)
 		goto exit;
 
 	{
-- 
2.43.0

On Mon, Sep 21, 2026 at 04:55:27PM +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@gmail.com>
> Fixes: 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi driver")

Really do some more checking to see if we can use a more sensible
limit instead of MAX_BSSINFO_LEN.  Also try figure out where
pnetwork->network.ie_length is set and verify that it can be out
out bounds.

regards,
dan carpenter