linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iio: cros_ec_sensors_core: fix unsigned compared less than zero on status
@ 2016-11-09 23:12 Colin King
  2016-11-10  3:16 ` Guenter Roeck
  0 siblings, 1 reply; 3+ messages in thread
From: Colin King @ 2016-11-09 23:12 UTC (permalink / raw)
  To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Guenter Roeck, Gwendal Grignou,
	Enric Balletbo i Serra, linux-iio
  Cc: linux-kernel

From: Colin Ian King <colin.king@canonical.com>

status is a u8 hence the check if status is less than zero has no effect.
Fix this by replacing status with int ret so the less than zero compare
will correctly detect errors.

Issue found with static analysis with CoverityScan, CID 1375919

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
index a3be799..416cae5 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -291,15 +291,15 @@ int cros_ec_sensors_read_lpc(struct iio_dev *indio_dev,
 			return -EIO;
 
 		/* Read status byte until EC is not busy. */
-		status = cros_ec_sensors_read_until_not_busy(st);
-		if (status < 0)
-			return status;
+		ret = cros_ec_sensors_read_until_not_busy(st);
+		if (ret < 0)
+			return ret;
 
 		/*
 		 * Store the current sample id so that we can compare to the
 		 * sample id after reading the data.
 		 */
-		samp_id = status & EC_MEMMAP_ACC_STATUS_SAMPLE_ID_MASK;
+		samp_id = ret & EC_MEMMAP_ACC_STATUS_SAMPLE_ID_MASK;
 
 		/* Read all EC data, format it, and store it into data. */
 		ret = cros_ec_sensors_read_data_unsafe(indio_dev, scan_mask,
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] iio: cros_ec_sensors_core: fix unsigned compared less than zero on status
  2016-11-09 23:12 [PATCH] iio: cros_ec_sensors_core: fix unsigned compared less than zero on status Colin King
@ 2016-11-10  3:16 ` Guenter Roeck
  2016-11-12 14:40   ` Jonathan Cameron
  0 siblings, 1 reply; 3+ messages in thread
From: Guenter Roeck @ 2016-11-10  3:16 UTC (permalink / raw)
  To: Colin King
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Guenter Roeck, Gwendal Grignou,
	Enric Balletbo i Serra, linux-iio, linux-kernel

On Wed, Nov 9, 2016 at 3:12 PM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> status is a u8 hence the check if status is less than zero has no effect.
> Fix this by replacing status with int ret so the less than zero compare
> will correctly detect errors.
>
> Issue found with static analysis with CoverityScan, CID 1375919
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Good catch!

Fixes: 974e6f02e27e ("iio: cros_ec_sensors_core: Add common functions
for the ChromeOS EC Sensor Hub")
Reviewed-by: Guenter Roeck <groeck@chromium.org>

For the benefit of others, this is in today's linux-next.

Thanks,
Guenter

> ---
>  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> index a3be799..416cae5 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> @@ -291,15 +291,15 @@ int cros_ec_sensors_read_lpc(struct iio_dev *indio_dev,
>                         return -EIO;
>
>                 /* Read status byte until EC is not busy. */
> -               status = cros_ec_sensors_read_until_not_busy(st);
> -               if (status < 0)
> -                       return status;
> +               ret = cros_ec_sensors_read_until_not_busy(st);
> +               if (ret < 0)
> +                       return ret;
>
>                 /*
>                  * Store the current sample id so that we can compare to the
>                  * sample id after reading the data.
>                  */
> -               samp_id = status & EC_MEMMAP_ACC_STATUS_SAMPLE_ID_MASK;
> +               samp_id = ret & EC_MEMMAP_ACC_STATUS_SAMPLE_ID_MASK;
>
>                 /* Read all EC data, format it, and store it into data. */
>                 ret = cros_ec_sensors_read_data_unsafe(indio_dev, scan_mask,
> --
> 2.10.2
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] iio: cros_ec_sensors_core: fix unsigned compared less than zero on status
  2016-11-10  3:16 ` Guenter Roeck
@ 2016-11-12 14:40   ` Jonathan Cameron
  0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2016-11-12 14:40 UTC (permalink / raw)
  To: Guenter Roeck, Colin King
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Guenter Roeck, Gwendal Grignou, Enric Balletbo i Serra, linux-iio,
	linux-kernel

On 10/11/16 03:16, Guenter Roeck wrote:
> On Wed, Nov 9, 2016 at 3:12 PM, Colin King <colin.king@canonical.com> wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> status is a u8 hence the check if status is less than zero has no effect.
>> Fix this by replacing status with int ret so the less than zero compare
>> will correctly detect errors.
>>
>> Issue found with static analysis with CoverityScan, CID 1375919
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> 
> Good catch!
> 
> Fixes: 974e6f02e27e ("iio: cros_ec_sensors_core: Add common functions
> for the ChromeOS EC Sensor Hub")
> Reviewed-by: Guenter Roeck <groeck@chromium.org>
Thanks and applied to the togreg branch of iio.git - shortly to be
pushed out as testing for the autobuilders to take a look at it.

Jonathan
> 
> For the benefit of others, this is in today's linux-next.
> 
> Thanks,
> Guenter
> 
>> ---
>>  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>> index a3be799..416cae5 100644
>> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>> @@ -291,15 +291,15 @@ int cros_ec_sensors_read_lpc(struct iio_dev *indio_dev,
>>                         return -EIO;
>>
>>                 /* Read status byte until EC is not busy. */
>> -               status = cros_ec_sensors_read_until_not_busy(st);
>> -               if (status < 0)
>> -                       return status;
>> +               ret = cros_ec_sensors_read_until_not_busy(st);
>> +               if (ret < 0)
>> +                       return ret;
>>
>>                 /*
>>                  * Store the current sample id so that we can compare to the
>>                  * sample id after reading the data.
>>                  */
>> -               samp_id = status & EC_MEMMAP_ACC_STATUS_SAMPLE_ID_MASK;
>> +               samp_id = ret & EC_MEMMAP_ACC_STATUS_SAMPLE_ID_MASK;
>>
>>                 /* Read all EC data, format it, and store it into data. */
>>                 ret = cros_ec_sensors_read_data_unsafe(indio_dev, scan_mask,
>> --
>> 2.10.2
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-11-12 14:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-09 23:12 [PATCH] iio: cros_ec_sensors_core: fix unsigned compared less than zero on status Colin King
2016-11-10  3:16 ` Guenter Roeck
2016-11-12 14:40   ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).