linux-kernel - Re: [PATCH] wifi: wilc1000: Add error handling for wilc_sdio

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>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMuHMdV=96tSH77P9Gqr1M3VfiKB9jYCeJDDFCNq_TGdYb0vFQ@mail.gmail.com>
Date: Mon, 19 May 2025 09:32:51 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Wentao Liang <vulab@...as.ac.cn>
Cc: ajay.kathat@...rochip.com, claudiu.beznea@...on.dev, kvalo@...nel.org, 
	linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org, 
	stable@...r.kernel.org
Subject: 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@...as.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@...r.kernel.org # v4.15
> Signed-off-by: Wentao Liang <vulab@...as.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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds