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

Hi Wentao,

Thanks for your patch!

On Fri, 16 May 2025 at 10:39, Wentao Liang vulab@iscas.ac.cn 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().

There is no need to print an error message, as wilc_sdio_cmd52() already does that. Same with the existing wilc_sdio_read_reg(), and with all of its callers, which leads to multiple error messages for a single failure.

Fixes: 0e1af73ddeb9 ("staging/wilc1000: use proper naming for global symbols")

This is not the commit you are looking for...

Cc: stable@vger.kernel.org # v4.15 Signed-off-by: Wentao Liang vulab@iscas.ac.cn

--- a/drivers/net/wireless/microchip/wilc1000/sdio.c +++ b/drivers/net/wireless/microchip/wilc1000/sdio.c @@ -771,6 +771,8 @@ static int wilc_sdio_read_size(struct wilc *wilc, u32 *size) { u32 tmp; struct sdio_cmd52 cmd;

•   struct sdio_func *func = dev_to_sdio_func(wilc->dev);
  

•   int ret;
  
    /**
     *      Read DMA count in words
  

@@ -780,12 +782,20 @@ 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) {
  

•           dev_err(&func->devm, "Fail cmd 52, interrupt data register...\n");
  

Looks like the AI wasn't trained properly. Please try to (at least) test-compile your patches.

•           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) {
  

•           dev_err(&func->devm, "Fail cmd 52, interrupt data register...\n");
  

Likewise.

•           return ret;
  

•   }
    tmp |= (cmd.data << 8);
  
    *size = tmp;
  

Gr{oetje,eeting}s,

Geert