hwmon: (applesmc) check status earlier.
authorTom Rix <trix@redhat.com>
Thu, 20 Aug 2020 13:19:32 +0000 (06:19 -0700)
committerGuenter Roeck <linux@roeck-us.net>
Fri, 21 Aug 2020 18:32:34 +0000 (11:32 -0700)
clang static analysis reports this representative problem

applesmc.c:758:10: warning: 1st function call argument is an
  uninitialized value
        left = be16_to_cpu(*(__be16 *)(buffer + 6)) >> 2;
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

buffer is filled by the earlier call

ret = applesmc_read_key(LIGHT_SENSOR_LEFT_KEY, ...

This problem is reported because a goto skips the status check.
Other similar problems use data from applesmc_read_key before checking
the status.  So move the checks to before the use.

Signed-off-by: Tom Rix <trix@redhat.com>
Reviewed-by: Henrik Rydberg <rydberg@bitmath.org>
Link: https://lore.kernel.org/r/20200820131932.10590-1-trix@redhat.com
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
drivers/hwmon/applesmc.c

index 3166184..a188879 100644 (file)
@@ -753,15 +753,18 @@ static ssize_t applesmc_light_show(struct device *dev,
        }
 
        ret = applesmc_read_key(LIGHT_SENSOR_LEFT_KEY, buffer, data_length);
+       if (ret)
+               goto out;
        /* newer macbooks report a single 10-bit bigendian value */
        if (data_length == 10) {
                left = be16_to_cpu(*(__be16 *)(buffer + 6)) >> 2;
                goto out;
        }
        left = buffer[2];
+
+       ret = applesmc_read_key(LIGHT_SENSOR_RIGHT_KEY, buffer, data_length);
        if (ret)
                goto out;
-       ret = applesmc_read_key(LIGHT_SENSOR_RIGHT_KEY, buffer, data_length);
        right = buffer[2];
 
 out:
@@ -810,12 +813,11 @@ static ssize_t applesmc_show_fan_speed(struct device *dev,
                  to_index(attr));
 
        ret = applesmc_read_key(newkey, buffer, 2);
-       speed = ((buffer[0] << 8 | buffer[1]) >> 2);
-
        if (ret)
                return ret;
-       else
-               return snprintf(sysfsbuf, PAGE_SIZE, "%u\n", speed);
+
+       speed = ((buffer[0] << 8 | buffer[1]) >> 2);
+       return snprintf(sysfsbuf, PAGE_SIZE, "%u\n", speed);
 }
 
 static ssize_t applesmc_store_fan_speed(struct device *dev,
@@ -851,12 +853,11 @@ static ssize_t applesmc_show_fan_manual(struct device *dev,
        u8 buffer[2];
 
        ret = applesmc_read_key(FANS_MANUAL, buffer, 2);
-       manual = ((buffer[0] << 8 | buffer[1]) >> to_index(attr)) & 0x01;
-
        if (ret)
                return ret;
-       else
-               return snprintf(sysfsbuf, PAGE_SIZE, "%d\n", manual);
+
+       manual = ((buffer[0] << 8 | buffer[1]) >> to_index(attr)) & 0x01;
+       return snprintf(sysfsbuf, PAGE_SIZE, "%d\n", manual);
 }
 
 static ssize_t applesmc_store_fan_manual(struct device *dev,
@@ -872,10 +873,11 @@ static ssize_t applesmc_store_fan_manual(struct device *dev,
                return -EINVAL;
 
        ret = applesmc_read_key(FANS_MANUAL, buffer, 2);
-       val = (buffer[0] << 8 | buffer[1]);
        if (ret)
                goto out;
 
+       val = (buffer[0] << 8 | buffer[1]);
+
        if (input)
                val = val | (0x01 << to_index(attr));
        else
@@ -951,13 +953,12 @@ static ssize_t applesmc_key_count_show(struct device *dev,
        u32 count;
 
        ret = applesmc_read_key(KEY_COUNT_KEY, buffer, 4);
-       count = ((u32)buffer[0]<<24) + ((u32)buffer[1]<<16) +
-                                               ((u32)buffer[2]<<8) + buffer[3];
-
        if (ret)
                return ret;
-       else
-               return snprintf(sysfsbuf, PAGE_SIZE, "%d\n", count);
+
+       count = ((u32)buffer[0]<<24) + ((u32)buffer[1]<<16) +
+                                               ((u32)buffer[2]<<8) + buffer[3];
+       return snprintf(sysfsbuf, PAGE_SIZE, "%d\n", count);
 }
 
 static ssize_t applesmc_key_at_index_read_show(struct device *dev,