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

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

[Lin YuChen]

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()

[Greg KH]

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()

[Dan Carpenter]

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()

[YuChen Lin]

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

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

[Dan Carpenter]

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