On 10/19/24 06:08, Karan Sanghavi wrote:
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.
This routine checks rsp in the earlier logic
if (rsp) {
/* each two bytes are followed by a crc8 */
rsp_size += rsp_size / 2;
} else {
tmp = arg;
while (arg_size) {
buf[i] = *tmp++;
buf[i + 1] = *tmp++;
buf[i + 2] = crc8(sps30_i2c_crc8_table, buf + i, 2, CRC8_INIT_VALUE);
arg_size -= 2;
i += 3;
}
}
ret = sps30_i2c_xfer(state, buf, i, buf, rsp_size);
if (ret)
return ret;
Looks like the tmp = rsp; could be reached depending on the
sps30_i2c_xfer() return value?
Maybe this isn't the right fix but looks like the code could
use looking into for accuracy.
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.
thanks,
-- Shuah
