Re: [PATCH v2] iio: chemical: scd30: reject (response=NULL, size>0) in scd30_i2c_command()
From: Andy Shevchenko
Date: Fri May 08 2026 - 03:42:20 EST
On Thu, May 07, 2026 at 08:28:00PM +0500, Stepan Ionichev 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).
>
> The (response=NULL, size>0) combination has no useful meaning: there
> is nowhere to put the bytes that come back from the chip. Treat it
> as an invalid argument and bail out at the top of the function with
> -EINVAL, instead of silently doing the i2c transfer and dereferencing
> a NULL pointer in the decode loop.
>
> smatch flagged the inconsistency:
>
> drivers/iio/chemical/scd30_i2c.c:104 scd30_i2c_command() error: we
> previously assumed rsp could be null (see line 77)
>
> No functional change for the existing callers, which only ever use
> (response=NULL, size=0) for writes and (response!=NULL, size>0) for
> reads.
Is this analysis AI assisted?
> Signed-off-by: Stepan Ionichev <sozdayvek@xxxxxxxxx>
> ---
> v2:
> - Move the check to the top of the function and return -EINVAL on
> the (response=NULL, size>0) combination, as suggested by Jonathan
> Cameron. Drop the v1 "if (!rsp) return 0" deeper in the function.
Do not reply to the same email thread with a new patch version.
...
Code wise LGTM, thanks.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
--
With Best Regards,
Andy Shevchenko