watchdog: pcwd_usb: Fix attempting to access uninitialized memory
Commit Message
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(-)
Comments
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
@@ -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;
+ }
/*
* Convert celsius to fahrenheit, since this was
@@ -342,10 +347,16 @@ static int usb_pcwd_get_timeleft(struct usb_pcwd_private *usb_pcwd,
int *time_left)
{
unsigned char msb, lsb;
+ int retval;
/* Read the time that's left before rebooting */
/* Note: if the board is not yet armed then we will read 0xFFFF */
- usb_pcwd_send_command(usb_pcwd, CMD_READ_WATCHDOG_TIMEOUT, &msb, &lsb);
+ retval = usb_pcwd_send_command(usb_pcwd, CMD_READ_WATCHDOG_TIMEOUT,
+ &msb, &lsb);
+ if (retval != 1) {
+ pr_err("Card did not acknowledge CMD_READ_WATCHDOG_TIMEOUT\n");
+ return -1;
+ }
*time_left = (msb << 8) + lsb;