Re: [PATCH] iio: imu: bmi323: Fix potential out-of-bounds access of bmi323_hw[]

[Jonathan Cameron <jic23@kernel.org>]

On Sat, 11 Apr 2026 14:28:07 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 3/27/26 5:32 AM, gerben@altlinux.org wrote:
> > From: Denis Rastyogin <gerben@altlinux.org>
> > 
> > The bmi323_channels[] array defines a channel with chan->type =
> > IIO_TEMP and enables the IIO_CHAN_INFO_SCALE mask. As a result,
> > bmi323_write_raw() may be called for this channel. However,
> > bmi323_iio_to_sensor() returns -EINVAL for IIO_TEMP, and if this
> > value is not validated, it can lead to an out-of-bounds access
> > when used as an array index.
> > 
> > A similar case is properly handled in bmi323_read_raw() and does
> > not result in an error.
> > 
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
Good catch.

> > 
> > Fixes: 8a636db3aa57 ("iio: imu: Add driver for BMI323 IMU")
> > Signed-off-by: Denis Rastyogin <gerben@altlinux.org>
> > ---
> >  drivers/iio/imu/bmi323/bmi323_core.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/iio/imu/bmi323/bmi323_core.c b/drivers/iio/imu/bmi323/bmi323_core.c
> > index 6bcb9a436581..64ead4f667e0 100644
> > --- a/drivers/iio/imu/bmi323/bmi323_core.c
> > +++ b/drivers/iio/imu/bmi323/bmi323_core.c
> > @@ -1713,6 +1713,8 @@ static int bmi323_write_raw(struct iio_dev *indio_dev,
> >  		iio_device_release_direct(indio_dev);
> >  		return ret;
> >  	case IIO_CHAN_INFO_SCALE:
> > +		if (chan->type == IIO_TEMP)
> > +			return -EINVAL;

Please flip this around as we want support for this write to be opt in rather
than opt out.
		if (chan->type != IIO_ACCEL &&
		    chan->type != IIO_ANGL_VEL)

Alternative below is better though as that is opt in anyway.


> >  		if (!iio_device_claim_direct(indio_dev))
> >  			return -EBUSY;
> >  		ret = bmi323_set_scale(data, bmi323_iio_to_sensor(chan->type),  
> 
> This is OK, but why not check and propagate the error return?
> 
>  	case IIO_CHAN_INFO_SCALE:
> 		ret = bmi323_iio_to_sensor(chan->type);
> 		if (ret < 0)
> 			return ret;
> 
>   		if (!iio_device_claim_direct(indio_dev))
>   			return -EBUSY;
>   		ret = bmi323_set_scale(data, ret, val, val2);
> 		...
> 
> And even if we shouldn't hit the error in other case statements,
> it seems like it would be good practice to still check for error
> there too.
> 
>