| Message ID | Z6DpZDJPDtiDzxDU@Emma (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] iio: light: Add check for array bounds in veml6075_read_int_time_ms | expand |
On Mon Feb 3, 2025 at 5:05 PM CET, Karan Sanghavi wrote: > The array contains only 5 elements, but the index calculated by > veml6075_read_int_time_index can range from 0 to 7, > which could lead to out-of-bounds access. The check prevents this issue. > > Coverity Issue > CID 1574309: (#1 of 1): Out-of-bounds read (OVERRUN) > overrun-local: Overrunning array veml6075_it_ms of 5 4-byte > elements at element index 7 (byte offset 31) using > index int_index (which evaluates to 7) > Hi Karan, You have dropped the Fixes: tag, please add it again. > Signed-off-by: Karan Sanghavi <karansanghvi98@gmail.com> > --- > drivers/iio/light/veml6075.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/light/veml6075.c b/drivers/iio/light/veml6075.c > index 05d4c0e9015d..21de193ca09d 100644 > --- a/drivers/iio/light/veml6075.c > +++ b/drivers/iio/light/veml6075.c > @@ -210,8 +210,8 @@ static int veml6075_read_int_time_ms(struct veml6075_data *data, int *val) > > guard(mutex)(&data->lock); > int_index = veml6075_read_int_time_index(data); > - if (int_index < 0) > - return int_index; > + if (int_index < 0 || int_index >= ARRAY_SIZE(veml6075_it_ms)) > + return -EINVAL; This approach modifies the original error codes that regmap_read() could return, which are not always -EINVAL. You could check the index within veml6075_read_int_time_index(), return -EINVAL, and this section would stay as it is. There is another call to the function in the driver, but the same approach would work there as well without modifications. > > *val = veml6075_it_ms[int_index]; > I would also like to remind you that giving some reasonable time between versions might let others review your patch. I am the maintainer of this driver, but no matter how many iterations we go through, the final decision is made by more qualified users i.e. subsystem maintainers. It would be nice if we could save them from a bunch of versions in a short period of time. Apart from what I pointed out, the patch looks better now. Thanks! Best regards, Javier Carrasco
diff --git a/drivers/iio/light/veml6075.c b/drivers/iio/light/veml6075.c index 05d4c0e9015d..21de193ca09d 100644 --- a/drivers/iio/light/veml6075.c +++ b/drivers/iio/light/veml6075.c @@ -210,8 +210,8 @@ static int veml6075_read_int_time_ms(struct veml6075_data *data, int *val) guard(mutex)(&data->lock); int_index = veml6075_read_int_time_index(data); - if (int_index < 0) - return int_index; + if (int_index < 0 || int_index >= ARRAY_SIZE(veml6075_it_ms)) + return -EINVAL; *val = veml6075_it_ms[int_index];
The array contains only 5 elements, but the index calculated by veml6075_read_int_time_index can range from 0 to 7, which could lead to out-of-bounds access. The check prevents this issue. Coverity Issue CID 1574309: (#1 of 1): Out-of-bounds read (OVERRUN) overrun-local: Overrunning array veml6075_it_ms of 5 4-byte elements at element index 7 (byte offset 31) using index int_index (which evaluates to 7) Signed-off-by: Karan Sanghavi <karansanghvi98@gmail.com> --- drivers/iio/light/veml6075.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)