Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Sender: Guenter Roeck <groeck7@gmail.com>
Date:   Tue, 15 Nov 2022 05:42:13 -0800
From:   Guenter Roeck <linux@roeck-us.net>
To:     Li Hua <hucool.lihua@huawei.com>
Cc:     wim@linux-watchdog.org, linux-watchdog@vger.kernel.org,
        linux-kernel@vger.kernel.org, weiyongjun1@huawei.com,
        yusongping@huawei.com
Subject: Re: [PATCH] watchdog: pcwd_usb: Fix attempting to access
 uninitialized memory
Message-ID: <20221115134213.GD4189373@roeck-us.net>
References: <20221115083555.22928-1-hucool.lihua@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20221115083555.22928-1-hucool.lihua@huawei.com>
Precedence: bulk

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@huawei.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