Re: [PATCH v2] iio: core: Print error in case sample bits do not fit storage bits

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Tue, 22 Mar 2022 12:16:19 +0100
Marek Vasut <marex@denx.de> wrote:

> Add runtime check to verify whether storagebits are at least as big
> as shifted realbits. This should help spot broken drivers which may
> set realbits + shift above storagebits.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Andy Shevchenko <andy@kernel.org>
> Cc: Daniel Baluta <daniel.baluta@nxp.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Hmm. I was thinking we'd fail the probe if this happens,
though I guess there might be cases where we get away 
(in kernel anyway) with a driver setting this wrong as
many drivers don't use realbits internally in an explicit
fashion, so maybe a message and skipping the channel is
the right choice...

Userspace running against such a description is likely
to generate garbage though unless it's very lucky and
the spill past storage bits is into padding space and
the driver doesn't put anything in there (padding might
contain old data or similar).

Either way it's a definite improvement so I'm probably fine
with the message and not failing the probe, (though will
think a bit more about it before picking this up.)


Jonathan`


> ---
> V2: Use dev_err() instead as WARN_ON() may panic() the kernel on existing machines
> ---
>  drivers/iio/industrialio-buffer.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index b078eb2f3c9de..b5670398b06d7 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -1629,6 +1629,18 @@ static int __iio_buffer_alloc_sysfs_and_mask(struct iio_buffer *buffer,
>  			if (channels[i].scan_index < 0)
>  				continue;
>  
> +			/* Verify that sample bits fit into storage */
> +			if (channels[i].scan_type.storagebits <
> +			    channels[i].scan_type.realbits +
> +			    channels[i].scan_type.shift) {
> +				dev_err(&indio_dev->dev,
> +					"Channel %d storagebits (%d) < shifted realbits (%d + %d)\n",
> +					i, channels[i].scan_type.storagebits,
> +					channels[i].scan_type.realbits,
> +					channels[i].scan_type.shift);
> +				continue;
> +			}
> +
>  			ret = iio_buffer_add_channel_sysfs(indio_dev, buffer,
>  							 &channels[i]);
>  			if (ret < 0)