The sdio_read32() calls sd_read(), but does not handle the error if
sd_read() fails. This could lead to subsequent operations processing
invalid data. A proper implementation can be found in sdio_readN().

Add error handling to the sd_read(), ensuring that the memcpy() is
only performed when the read operation is successful.

Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/staging/rtl8723bs/hal/sdio_ops.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/sdio_ops.c b/drivers/staging/rtl8723bs/hal/sdio_ops.c
index 21e9f1858745..86776d924c15 100644
--- a/drivers/staging/rtl8723bs/hal/sdio_ops.c
+++ b/drivers/staging/rtl8723bs/hal/sdio_ops.c
@@ -185,9 +185,11 @@ static u32 sdio_read32(struct intf_hdl *intfhdl, u32 addr)
 			return SDIO_ERR_VAL32;
 
 		ftaddr &= ~(u16)0x3;
-		sd_read(intfhdl, ftaddr, 8, tmpbuf);
-		memcpy(&le_tmp, tmpbuf + shift, 4);
-		val = le32_to_cpu(le_tmp);
+		err = sd_read(intfhdl, ftaddr, 8, tmpbuf);
+		if (!err) {
+			memcpy(&le_tmp, tmpbuf + shift, 4);
+			val = le32_to_cpu(le_tmp);
+		}
 
 		kfree(tmpbuf);
 	}
-- 
2.42.0.windows.2

Hi Wentao,

kernel test robot noticed the following build warnings:

[auto build test WARNING on staging/staging-testing]

url:    https://github.com/intel-lab-lkp/linux/commits/Wentao-Liang/staging-rtl8723bs-Add-error-handling-for-sd_read/20250403-174101
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20250403093741.2372-1-vulab%40iscas.ac.cn
patch subject: [PATCH] staging: rtl8723bs: Add error handling for sd_read().
config: hexagon-allyesconfig (https://download.01.org/0day-ci/archive/20250404/202504042334.1pXFeKzN-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project 92c93f5286b9ff33f27ff694d2dc33da1c07afdd)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250404/202504042334.1pXFeKzN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504042334.1pXFeKzN-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/staging/rtl8723bs/hal/sdio_ops.c:189:7: warning: variable 'val' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     189 |                 if (!err) {
         |                     ^~~~
   drivers/staging/rtl8723bs/hal/sdio_ops.c:196:9: note: uninitialized use occurs here
     196 |         return val;
         |                ^~~
   drivers/staging/rtl8723bs/hal/sdio_ops.c:189:3: note: remove the 'if' if its condition is always true
     189 |                 if (!err) {
         |                 ^~~~~~~~~
   drivers/staging/rtl8723bs/hal/sdio_ops.c:159:9: note: initialize the variable 'val' to silence this warning
     159 |         u32 val;
         |                ^
         |                 = 0
   1 warning generated.


vim +189 drivers/staging/rtl8723bs/hal/sdio_ops.c

   150	
   151	static u32 sdio_read32(struct intf_hdl *intfhdl, u32 addr)
   152	{
   153		struct adapter *adapter;
   154		u8 mac_pwr_ctrl_on;
   155		u8 device_id;
   156		u16 offset;
   157		u32 ftaddr;
   158		u8 shift;
   159		u32 val;
   160		s32 __maybe_unused err;
   161		__le32 le_tmp;
   162	
   163		adapter = intfhdl->padapter;
   164		ftaddr = _cvrt2ftaddr(addr, &device_id, &offset);
   165	
   166		rtw_hal_get_hwreg(adapter, HW_VAR_APFM_ON_MAC, &mac_pwr_ctrl_on);
   167		if (
   168			((device_id == WLAN_IOREG_DEVICE_ID) && (offset < 0x100)) ||
   169			(!mac_pwr_ctrl_on) ||
   170			(adapter_to_pwrctl(adapter)->fw_current_in_ps_mode)
   171		) {
   172			err = sd_cmd52_read(intfhdl, ftaddr, 4, (u8 *)&le_tmp);
   173			return le32_to_cpu(le_tmp);
   174		}
   175	
   176		/*  4 bytes alignment */
   177		shift = ftaddr & 0x3;
   178		if (shift == 0) {
   179			val = sd_read32(intfhdl, ftaddr, NULL);
   180		} else {
   181			u8 *tmpbuf;
   182	
   183			tmpbuf = rtw_malloc(8);
   184			if (!tmpbuf)
   185				return SDIO_ERR_VAL32;
   186	
   187			ftaddr &= ~(u16)0x3;
   188			err = sd_read(intfhdl, ftaddr, 8, tmpbuf);
 > 189			if (!err) {
   190				memcpy(&le_tmp, tmpbuf + shift, 4);
   191				val = le32_to_cpu(le_tmp);
   192			}
   193	
   194			kfree(tmpbuf);
   195		}
   196		return val;
   197	}
   198	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

On Thu, Apr 03, 2025 at 05:37:41PM +0800, Wentao Liang wrote:
> The sdio_read32() calls sd_read(), but does not handle the error if
> sd_read() fails. This could lead to subsequent operations processing
> invalid data. A proper implementation can be found in sdio_readN().
> 
> Add error handling to the sd_read(), ensuring that the memcpy() is
> only performed when the read operation is successful.
> 
> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  drivers/staging/rtl8723bs/hal/sdio_ops.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/hal/sdio_ops.c b/drivers/staging/rtl8723bs/hal/sdio_ops.c
> index 21e9f1858745..86776d924c15 100644
> --- a/drivers/staging/rtl8723bs/hal/sdio_ops.c
> +++ b/drivers/staging/rtl8723bs/hal/sdio_ops.c
> @@ -185,9 +185,11 @@ static u32 sdio_read32(struct intf_hdl *intfhdl, u32 addr)
>  			return SDIO_ERR_VAL32;
>  
>  		ftaddr &= ~(u16)0x3;
> -		sd_read(intfhdl, ftaddr, 8, tmpbuf);
> -		memcpy(&le_tmp, tmpbuf + shift, 4);
> -		val = le32_to_cpu(le_tmp);
> +		err = sd_read(intfhdl, ftaddr, 8, tmpbuf);
> +		if (!err) {
> +			memcpy(&le_tmp, tmpbuf + shift, 4);
> +			val = le32_to_cpu(le_tmp);

Now val isn't initialized on the error path.

regards,
dan carpenter