[PATCH] staging: rtl8723bs: fix ie_length bound check in rtw_cfg80211_inform_bss

[PATCH] staging: rtl8723bs: fix ie_length bound check in rtw_cfg80211_inform_bss

[Adi Prasan]

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

Re: [PATCH] staging: rtl8723bs: fix ie_length bound check in rtw_cfg80211_inform_bss

[Greg KH]

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

Re: [PATCH] staging: rtl8723bs: fix ie_length bound check in rtw_cfg80211_inform_bss

[Adi Prasan]

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

Re: [PATCH] staging: rtl8723bs: fix ie_length bound check in rtw_cfg80211_inform_bss

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

[PATCH v2] staging: rtl8723bs: fix ie_length bound check in rtw_cfg80211_inform_bss

[Adi Prasan]

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

Re: [PATCH] staging: rtl8723bs: fix ie_length bound check in rtw_cfg80211_inform_bss

[Adi Prasan]

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

Re: [PATCH] staging: rtl8723bs: fix ie_length bound check in rtw_cfg80211_inform_bss

[Dan Carpenter]

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

Re: [PATCH v2] staging: rtl8723bs: fix ie_length bound check in rtw_cfg80211_inform_bss

[Dan Carpenter]

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

Re: [PATCH v2] staging: rtl8723bs: fix ie_length bound check in rtw_cfg80211_inform_bss

[Dan Carpenter]

On Thu, Oct 01, 2026 at 10:51:00PM +0530, Adi Prasan wrote:
> Sorry for the late reply.
> On where ie_length gets set - there's three places: collect_bss_info() in
> rtw_mlme_ext.c, and the two H2C_PARAMETERS_ERROR checks further down in the
> same file. All three cap it to MAX_IE_SZ before it's ever assigned, so it
> can't actually go out of bounds.
> 
> On a more sensible limit instead of MAX_BSSINFO_LEN.what do you have in
> mind?

Please don't top post.

Why not use MAX_IE_SZ as the limit?

regards,
dan carpenter

[PATCH v3] staging: rtl8723bs: fix ie_length bound check in rtw_cfg80211_inform_bss

[Adi Prasan]

The buffer bound check in rtw_cfg80211_inform_bss() checked bssinf_len
(ie_length + header size) against MAX_BSSINFO_LEN (1000 bytes), but
network.ies[] is only MAX_IE_SZ (768) bytes. This allowed ie_length
values up to ~976 bytes to pass the check while a subsequent memcpy()
from network.ies still only has 768 valid bytes, and other paths that
write to network.ies consistently cap ie_length to MAX_IE_SZ.

Check ie_length against MAX_IE_SZ directly, which is the actual size
of network.ies, and drop the now-unused bssinf_len/MAX_BSSINFO_LEN
comparison.

Signed-off-by: Adi Prasan <itsadi2409@gmail.com>
Fixes: 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi driver")
---
 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
index 27e7b8442d7b..7e964b15ae6d 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -210,7 +210,7 @@ struct cfg80211_bss *rtw_cfg80211_inform_bss(struct adapter *padapter, struct wl
 	u64 notify_timestamp;
 	s32 notify_signal;
 	u8 *buf = NULL, *pbuf;
-	size_t len, bssinf_len = 0;
+	size_t len;
 	struct ieee80211_hdr *pwlanhdr;
 	__le16 *fctrl;
 
@@ -218,8 +218,7 @@ struct cfg80211_bss *rtw_cfg80211_inform_bss(struct adapter *padapter, struct wl
 	struct wiphy *wiphy = wdev->wiphy;
 	struct mlme_priv *pmlmepriv = &(padapter->mlmepriv);
 
-	bssinf_len = pnetwork->network.ie_length + sizeof(struct ieee80211_hdr_3addr);
-	if (bssinf_len > MAX_BSSINFO_LEN || pnetwork->network.ie_length > MAX_IE_SZ)
+	if (pnetwork->network.ie_length > MAX_IE_SZ)
 		goto exit;
 
 	{
-- 
2.43.0

Re: [PATCH] staging: rtl8723bs: fix ie_length bound check in rtw_cfg80211_inform_bss

[Adi Prasan]

On Fri, Oct 02, 2026, Dan Carpenter wrote:
> Why not use MAX_IE_SZ as the limit?

Makes sense, done in v3 - dropped the bssinf_len/MAX_BSSINFO_LEN
check entirely and check ie_length against MAX_IE_SZ directly, which
is the actual size of network.ies. Removed bssinf_len too since
nothing else used it.

Adi

Re: [PATCH v3] staging: rtl8723bs: fix ie_length bound check in rtw_cfg80211_inform_bss

[Greg KH]

On Fri, Oct 02, 2026 at 02:04:50PM +0000, Adi Prasan wrote:
> The buffer bound check in rtw_cfg80211_inform_bss() checked bssinf_len
> (ie_length + header size) against MAX_BSSINFO_LEN (1000 bytes), but
> network.ies[] is only MAX_IE_SZ (768) bytes. This allowed ie_length
> values up to ~976 bytes to pass the check while a subsequent memcpy()
> from network.ies still only has 768 valid bytes, and other paths that
> write to network.ies consistently cap ie_length to MAX_IE_SZ.
> 
> Check ie_length against MAX_IE_SZ directly, which is the actual size
> of network.ies, and drop the now-unused bssinf_len/MAX_BSSINFO_LEN
> comparison.
> 
> Signed-off-by: Adi Prasan <itsadi2409@gmail.com>
> Fixes: 554c0a3abf216 ("staging: Add rtl8723bs sdio wifi driver")
> ---
>  drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
> index 27e7b8442d7b..7e964b15ae6d 100644
> --- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
> +++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
> @@ -210,7 +210,7 @@ struct cfg80211_bss *rtw_cfg80211_inform_bss(struct adapter *padapter, struct wl
>  	u64 notify_timestamp;
>  	s32 notify_signal;
>  	u8 *buf = NULL, *pbuf;
> -	size_t len, bssinf_len = 0;
> +	size_t len;
>  	struct ieee80211_hdr *pwlanhdr;
>  	__le16 *fctrl;
>  
> @@ -218,8 +218,7 @@ struct cfg80211_bss *rtw_cfg80211_inform_bss(struct adapter *padapter, struct wl
>  	struct wiphy *wiphy = wdev->wiphy;
>  	struct mlme_priv *pmlmepriv = &(padapter->mlmepriv);
>  
> -	bssinf_len = pnetwork->network.ie_length + sizeof(struct ieee80211_hdr_3addr);
> -	if (bssinf_len > MAX_BSSINFO_LEN || pnetwork->network.ie_length > MAX_IE_SZ)
> +	if (pnetwork->network.ie_length > MAX_IE_SZ)
>  		goto exit;
>  
>  	{
> -- 
> 2.43.0
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

- You have marked a patch with a "Fixes:" tag for a commit that is in an
  older released kernel, yet you do not have a cc: stable line in the
  signed-off-by area at all, which means that the patch will not be
  applied to any older kernel releases.  To properly fix this, please
  follow the documented rules in the
  Documentation/process/stable-kernel-rules.rst file for how to resolve
  this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot