watchdog: pcwd_usb: Fix attempting to access uninitialized memory

Message ID 20221115083555.22928-1-hucool.lihua@huawei.com
State New
Headers
Series watchdog: pcwd_usb: Fix attempting to access uninitialized memory |

Commit Message

Lihua (lihua, ran) Nov. 15, 2022, 8:35 a.m. UTC
  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

Guenter Roeck Nov. 15, 2022, 1:42 p.m. UTC | #1
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
  

Patch

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;
+	}
 
 	/*
 	 * 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;