[PATCH] wifi: rtl8xxxu: fix potential use of uninitialized value

[PATCH] wifi: rtl8xxxu: fix potential use of uninitialized value

[Yi Cong]

From: Yi Cong <yicong@kylinos.cn>

The local variables 'mcs' and 'nss' in rtl8xxxu_update_ra_report() are
passed to rtl8xxxu_desc_to_mcsrate() as output parameters. If the helper
function encounters an unhandled rate index, it may return without setting
these values, leading to the use of uninitialized stack data.

Initialize 'mcs' to 0 and 'nss' to 1 at declaration to ensure safe defaults
(MCS 0, 1 spatial stream) are used even if parsing fails. Note that 'nss'
must be at least 1 to be valid.

Fixes: 7de16123d9e2 ("wifi: rtl8xxxu: Introduce rtl8xxxu_update_ra_report")
Cc: stable@vger.kernel.org
Signed-off-by: Yi Cong <yicong@kylinos.cn>
---
 drivers/net/wireless/realtek/rtl8xxxu/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c b/drivers/net/wireless/realtek/rtl8xxxu/core.c
index 794187d28caa..d0035960f8d4 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
@@ -4820,7 +4820,7 @@ static void rtl8xxxu_set_aifs(struct rtl8xxxu_priv *priv, u8 slot_time)
 void rtl8xxxu_update_ra_report(struct rtl8xxxu_ra_report *rarpt,
 			       u8 rate, u8 sgi, u8 bw)
 {
-	u8 mcs, nss;
+	u8 mcs = 0, nss = 1;
 
 	rarpt->txrate.flags = 0;
 
-- 
2.25.1

RE: [PATCH] wifi: rtl8xxxu: fix potential use of uninitialized value

[Ping-Ke Shih]

Yi Cong <cong.yi@linux.dev> wrote:
> From: Yi Cong <yicong@kylinos.cn>
> 
> The local variables 'mcs' and 'nss' in rtl8xxxu_update_ra_report() are
> passed to rtl8xxxu_desc_to_mcsrate() as output parameters. If the helper
> function encounters an unhandled rate index, it may return without setting
> these values, leading to the use of uninitialized stack data.
> 
> Initialize 'mcs' to 0 and 'nss' to 1 at declaration to ensure safe defaults
> (MCS 0, 1 spatial stream) are used even if parsing fails. Note that 'nss'
> must be at least 1 to be valid.
> 
> Fixes: 7de16123d9e2 ("wifi: rtl8xxxu: Introduce rtl8xxxu_update_ra_report")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yi Cong <yicong@kylinos.cn>
> ---
>  drivers/net/wireless/realtek/rtl8xxxu/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> index 794187d28caa..d0035960f8d4 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> @@ -4820,7 +4820,7 @@ static void rtl8xxxu_set_aifs(struct rtl8xxxu_priv *priv, u8 slot_time)
>  void rtl8xxxu_update_ra_report(struct rtl8xxxu_ra_report *rarpt,
>                                u8 rate, u8 sgi, u8 bw)
>  {
> -       u8 mcs, nss;
> +       u8 mcs = 0, nss = 1;
> 
>         rarpt->txrate.flags = 0;

With ` rarpt->txrate.flags |= RATE_INFO_FLAGS_MCS;` in rtl8xxxu_update_ra_report(), 
I believe rtl8xxxu can only support HT rate, so no need 'nss' which is for VHT rate. 

I think the correct fix is something like 

if (rate <= DESC_RATE_54M) {
		// does something as current
} else if (rate >= DESC_RATE_MCS0 && rate <= DESC_RATE_MCS15) {
		mcs = rate - DESC_RATE_MCS0;

		// and HT flags
} else {
		return; // do nothing for unexpected rate
}

// fill common flags for legacy and HT rate.