[PATCH] iio: chemical: mhz19b: bound receive buffer copy

[PATCH] iio: chemical: mhz19b: bound receive buffer copy

[Pengpeng Hou]

`mhz19b_receive_buf()` appends bytes to the fixed 9-byte command buffer
without first checking that the new chunk fits in the remaining space.
A single receive callback can therefore write past the end of `st->buf`
before the driver sees that the command is complete.

Drop overlong chunks and reset the partial command state before the
copy.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/iio/chemical/mhz19b.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/iio/chemical/mhz19b.c b/drivers/iio/chemical/mhz19b.c
index 3c64154918b1..a028216fa2a9 100644
--- a/drivers/iio/chemical/mhz19b.c
+++ b/drivers/iio/chemical/mhz19b.c
@@ -241,6 +241,11 @@ static size_t mhz19b_receive_buf(struct serdev_device *serdev,
 	struct iio_dev *indio_dev = dev_get_drvdata(&serdev->dev);
 	struct mhz19b_state *st = iio_priv(indio_dev);
 
+	if (len > sizeof(st->buf) - st->buf_idx) {
+		st->buf_idx = 0;
+		return len;
+	}
+
 	memcpy(st->buf + st->buf_idx, data, len);
 	st->buf_idx += len;
 
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] iio: chemical: mhz19b: bound receive buffer copy

[Gyeyoung Baek]

Hello Pengpeng,

On Sun, Mar 22, 2026 at 12:19 PM Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:
>
> `mhz19b_receive_buf()` appends bytes to the fixed 9-byte command buffer
> without first checking that the new chunk fits in the remaining space.
> A single receive callback can therefore write past the end of `st->buf`
> before the driver sees that the command is complete.
>
> Drop overlong chunks and reset the partial command state before the
> copy.
>
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  drivers/iio/chemical/mhz19b.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/iio/chemical/mhz19b.c b/drivers/iio/chemical/mhz19b.c
> index 3c64154918b1..a028216fa2a9 100644
> --- a/drivers/iio/chemical/mhz19b.c
> +++ b/drivers/iio/chemical/mhz19b.c
> @@ -241,6 +241,11 @@ static size_t mhz19b_receive_buf(struct serdev_device *serdev,
>         struct iio_dev *indio_dev = dev_get_drvdata(&serdev->dev);
>         struct mhz19b_state *st = iio_priv(indio_dev);
>
> +       if (len > sizeof(st->buf) - st->buf_idx) {
> +               st->buf_idx = 0;
> +               return len;
> +       }
> +

It would be better to handle err immediately on the condition, but it
still looks much better than before.

Acked-by: Gyeyoung Baek <gye976@gmail.com>

--
Thanks,
Gyeyoung

[PATCH v2] iio: chemical: mhz19b: bound receive buffer copy

[Pengpeng Hou]

`mhz19b_receive_buf()` appends bytes to the fixed 9-byte command buffer
without first checking that the new chunk fits in the remaining space.
A single receive callback can therefore write past the end of `st->buf`
before the driver sees that the command is complete.

Drop overlong chunks and reset the partial command state before the
copy.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/iio/chemical/mhz19b.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/iio/chemical/mhz19b.c b/drivers/iio/chemical/mhz19b.c
index 3c64154918b1..fbd7f14483b3 100644
--- a/drivers/iio/chemical/mhz19b.c
+++ b/drivers/iio/chemical/mhz19b.c
@@ -240,6 +240,12 @@ static size_t mhz19b_receive_buf(struct serdev_device *serdev,
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(&serdev->dev);
 	struct mhz19b_state *st = iio_priv(indio_dev);
+	size_t remaining = sizeof(st->buf) - st->buf_idx;
+
+	if (unlikely(len > remaining)) {
+		st->buf_idx = 0;
+		return len;
+	}
 
 	memcpy(st->buf + st->buf_idx, data, len);
 	st->buf_idx += len;
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v2] iio: chemical: mhz19b: bound receive buffer copy

[David Lechner]

On 3/22/26 8:48 AM, Pengpeng Hou wrote:
> `mhz19b_receive_buf()` appends bytes to the fixed 9-byte command buffer
> without first checking that the new chunk fits in the remaining space.
> A single receive callback can therefore write past the end of `st->buf`
> before the driver sees that the command is complete.
> 
> Drop overlong chunks and reset the partial command state before the
> copy.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---

Please don't make v2 a reply to v1. It makes it hard to see that a new
version has been sent. Also, please include a changelog here (below the
---) when sending new revisions so that we know what changed compared
to v1.

For this patch, you can just reply with the changelog instead of sending
a new revision.

Also, you received an Acked-by on v1, so you need to explain why you did
not keep that tag when you sent v2.

>  drivers/iio/chemical/mhz19b.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/iio/chemical/mhz19b.c b/drivers/iio/chemical/mhz19b.c
> index 3c64154918b1..fbd7f14483b3 100644
> --- a/drivers/iio/chemical/mhz19b.c
> +++ b/drivers/iio/chemical/mhz19b.c
> @@ -240,6 +240,12 @@ static size_t mhz19b_receive_buf(struct serdev_device *serdev,
>  {
>  	struct iio_dev *indio_dev = dev_get_drvdata(&serdev->dev);
>  	struct mhz19b_state *st = iio_priv(indio_dev);
> +	size_t remaining = sizeof(st->buf) - st->buf_idx;
> +
> +	if (unlikely(len > remaining)) {
> +		st->buf_idx = 0;
> +		return len;
> +	}
>  
>  	memcpy(st->buf + st->buf_idx, data, len);
>  	st->buf_idx += len;

Re: [PATCH v2] iio: chemical: mhz19b: bound receive buffer copy

[Jonathan Cameron]

On Sun, 22 Mar 2026 21:48:07 +0800
Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:

> `mhz19b_receive_buf()` appends bytes to the fixed 9-byte command buffer
> without first checking that the new chunk fits in the remaining space.
> A single receive callback can therefore write past the end of `st->buf`
> before the driver sees that the command is complete.
> 
> Drop overlong chunks and reset the partial command state before the
> copy.

This 'before the copy' part of this implies the copy still happens.
I don't think that is true.

> 
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  drivers/iio/chemical/mhz19b.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/iio/chemical/mhz19b.c b/drivers/iio/chemical/mhz19b.c
> index 3c64154918b1..fbd7f14483b3 100644
> --- a/drivers/iio/chemical/mhz19b.c
> +++ b/drivers/iio/chemical/mhz19b.c
> @@ -240,6 +240,12 @@ static size_t mhz19b_receive_buf(struct serdev_device *serdev,
>  {
>  	struct iio_dev *indio_dev = dev_get_drvdata(&serdev->dev);
>  	struct mhz19b_state *st = iio_priv(indio_dev);
> +	size_t remaining = sizeof(st->buf) - st->buf_idx;
> +
> +	if (unlikely(len > remaining)) {
> +		st->buf_idx = 0;
> +		return len;
Given this doesn't complete we will see a timeout error if this is hit?
That is rather misleading. I think I'd at least print a message in this
path to provide a hint of what happened.

Jonathan


> +	}
>  
>  	memcpy(st->buf + st->buf_idx, data, len);
>  	st->buf_idx += len;