Re: [PATCH] iio: chemical: scd30: avoid potential NULL deref in scd30_i2c_command()

[Jonathan Cameron <jic23@kernel.org>]

On Wed,  6 May 2026 23:15:33 +0500
Stepan Ionichev <sozdayvek@gmail.com> wrote:

> scd30_i2c_command() takes an opaque "response" buffer plus its size.
> At the start of the function the code already checks if response is
> NULL (via the rsp local), but the response-decoding loop after the
> i2c transfer always dereferences rsp without re-checking.
> 
> With the current callers in scd30_core.c this is harmless, since
> write commands pass response=NULL together with size=0 (so the loop
> body is never entered). However, the inconsistency is an accident
> waiting to happen if a future caller passes response=NULL together
> with size > 0 -- the loop would then write through a NULL pointer.
> 
> smatch flags this:
> 
>   drivers/iio/chemical/scd30_i2c.c:104 scd30_i2c_command() error: we
>     previously assumed rsp could be null (see line 77)
> 
> Bail out early when rsp is NULL so the function is robust regardless
> of the (cmd, size) combination chosen by the caller.
> 
> No functional change for the existing callers.
> 
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
Hi Stephan

Thanks for the analysis - as you say no actual bug here but good
to make that more obvious to static analzers.

If we ever did hit this I think it would be better to return an
error code.  I'd also do it at the top given such a combination doesn't
make sense so we should exclude it early.

if (!response && size != 0)
	return -EINVAL;


> ---
>  drivers/iio/chemical/scd30_i2c.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/iio/chemical/scd30_i2c.c b/drivers/iio/chemical/scd30_i2c.c
> index 436df9c61..fb06bec75 100644
> --- a/drivers/iio/chemical/scd30_i2c.c
> +++ b/drivers/iio/chemical/scd30_i2c.c
> @@ -93,6 +93,9 @@ static int scd30_i2c_command(struct scd30_state *state, enum scd30_cmd cmd, u16
>  	if (ret)
>  		return ret;
>  
> +	if (!rsp)
> +		return 0;
> +
>  	/* validate received data and strip off crc bytes */
>  	for (i = 0; i < size; i += 3) {
>  		crc = crc8(scd30_i2c_crc8_tbl, buf + i, 2, CRC8_INIT_VALUE);