On Sat, Oct 19, 2024 at 12:21:33PM +0100, Jonathan Cameron wrote: > On Fri, 18 Oct 2024 18:54:42 +0000 > Karan Sanghavi <karansanghvi98@xxxxxxxxx> wrote: > > > Add a Null pointer check before assigning and incrementing > > the null pointer > > > > Signed-off-by: Karan Sanghavi <karansanghvi98@xxxxxxxxx> > > It would be a bug if rsp_size was anything other than 0 and rsp is NULL. > So this looks like a false positive as the loop will never be > entered. > > How did you find it, in particular have you managed to trigger this > in the driver? > > Jonathan > > I found this bug in Coverity scan with Cid: 1504707. Link below, for the same. https://scan7.scan.coverity.com/#/project-view/51946/11354?selectedIssue=1504707 Rsp here is a void pointer received from the function arguments which can be NULL for a no respone call. Thus incrementing the NULL pointer can lead to some unexpected behavior which cross my mind thus added the check. > > --- > > drivers/iio/chemical/sps30_i2c.c | 20 +++++++++++--------- > > 1 file changed, 11 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/iio/chemical/sps30_i2c.c b/drivers/iio/chemical/sps30_i2c.c > > index 1b21b6bcd0e7..d2142e4c748c 100644 > > --- a/drivers/iio/chemical/sps30_i2c.c > > +++ b/drivers/iio/chemical/sps30_i2c.c > > @@ -105,16 +105,18 @@ static int sps30_i2c_command(struct sps30_state *state, u16 cmd, void *arg, size > > return ret; > > > > /* validate received data and strip off crc bytes */ > > - tmp = rsp; > > - for (i = 0; i < rsp_size; i += 3) { > > - crc = crc8(sps30_i2c_crc8_table, buf + i, 2, CRC8_INIT_VALUE); > > - if (crc != buf[i + 2]) { > > - dev_err(state->dev, "data integrity check failed\n"); > > - return -EIO; > > + if (rsp) { > > + tmp = rsp; > > + for (i = 0; i < rsp_size; i += 3) { > > + crc = crc8(sps30_i2c_crc8_table, buf + i, 2, CRC8_INIT_VALUE); > > + if (crc != buf[i + 2]) { > > + dev_err(state->dev, "data integrity check failed\n"); > > + return -EIO; > > + } > > + > > + *tmp++ = buf[i]; > > + *tmp++ = buf[i + 1]; > > } > > - > > - *tmp++ = buf[i]; > > - *tmp++ = buf[i + 1]; > > } > > > > return 0; > > > > --- > > base-commit: f2493655d2d3d5c6958ed996b043c821c23ae8d3 > > change-id: 20241018-cid1593398badshift-9c512a3b92d9 > > > > Best regards, > Thank you, Karan.
