* [PATCH v2] staging: rtl8723bs: initialize le_tmp64 in rtw_BIP_verify()
@ 2026-03-20 17:25 Lin YuChen
2026-03-20 17:29 ` Greg KH
2026-03-21 7:47 ` Dan Carpenter
0 siblings, 2 replies; 5+ messages in thread
From: Lin YuChen @ 2026-03-20 17:25 UTC (permalink / raw)
To: gregkh, dan.carpenter
Cc: straube.linux, starpt.official, linux-staging, linux-kernel
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] staging: rtl8723bs: initialize le_tmp64 in rtw_BIP_verify()
2026-03-20 17:25 [PATCH v2] staging: rtl8723bs: initialize le_tmp64 in rtw_BIP_verify() Lin YuChen
@ 2026-03-20 17:29 ` Greg KH
2026-03-21 7:57 ` Dan Carpenter
2026-03-21 7:47 ` Dan Carpenter
1 sibling, 1 reply; 5+ messages in thread
From: Greg KH @ 2026-03-20 17:29 UTC (permalink / raw)
To: Lin YuChen; +Cc: dan.carpenter, straube.linux, linux-staging, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] staging: rtl8723bs: initialize le_tmp64 in rtw_BIP_verify()
2026-03-20 17:25 [PATCH v2] staging: rtl8723bs: initialize le_tmp64 in rtw_BIP_verify() Lin YuChen
2026-03-20 17:29 ` Greg KH
@ 2026-03-21 7:47 ` Dan Carpenter
1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2026-03-21 7:47 UTC (permalink / raw)
To: Lin YuChen; +Cc: gregkh, straube.linux, linux-staging, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] staging: rtl8723bs: initialize le_tmp64 in rtw_BIP_verify()
2026-03-20 17:29 ` Greg KH
@ 2026-03-21 7:57 ` Dan Carpenter
2026-03-23 10:38 ` YuChen Lin
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2026-03-21 7:57 UTC (permalink / raw)
To: Greg KH; +Cc: Lin YuChen, straube.linux, linux-staging, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] staging: rtl8723bs: initialize le_tmp64 in rtw_BIP_verify()
2026-03-21 7:57 ` Dan Carpenter
@ 2026-03-23 10:38 ` YuChen Lin
0 siblings, 0 replies; 5+ messages in thread
From: YuChen Lin @ 2026-03-23 10:38 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Greg KH, straube.linux, linux-staging, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-03-23 10:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-20 17:25 [PATCH v2] staging: rtl8723bs: initialize le_tmp64 in rtw_BIP_verify() Lin YuChen
2026-03-20 17:29 ` Greg KH
2026-03-21 7:57 ` Dan Carpenter
2026-03-23 10:38 ` YuChen Lin
2026-03-21 7:47 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox