linux-kernel - Re: [PATCH v1 2/9] iio: inkern: error out on unsupported offset type

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]

Message-ID: <7e6e7e30-80b6-adfd-abda-7c9ac5333a47@axentia.se>
Date:   Mon, 31 May 2021 11:45:10 +0200
From:   Peter Rosin <peda@...ntia.se>
To:     Liam Beguin <liambeguin@...il.com>, jic23@...nel.org,
        lars@...afoo.de, pmeerw@...erw.net
Cc:     linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org,
        devicetree@...r.kernel.org, robh+dt@...nel.org
Subject: Re: [PATCH v1 2/9] iio: inkern: error out on unsupported offset type

Hi!

On 2021-05-30 02:59, Liam Beguin wrote:
> From: Liam Beguin <lvb@...hos.com>
> 
> iio_convert_raw_to_processed_unlocked() assumes the offset is an
> integer. Make that clear to the consumer by returning an error when an
> unsupported offset type is detected.
> 
> Signed-off-by: Liam Beguin <lvb@...hos.com>
> ---
>  drivers/iio/inkern.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 4b6a8e11116a..dede4536d499 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -595,8 +595,12 @@ static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan,
>  	int ret;
>  
>  	ret = iio_channel_read(chan, &offset, NULL, IIO_CHAN_INFO_OFFSET);
> -	if (ret >= 0)
> +	if (ret == IIO_VAL_INT) {
>  		raw64 += offset;
> +	} else if (ret >= 0) {
> +		dev_err(&chan->indio_dev->dev, "unsupported offset type");
> +		return -EINVAL;
> +	}
>  
>  	scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
>  					IIO_CHAN_INFO_SCALE);
> 

This breaks the implicit truncation that happens for drivers that have
offsets of type IIO_VAL_INT_PLUS_{MICRO_DB,MICRO,NANO}

Implicit truncation might be more appropriate than an error?

However, to error out on fractional offsets etc still seem appropriate, but
there are corner cases where the existing code did the right thing. E.g.
a denominator of one or a fractional-log2 of zero, but a big denominator and
a smaller numerator would also just result in a relatively harmless truncation.

I don't know if it's really right to just break that?

Cheers,
Peter