[PATCH v2] staging: rtl8723bs: initialize le_tmp64 in rtw_BIP_verify()

Lin YuChen posted 1 patch 2 weeks ago
drivers/staging/rtl8723bs/core/rtw_security.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v2] staging: rtl8723bs: initialize le_tmp64 in rtw_BIP_verify()
Posted by Lin YuChen 2 weeks ago
Initialize le_tmp64 to zero in rtw_BIP_verify() to prevent using
uninitialized data.

Smatch warns that only 6 bytes are copied to this 8-byte (u64)
variable, leaving the last two bytes uninitialized:

drivers/staging/rtl8723bs/core/rtw_security.c:1308 rtw_BIP_verify()
warn: not copying enough bytes for '&le_tmp64' (8 vs 6 bytes)

Initializing the variable at the start of the function fixes this
warning and ensures predictable behavior.

Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/linux-staging/abvwIQh0CHTp4wNJ@stanley.mountain/
Signed-off-by: Lin YuChen <starpt.official@gmail.com>
---
v2: Add Fixes: tag as suggested by Dan Carpenter.

 drivers/staging/rtl8723bs/core/rtw_security.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
index b489babe7432..c3f5fc4abd17 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -1291,7 +1291,7 @@ u32 rtw_BIP_verify(struct adapter *padapter, u8 *precvframe)
 	u8 mic[16];
 	struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
 	__le16 le_tmp;
-	__le64 le_tmp64;
+	__le64 le_tmp64 = 0;
 
 	ori_len = pattrib->pkt_len - WLAN_HDR_A3_LEN + BIP_AAD_SIZE;
 	BIP_AAD = kzalloc(ori_len, GFP_KERNEL);
-- 
2.34.1
Re: [PATCH v2] staging: rtl8723bs: initialize le_tmp64 in rtw_BIP_verify()
Posted by Dan Carpenter 1 week, 6 days ago
On Sat, Mar 21, 2026 at 01:25:02AM +0800, Lin YuChen wrote:
> Initialize le_tmp64 to zero in rtw_BIP_verify() to prevent using
> uninitialized data.
> 
> Smatch warns that only 6 bytes are copied to this 8-byte (u64)
> variable, leaving the last two bytes uninitialized:
> 
> drivers/staging/rtl8723bs/core/rtw_security.c:1308 rtw_BIP_verify()
> warn: not copying enough bytes for '&le_tmp64' (8 vs 6 bytes)
> 
> Initializing the variable at the start of the function fixes this
> warning and ensures predictable behavior.
> 
> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/linux-staging/abvwIQh0CHTp4wNJ@stanley.mountain/
> Signed-off-by: Lin YuChen <starpt.official@gmail.com>
> ---
> v2: Add Fixes: tag as suggested by Dan Carpenter.

Fantastic.  Thanks!

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter
Re: [PATCH v2] staging: rtl8723bs: initialize le_tmp64 in rtw_BIP_verify()
Posted by Greg KH 2 weeks ago
On Sat, Mar 21, 2026 at 01:25:02AM +0800, Lin YuChen wrote:
> Initialize le_tmp64 to zero in rtw_BIP_verify() to prevent using
> uninitialized data.
> 
> Smatch warns that only 6 bytes are copied to this 8-byte (u64)
> variable, leaving the last two bytes uninitialized:
> 
> drivers/staging/rtl8723bs/core/rtw_security.c:1308 rtw_BIP_verify()
> warn: not copying enough bytes for '&le_tmp64' (8 vs 6 bytes)
> 
> Initializing the variable at the start of the function fixes this
> warning and ensures predictable behavior.

Which makes me wonder how this ever worked at all, if random data was in
those 2 bytes.

Was this tested?  Are you sure this will keep working?  If so, is this
code ever even called?

thanks,

greg k-h
Re: [PATCH v2] staging: rtl8723bs: initialize le_tmp64 in rtw_BIP_verify()
Posted by Dan Carpenter 1 week, 6 days ago
On Fri, Mar 20, 2026 at 06:29:13PM +0100, Greg KH wrote:
> On Sat, Mar 21, 2026 at 01:25:02AM +0800, Lin YuChen wrote:
> > Initialize le_tmp64 to zero in rtw_BIP_verify() to prevent using
> > uninitialized data.
> > 
> > Smatch warns that only 6 bytes are copied to this 8-byte (u64)
> > variable, leaving the last two bytes uninitialized:
> > 
> > drivers/staging/rtl8723bs/core/rtw_security.c:1308 rtw_BIP_verify()
> > warn: not copying enough bytes for '&le_tmp64' (8 vs 6 bytes)
> > 
> > Initializing the variable at the start of the function fixes this
> > warning and ensures predictable behavior.
> 
> Which makes me wonder how this ever worked at all, if random data was in
> those 2 bytes.

These days, everyone sane zeroes their stack variables, but this driver
is older than the zeroing code so it's a puzzling thing.

I could imagine a couple different ways that the code might be able to
work even with uninitialized data...  It wouldn't surprise me if the
check for:

	/* BIP packet number should bigger than previous BIP packet */

is some kind of work around for bug?

regards,
dan carpenter
Re: [PATCH v2] staging: rtl8723bs: initialize le_tmp64 in rtw_BIP_verify()
Posted by YuChen Lin 1 week, 4 days ago
On Sat, Mar 21, 2026 at 3:58 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> On Fri, Mar 20, 2026 at 06:29:13PM +0100, Greg KH wrote:
> > ...
>
> These days, everyone sane zeroes their stack variables, but this driver
> is older than the zeroing code so it's a puzzling thing.
>
> I could imagine a couple different ways that the code might be able to
> work even with uninitialized data...  It wouldn't surprise me if the
> check for:
>
>         /* BIP packet number should bigger than previous BIP packet */
>
> is some kind of work around for bug?

Hi Greg and Dan,

Thanks for the comments.

I do not have the physical RTL8723BS hardware to perform runtime tests.
Therefore, I cannot confirm if the code currently "works" due to a zeroed
stack, a specific workaround, or if it is silently failing in ways users
haven't reported yet.

Regardless of why it might appear to work now, the current state relies
on uninitialized stack data, which introduces non-deterministic behavior.

Initializing the variable to zero is a reasonable and defensive fix. It
ensures the IPN calculation is predictable and correctly represents the
6-byte value from the MMIE, as identified by Smatch.

I believe this is a safe improvement to ensure the long-term stability
of the driver.

Regards,
Lin YuChen