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

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

[Marek Vasut]

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>
---
V2: Use dev_err() instead as WARN_ON() may panic() the kernel on existing machines
V3: Abort probe and return -EINVAL in case this condition is triggered
---
 drivers/iio/industrialio-buffer.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index b078eb2f3c9de..75a1c57b49102 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -1629,6 +1629,19 @@ 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);
+				ret = -EINVAL;
+				goto error_cleanup_dynamic;
+			}
+
 			ret = iio_buffer_add_channel_sysfs(indio_dev, buffer,
 							 &channels[i]);
 			if (ret < 0)
-- 
2.35.1

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

[Andy Shevchenko]

On Mon, Mar 28, 2022 at 09:53:07PM +0200, Marek Vasut 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.

Let's go with it.
Reviewed-by: Andy Shevchenko <andy@kernel.org>

> 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>
> ---
> V2: Use dev_err() instead as WARN_ON() may panic() the kernel on existing machines
> V3: Abort probe and return -EINVAL in case this condition is triggered
> ---
>  drivers/iio/industrialio-buffer.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index b078eb2f3c9de..75a1c57b49102 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -1629,6 +1629,19 @@ 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);
> +				ret = -EINVAL;
> +				goto error_cleanup_dynamic;
> +			}
> +
>  			ret = iio_buffer_add_channel_sysfs(indio_dev, buffer,
>  							 &channels[i]);
>  			if (ret < 0)
> -- 
> 2.35.1
> 

-- 
With Best Regards,
Andy Shevchenko

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

[Sa, Nuno]

> From: Marek Vasut <marex@denx.de>
> Sent: Monday, March 28, 2022 9:53 PM
> To: linux-iio@vger.kernel.org
> Cc: Marek Vasut <marex@denx.de>; Andy Shevchenko
> <andy@kernel.org>; Daniel Baluta <daniel.baluta@nxp.com>;
> Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Subject: [PATCH v3] iio: core: Print error in case sample bits do not fit
> storage bits
> 
> [External]
> 
> 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>
> ---
> V2: Use dev_err() instead as WARN_ON() may panic() the kernel on
> existing machines
> V3: Abort probe and return -EINVAL in case this condition is triggered
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/industrialio-buffer.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-
> buffer.c
> index b078eb2f3c9de..75a1c57b49102 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -1629,6 +1629,19 @@ 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);
> +				ret = -EINVAL;
> +				goto error_cleanup_dynamic;
> +			}
> +
>  			ret = iio_buffer_add_channel_sysfs(indio_dev,
> buffer,
>  							 &channels[i]);
>  			if (ret < 0)
> --
> 2.35.1

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

[Jonathan Cameron]

On Wed, 30 Mar 2022 17:52:13 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Mon, Mar 28, 2022 at 09:53:07PM +0200, Marek Vasut 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.  
> 
> Let's go with it.
> Reviewed-by: Andy Shevchenko <andy@kernel.org>
Agreed.  Applied with a slight change to the title to make it clear
we don't just print the error, we also fail the iio_device_register() call

Applied to the togreg branch of iio.git and pushed out as testing both to let
0-day poke at it and because I will rebase on rc1 once available.

Thanks,

Jonathan

> 
> > 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>
> > ---
> > V2: Use dev_err() instead as WARN_ON() may panic() the kernel on existing machines
> > V3: Abort probe and return -EINVAL in case this condition is triggered
> > ---
> >  drivers/iio/industrialio-buffer.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> > index b078eb2f3c9de..75a1c57b49102 100644
> > --- a/drivers/iio/industrialio-buffer.c
> > +++ b/drivers/iio/industrialio-buffer.c
> > @@ -1629,6 +1629,19 @@ 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);
> > +				ret = -EINVAL;
> > +				goto error_cleanup_dynamic;
> > +			}
> > +
> >  			ret = iio_buffer_add_channel_sysfs(indio_dev, buffer,
> >  							 &channels[i]);
> >  			if (ret < 0)
> > -- 
> > 2.35.1
> >   
>