linux-kernel - Re: [PATCH] watchdog: pcwd_usb: Fix attempting to access uninitialized memory

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

Message-ID: <20221115134213.GD4189373@roeck-us.net>
Date:   Tue, 15 Nov 2022 05:42:13 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Li Hua <hucool.lihua@...wei.com>
Cc:     wim@...ux-watchdog.org, linux-watchdog@...r.kernel.org,
        linux-kernel@...r.kernel.org, weiyongjun1@...wei.com,
        yusongping@...wei.com
Subject: Re: [PATCH] watchdog: pcwd_usb: Fix attempting to access
 uninitialized memory

On Tue, Nov 15, 2022 at 04:35:55PM +0800, Li Hua wrote:
> The stack variable msb and lsb may be used uninitialized in function
> usb_pcwd_get_temperature and usb_pcwd_get_timeleft when usb card no response.
> 
> The build waring is:
> drivers/watchdog/pcwd_usb.c:336:22: error: ‘lsb’ is used uninitialized in this function [-Werror=uninitialized]
>   *temperature = (lsb * 9 / 5) + 32;
>                   ~~~~^~~
> drivers/watchdog/pcwd_usb.c:328:21: note: ‘lsb’ was declared here
>   unsigned char msb, lsb;
>                      ^~~
> cc1: all warnings being treated as errors
> scripts/Makefile.build:250: recipe for target 'drivers/watchdog/pcwd_usb.o' failed
> make[3]: *** [drivers/watchdog/pcwd_usb.o] Error 1
> 
> Fixes: b7e04f8c61a4 ("mv watchdog tree under drivers")
> Signed-off-by: Li Hua <hucool.lihua@...wei.com>
> ---
>  drivers/watchdog/pcwd_usb.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/pcwd_usb.c b/drivers/watchdog/pcwd_usb.c
> index 1bdaf17c1d38..89b0b5d8a7e6 100644
> --- a/drivers/watchdog/pcwd_usb.c
> +++ b/drivers/watchdog/pcwd_usb.c
> @@ -326,8 +326,13 @@ static int usb_pcwd_get_temperature(struct usb_pcwd_private *usb_pcwd,
>  							int *temperature)
>  {
>  	unsigned char msb, lsb;
> +	int retval;
>  
> -	usb_pcwd_send_command(usb_pcwd, CMD_READ_TEMP, &msb, &lsb);
> +	retval = usb_pcwd_send_command(usb_pcwd, CMD_READ_TEMP, &msb, &lsb);
> +	if (retval != 1) {
> +		pr_err("Card did not acknowledge CMD_READ_TEMP\n");
> +		return -1;
> +	}

That all isn't really worth the trouble. If anyone still has the hardware,
the driver should be converted to use the watchdog subsystem and to return
useful error codes where appropriate. For example, returning -EFAULT as
response to errors from WDIOC_GETTIMELEFT or WDIOC_GETTEMP is just wrong.
If you really want to do anything, just initialize lsb and msb with 0.

Guenter