All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Maxwell Doose" <m32285159@gmail.com>
To: "Stepan Ionichev" <sozdayvek@gmail.com>, <jic23@kernel.org>
Cc: <m32285159@gmail.com>, <dlechner@baylibre.com>,
	<nuno.sa@analog.com>, <andy@kernel.org>,
	<linux-iio@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] iio: chemical: scd30: reject (response=NULL, size>0) in scd30_i2c_command()
Date: Fri, 08 May 2026 11:02:53 -0500	[thread overview]
Message-ID: <DIDF7CF01QK0.38MS3JFMU6RPL@gmail.com> (raw)
In-Reply-To: <20260507152800.9062-1-sozdayvek@gmail.com>

On Thu May 7, 2026 at 10:28 AM CDT, 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).
>
[snip]
> @@ -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;
>

I guess we're still handling the cases where both response/rsp and
size are zero here below?

	if (rsp) {
		/* each two bytes are followed by a crc8 */
		size += size / 2;
	} else {
		put_unaligned_be16(arg, buf + i);
		crc = crc8(scd30_i2c_crc8_tbl, buf + i, 2, CRC8_INIT_VALUE);
		i += 2;
		buf[i] = crc;
		i += 1;

		/* commands below don't take an argument */
		if ((cmd == CMD_STOP_MEAS) || (cmd == CMD_RESET))
			i -= 3;
	}

Should add a comment showing that this handles this case but that's
just my personal nit. Not worth forcing a v3 so

Acked-by: Maxwell Doose <m32285159@gmail.com>

best regards,
max

  parent reply	other threads:[~2026-05-08 16:02 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-06 18:15 [PATCH] iio: chemical: scd30: avoid potential NULL deref in scd30_i2c_command() Stepan Ionichev
2026-05-07 15:28 ` [PATCH v2] iio: chemical: scd30: reject (response=NULL, size>0) " Stepan Ionichev
2026-05-08  7:36   ` Andy Shevchenko
2026-05-08  7:29     ` Stepan Ionichev
2026-05-08 16:02   ` Maxwell Doose [this message]
2026-05-08 18:16     ` Stepan Ionichev
2026-05-08 19:50       ` Maxwell Doose
2026-05-11 11:51         ` Jonathan Cameron
2026-05-07 16:18 ` [PATCH] iio: chemical: scd30: avoid potential NULL deref " Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DIDF7CF01QK0.38MS3JFMU6RPL@gmail.com \
    --to=m32285159@gmail.com \
    --cc=andy@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    --cc=sozdayvek@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.