linux-kernel - Re: [PATCH 08/10] iio: imu: adis16480: assign bias value only if operation succeeded

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

Date:   Sun, 3 Nov 2019 10:41:31 +0000
From:   Jonathan Cameron <jic23@...nel.org>
To:     Alexandru Ardelean <alexandru.ardelean@...log.com>
Cc:     <linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <lars@...afoo.de>, <Michael.Hennerich@...log.com>,
        <dragos.bogdan@...log.com>
Subject: Re: [PATCH 08/10] iio: imu: adis16480: assign bias value only if
 operation succeeded

On Fri, 1 Nov 2019 11:35:03 +0200
Alexandru Ardelean <alexandru.ardelean@...log.com> wrote:

> This was found only after the whole thing with the inline functions, but
> the compiler actually found something. The value of the `bias` (in
> adis16480_get_calibbias()) should only be set if the read operation was
> successful.
> 
> Fixes: 2f3abe6cbb6c9 ("iio:imu: Add support for the ADIS16480 and similar IMUs")
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@...log.com>

Hmm. It's not really a fix as such unless there is an in kernel consumer
that is using this and not checking the return value.   I thought about dropping
the fixes tag, but it is avoiding confusing the compiler so I suppose we might
as well leave it marked as a fix.  I will add a note that this doesn't cause
any known real problems and so probably isn't stable material.

Applied.

Thanks,

Jonathan

> ---
>  drivers/iio/imu/adis16480.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> index b9e2695bcfad..c0e7e768be41 100644
> --- a/drivers/iio/imu/adis16480.c
> +++ b/drivers/iio/imu/adis16480.c
> @@ -451,12 +451,14 @@ static int adis16480_get_calibbias(struct iio_dev *indio_dev,
>  	case IIO_MAGN:
>  	case IIO_PRESSURE:
>  		ret = adis_read_reg_16(&st->adis, reg, &val16);
> -		*bias = sign_extend32(val16, 15);
> +		if (ret == 0)
> +			*bias = sign_extend32(val16, 15);
>  		break;
>  	case IIO_ANGL_VEL:
>  	case IIO_ACCEL:
>  		ret = adis_read_reg_32(&st->adis, reg, &val32);
> -		*bias = sign_extend32(val32, 31);
> +		if (ret == 0)
> +			*bias = sign_extend32(val32, 31);
>  		break;
>  	default:
>  		ret = -EINVAL;