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

["Nuno Sá" <noname.nuno@gmail.com>]

On Tue, 2022-03-22 at 21:42 +0000, Jonathan Cameron wrote:
> 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`
> 

FWIW, if we assume we are ok with potentially some drivers starting to
fail probe, I'm +1 on this should fail probe...

- Nuno Sá
> 
> > ---
> > 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.realb
> > its,
> > +                                       channels[i].scan_type.shift
> > );
> > +                               continue;
> > +                       }
> > +
> >                         ret =
> > iio_buffer_add_channel_sysfs(indio_dev, buffer,
> >                                                         
> > &channels[i]);
> >                         if (ret < 0)
>