[PATCH v3] wifi: wilc1000: Add error handling for wilc_sdio_cmd52()

[PATCH v3] wifi: wilc1000: Add error handling for wilc_sdio_cmd52()

[Wentao Liang]

The wilc_sdio_read_size() calls wilc_sdio_cmd52() but does not check the
return value. This could lead to execution with potentially invalid data
if wilc_sdio_cmd52() fails. A proper implementation can be found in
wilc_sdio_read_reg().

Add error handling for wilc_sdio_cmd52(). If wilc_sdio_cmd52() fails,
log an error message via dev_err().

Fixes: ea5779b4fbc7 ("staging: wilc1000: wilc_sdio_cmd52: pass struct wilc")
Cc: stable@vger.kernel.org # v4.5
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
v3: Remove redundant error log. Fix code error. Fix fixes flag error.
v2: Fix code error.

 drivers/net/wireless/microchip/wilc1000/sdio.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c b/drivers/net/wireless/microchip/wilc1000/sdio.c
index 5262c8846c13..d77f88996250 100644
--- a/drivers/net/wireless/microchip/wilc1000/sdio.c
+++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
@@ -771,6 +771,7 @@ static int wilc_sdio_read_size(struct wilc *wilc, u32 *size)
 {
 	u32 tmp;
 	struct sdio_cmd52 cmd;
+	int ret;
 
 	/**
 	 *      Read DMA count in words
@@ -780,12 +781,16 @@ static int wilc_sdio_read_size(struct wilc *wilc, u32 *size)
 	cmd.raw = 0;
 	cmd.address = WILC_SDIO_INTERRUPT_DATA_SZ_REG;
 	cmd.data = 0;
-	wilc_sdio_cmd52(wilc, &cmd);
+	ret = wilc_sdio_cmd52(wilc, &cmd);
+	if (ret)
+		return ret;
 	tmp = cmd.data;
 
 	cmd.address = WILC_SDIO_INTERRUPT_DATA_SZ_REG + 1;
 	cmd.data = 0;
-	wilc_sdio_cmd52(wilc, &cmd);
+	ret = wilc_sdio_cmd52(wilc, &cmd);
+	if (ret)
+		return ret;
 	tmp |= (cmd.data << 8);
 
 	*size = tmp;
-- 
2.42.0.windows.2

Re: [PATCH v3] wifi: wilc1000: Add error handling for wilc_sdio_cmd52()

[Johannes Berg]

On Mon, 2025-05-19 at 16:42 +0800, Wentao Liang wrote:
> The wilc_sdio_read_size() calls wilc_sdio_cmd52() but does not check the
> return value. This could lead to execution with potentially invalid data
> if wilc_sdio_cmd52() fails. A proper implementation can be found in
> wilc_sdio_read_reg().
> 
> Add error handling for wilc_sdio_cmd52(). If wilc_sdio_cmd52() fails,
> log an error message via dev_err().

That's clearly hno longer true, please submit consistent patches.

Also, this change (at best!) addresses half the problem, please fix it
fully if you want to bother us with it at all.

johannes