linux-kernel - [PATCH] staging: r8188eu: fix potential uninitialised variable use in rtw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:   Sun, 31 Jul 2022 00:59:10 +0100
From:   Phillip Potter <phil@...lpotter.co.uk>
To:     gregkh@...uxfoundation.org
Cc:     Larry.Finger@...inger.net, dan.carpenter@...cle.com,
        paskripkin@...il.com, martin@...ser.cx,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: [PATCH] staging: r8188eu: fix potential uninitialised variable use in rtw_pwrctrl.c

Set ret to 0 (success) before entering first if statement, thereby
assuring that even if the device is not associated and further checks
pass, we do not then end up returning the uninitialized value of ret.
This assignment is deliberately now directly before the if statement, in
order to keep it clear what is happening as opposed to having it as an
initialization at the start of the function like it was originally.

Also add a comment to make it clear this first if block is currently a
success path. As a side note, smatch does not trigger warnings for this
change, for me at least.

Within core/rtw_pwrctrl.c in the rtw_pwr_wakeup function, I previously
dropped the initialization of 'ret' (int ret = 0;) in favour of its
assignment which happens inside the first if block directly before its
corresponding goto. This was the cause of this bug, and was introduced
by: commit f3a76018dd55 ("staging: r8188eu: remove initializer from ret
in rtw_pwr_wakeup").

Fixes: f3a76018dd55 ("staging: r8188eu: remove initializer from ret in rtw_pwr_wakeup")
Reported-by: kernel test robot <lkp@...el.com>
Signed-off-by: Phillip Potter <phil@...lpotter.co.uk>
---
 drivers/staging/r8188eu/core/rtw_pwrctrl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index 75e655bae16a..10550bd2c16d 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -387,10 +387,10 @@ int rtw_pwr_wakeup(struct adapter *padapter)
 		msleep(10);

 	/* I think this should be check in IPS, LPS, autosuspend functions... */
-	if (check_fwstate(pmlmepriv, _FW_LINKED)) {
-		ret = 0;
+	/* Below goto is a success path taken for already linked devices */
+	ret = 0;
+	if (check_fwstate(pmlmepriv, _FW_LINKED))
 		goto exit;
-	}

 	if (pwrpriv->rf_pwrstate == rf_off && ips_leave(padapter) == _FAIL) {
 		ret = -ENOMEM;
-- 
2.36.1