* Re: [PATCH v2] iio: dac: ad3552r-hs: fix out-of-bound write in ad3552r_hs_write_data_source
2026-01-07 14:35 [PATCH v2] iio: dac: ad3552r-hs: fix out-of-bound write in ad3552r_hs_write_data_source Miaoqian Lin
@ 2026-01-07 15:30 ` Andy Shevchenko
2026-01-08 9:19 ` Nuno Sá
2026-01-11 13:27 ` Jonathan Cameron
2 siblings, 0 replies; 4+ messages in thread
From: Andy Shevchenko @ 2026-01-07 15:30 UTC (permalink / raw)
To: Miaoqian Lin
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Angelo Dureghello,
linux-iio, linux-kernel, stable
On Wed, Jan 07, 2026 at 10:35:50PM +0800, Miaoqian Lin wrote:
> When simple_write_to_buffer() succeeds, it returns the number of bytes
> actually copied to the buffer. The code incorrectly uses 'count'
> as the index for null termination instead of the actual bytes copied.
> If count exceeds the buffer size, this leads to out-of-bounds write.
> Add a check for the count and use the return value as the index.
>
> The bug was validated using a demo module that mirrors the original
> code and was tested under QEMU.
>
> Pattern of the bug:
> - A fixed 64-byte stack buffer is filled using count.
> - If count > 64, the code still does buf[count] = '\0', causing an
> - out-of-bounds write on the stack.
>
> Steps for reproduce:
> - Opens the device node.
> - Writes 128 bytes of A to it.
> - This overflows the 64-byte stack buffer and KASAN reports the OOB.
>
> Found via static analysis. This is similar to the
> commit da9374819eb3 ("iio: backend: fix out-of-bound write")
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] iio: dac: ad3552r-hs: fix out-of-bound write in ad3552r_hs_write_data_source
2026-01-07 14:35 [PATCH v2] iio: dac: ad3552r-hs: fix out-of-bound write in ad3552r_hs_write_data_source Miaoqian Lin
2026-01-07 15:30 ` Andy Shevchenko
@ 2026-01-08 9:19 ` Nuno Sá
2026-01-11 13:27 ` Jonathan Cameron
2 siblings, 0 replies; 4+ messages in thread
From: Nuno Sá @ 2026-01-08 9:19 UTC (permalink / raw)
To: Miaoqian Lin, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Angelo Dureghello, linux-iio, linux-kernel
Cc: stable
On Wed, 2026-01-07 at 22:35 +0800, Miaoqian Lin wrote:
> When simple_write_to_buffer() succeeds, it returns the number of bytes
> actually copied to the buffer. The code incorrectly uses 'count'
> as the index for null termination instead of the actual bytes copied.
> If count exceeds the buffer size, this leads to out-of-bounds write.
> Add a check for the count and use the return value as the index.
>
> The bug was validated using a demo module that mirrors the original
> code and was tested under QEMU.
>
> Pattern of the bug:
> - A fixed 64-byte stack buffer is filled using count.
> - If count > 64, the code still does buf[count] = '\0', causing an
> - out-of-bounds write on the stack.
>
> Steps for reproduce:
> - Opens the device node.
> - Writes 128 bytes of A to it.
> - This overflows the 64-byte stack buffer and KASAN reports the OOB.
>
> Found via static analysis. This is similar to the
> commit da9374819eb3 ("iio: backend: fix out-of-bound write")
>
> Fixes: b1c5d68ea66e ("iio: dac: ad3552r-hs: add support for internal ramp")
> Cc: stable@vger.kernel.org
> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
> ---
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> changes in v2:
> - update commit message
> - v1 link: https://lore.kernel.org/all/20251027150713.59067-1-linmq006@gmail.com/
> ---
> drivers/iio/dac/ad3552r-hs.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> index 41b96b48ba98..a9578afa7015 100644
> --- a/drivers/iio/dac/ad3552r-hs.c
> +++ b/drivers/iio/dac/ad3552r-hs.c
> @@ -549,12 +549,15 @@ static ssize_t ad3552r_hs_write_data_source(struct file *f,
>
> guard(mutex)(&st->lock);
>
> + if (count >= sizeof(buf))
> + return -ENOSPC;
> +
> ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf,
> count);
> if (ret < 0)
> return ret;
>
> - buf[count] = '\0';
> + buf[ret] = '\0';
>
> ret = match_string(dbgfs_attr_source, ARRAY_SIZE(dbgfs_attr_source),
> buf);
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] iio: dac: ad3552r-hs: fix out-of-bound write in ad3552r_hs_write_data_source
2026-01-07 14:35 [PATCH v2] iio: dac: ad3552r-hs: fix out-of-bound write in ad3552r_hs_write_data_source Miaoqian Lin
2026-01-07 15:30 ` Andy Shevchenko
2026-01-08 9:19 ` Nuno Sá
@ 2026-01-11 13:27 ` Jonathan Cameron
2 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2026-01-11 13:27 UTC (permalink / raw)
To: Miaoqian Lin
Cc: Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Angelo Dureghello, linux-iio,
linux-kernel, stable
On Wed, 7 Jan 2026 22:35:50 +0800
Miaoqian Lin <linmq006@gmail.com> wrote:
> When simple_write_to_buffer() succeeds, it returns the number of bytes
> actually copied to the buffer. The code incorrectly uses 'count'
> as the index for null termination instead of the actual bytes copied.
> If count exceeds the buffer size, this leads to out-of-bounds write.
> Add a check for the count and use the return value as the index.
>
> The bug was validated using a demo module that mirrors the original
> code and was tested under QEMU.
>
> Pattern of the bug:
> - A fixed 64-byte stack buffer is filled using count.
> - If count > 64, the code still does buf[count] = '\0', causing an
> - out-of-bounds write on the stack.
>
> Steps for reproduce:
> - Opens the device node.
> - Writes 128 bytes of A to it.
> - This overflows the 64-byte stack buffer and KASAN reports the OOB.
>
> Found via static analysis. This is similar to the
> commit da9374819eb3 ("iio: backend: fix out-of-bound write")
>
> Fixes: b1c5d68ea66e ("iio: dac: ad3552r-hs: add support for internal ramp")
> Cc: stable@vger.kernel.org
> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
Hi Miaoquian,
You haven't listed why you didn't add David's Reviewed-by tag from v1.
b4 found it though so all is fine, but none the less please make sure you either
pick up tags, or in the change log say that you didn't deliberately.
Applied to the fixes-togreg branch of iio.git
Thanks,
Jonathan
> ---
> changes in v2:
> - update commit message
> - v1 link: https://lore.kernel.org/all/20251027150713.59067-1-linmq006@gmail.com/
> ---
> drivers/iio/dac/ad3552r-hs.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> index 41b96b48ba98..a9578afa7015 100644
> --- a/drivers/iio/dac/ad3552r-hs.c
> +++ b/drivers/iio/dac/ad3552r-hs.c
> @@ -549,12 +549,15 @@ static ssize_t ad3552r_hs_write_data_source(struct file *f,
>
> guard(mutex)(&st->lock);
>
> + if (count >= sizeof(buf))
> + return -ENOSPC;
> +
> ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf,
> count);
> if (ret < 0)
> return ret;
>
> - buf[count] = '\0';
> + buf[ret] = '\0';
>
> ret = match_string(dbgfs_attr_source, ARRAY_SIZE(dbgfs_attr_source),
> buf);
^ permalink raw reply [flat|nested] 4+ messages in thread