From: Dan Carpenter <error27@gmail.com>
To: Adi Prasan <itsadi2409@gmail.com>
Cc: gregkh@linuxfoundation.org, linux-staging@lists.linux.dev,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: rtl8723bs: fix ie_length bound check in rtw_cfg80211_inform_bss
Date: Mon, 21 Sep 2026 18:23:17 +0300 [thread overview]
Message-ID: <arFL5SLHwLflaONc@stanley.mountain> (raw)
In-Reply-To: <20260920142849.294162-1-itsadi2409@gmail.com>
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
next prev parent reply other threads:[~2026-09-21 15:23 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-20 14:28 Adi Prasan
2026-09-20 15:37 ` Greg KH
2026-09-21 5:13 ` Adi Prasan
2026-09-21 15:23 ` Dan Carpenter [this message]
2026-09-21 17:03 ` Adi Prasan
2026-09-21 16:55 ` [PATCH v2] " Adi Prasan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=arFL5SLHwLflaONc@stanley.mountain \
--to=error27@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=itsadi2409@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®