iio: chemical: scd30: reject (response=NULL, size>0) in scd30_i2c_command()

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.

Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
Acked-by: Maxwell Doose <m32285159@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Jonathan Cameron <jic23@kernel.org>

diff --git a/drivers/iio/chemical/scd30_i2c.c b/drivers/iio/chemical/scd30_i2c.c
index bf465cc71be7d44c17eb25dbed443b133436f95e..9e841f56514990ed8f3b5279bdfa938101e820fb 100644 (file)
--- a/drivers/iio/chemical/scd30_i2c.c
+++ b/drivers/iio/chemical/scd30_i2c.c
@@ -71,6 +71,9 @@ static int scd30_i2c_command(struct scd30_state *state, enum scd30_cmd cmd, u16
        int i, ret;
        char crc;
 
+       if (!response && size != 0)
+               return -EINVAL;
+
        put_unaligned_be16(scd30_i2c_cmd_lookup_tbl[cmd], buf);
        i = 2;