linux-kernel - re: iio:adc:ad7280a: Move out of staging

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

Message-ID: <69aeef03-69ba-07f4-2506-31e481f86c76@gmail.com>
Date:   Thu, 24 Feb 2022 22:56:20 +0000
From:   "Colin King (gmail)" <colin.i.king@...il.com>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     Marcelo Schmitt <marcelo.schmitt1@...il.com>,
        linux-iio@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: re: iio:adc:ad7280a: Move out of staging

Hi,

Static analysis with clang scan picked up a potential issue with 
drivers/iio/adc/ad7280a.c in function ad7280a_write_thresh, the analysis 
is as follows:

         switch (chan->type) {
         case IIO_VOLTAGE:
                 value = ((val - 1000) * 100) / 1568; /* LSB 15.68mV */
                 value = clamp(value, 0L, 0xFFL);

                 ^^
Note: variable value is being assigned a value

                 switch (dir) {
                 case IIO_EV_DIR_RISING:
                         addr = AD7280A_CELL_OVERVOLTAGE_REG;
                         ret = ad7280_write(st, AD7280A_DEVADDR_MASTER, 
addr,
                                            1, val);
                         if (ret)
                                 break;
                         st->cell_threshhigh = value;

..and value is being used here ^^

                         break;
                 case IIO_EV_DIR_FALLING:
                         addr = AD7280A_CELL_UNDERVOLTAGE_REG;
                         ret = ad7280_write(st, AD7280A_DEVADDR_MASTER, 
addr,
                                            1, val);
                         if (ret)
                                 break;
                         st->cell_threshlow = value;

and value is being used here ^^

                         break;
                 default:
                         ret = -EINVAL;
                         goto err_unlock;
                 }
                 break;

However for the IIO_TEMP case:

         case IIO_TEMP:
                 value = (val * 10) / 196; /* LSB 19.6mV */
                 value = clamp(value, 0L, 0xFFL);

                 ^^
Note: variable value is being assigned a value

                 switch (dir) {
                 case IIO_EV_DIR_RISING:
                         addr = AD7280A_AUX_ADC_OVERVOLTAGE_REG;
                         ret = ad7280_write(st, AD7280A_DEVADDR_MASTER, 
addr,
                                            1, val);
                         if (ret)
                                 break;
                         st->aux_threshhigh = val;
                                              ^^
But val is being used here rather than value

                         break;
                 case IIO_EV_DIR_FALLING:
                         addr = AD7280A_AUX_ADC_UNDERVOLTAGE_REG;
                         ret = ad7280_write(st, AD7280A_DEVADDR_MASTER, 
addr,
                                            1, val);
                         if (ret)
                                 break;
                         st->aux_threshlow = val;
                                             ^^
and val us being used here rather than value too


So for the IIO_TEMP case either the assignment to value is redundant or 
the setting of st->aux_threshhigh or st->auxthreashlow is incorrect.

Colin