* [PATCH v3] iio: dac: ad3552r-hs: fix scnprintf() buffer bound in data source show
@ 2026-07-18 18:22 Babanpreet Singh
2026-07-18 22:00 ` Jonathan Cameron
0 siblings, 1 reply; 2+ messages in thread
From: Babanpreet Singh @ 2026-07-18 18:22 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Nuno Sá, Michael Hennerich, David Lechner, Andy Shevchenko,
Andy Shevchenko, Angelo Dureghello, linux, linux-iio,
linux-kernel, Babanpreet Singh
ad3552r_hs_show_data_source_avail() formats the available data source
names into a 128-byte stack buffer, but bounds each scnprintf() with
PAGE_SIZE instead of the buffer size, so the bound does not protect
the destination at all.
This cannot overflow today - dbgfs_attr_source[] has two entries,
"normal" and "ramp-16bit", 18 bytes formatted - but the bound stops
protecting the stack the day the table grows. Use sizeof(buf) so the
bound matches the destination.
Found by smatch:
drivers/iio/dac/ad3552r-hs.c:593 ad3552r_hs_show_data_source_avail()
error: scnprintf() 'buf[len]' too small (128 vs 4096)
Fixes: b1c5d68ea66e ("iio: dac: ad3552r-hs: add support for internal ramp")
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Babanpreet Singh <bbnpreetsingh@gmail.com>
---
v3:
- Return to the v1 fix: bound scnprintf() with sizeof(buf) instead
of switching to sysfs_emit_at(). As Andy noticed in the v2 review,
this is a custom debugfs read handler formatting a kernel buffer
for simple_read_from_buffer(), not a sysfs show callback, so
sysfs_emit_at() does not apply here:
https://lore.kernel.org/r/als6lRi-H4DM7ra6@ashevche-desk.local
- Dropped Suggested-by and restored the v1 title; the diff is
identical to v1.
v2: https://lore.kernel.org/r/20260718044244.7-1-bbnpreetsingh@gmail.com
v1: https://lore.kernel.org/r/20260717040024.7-1-bbnpreetsingh@gmail.com
drivers/iio/dac/ad3552r-hs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
index 02a124ac4855..f865843aa439 100644
--- a/drivers/iio/dac/ad3552r-hs.c
+++ b/drivers/iio/dac/ad3552r-hs.c
@@ -590,7 +590,7 @@ static ssize_t ad3552r_hs_show_data_source_avail(struct file *f,
int i;
for (i = 0; i < ARRAY_SIZE(dbgfs_attr_source); i++) {
- len += scnprintf(buf + len, PAGE_SIZE - len, "%s ",
+ len += scnprintf(buf + len, sizeof(buf) - len, "%s ",
dbgfs_attr_source[i]);
}
buf[len - 1] = '\n';
base-commit: fce2dfa773ced15f27dd27cd0b482a7473cdcf2a
--
2.43.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3] iio: dac: ad3552r-hs: fix scnprintf() buffer bound in data source show
2026-07-18 18:22 [PATCH v3] iio: dac: ad3552r-hs: fix scnprintf() buffer bound in data source show Babanpreet Singh
@ 2026-07-18 22:00 ` Jonathan Cameron
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Cameron @ 2026-07-18 22:00 UTC (permalink / raw)
To: Babanpreet Singh
Cc: Nuno Sá, Michael Hennerich, David Lechner, Andy Shevchenko,
Andy Shevchenko, Angelo Dureghello, linux, linux-iio,
linux-kernel
On Sat, 18 Jul 2026 18:22:36 +0000
Babanpreet Singh <bbnpreetsingh@gmail.com> wrote:
> ad3552r_hs_show_data_source_avail() formats the available data source
> names into a 128-byte stack buffer, but bounds each scnprintf() with
> PAGE_SIZE instead of the buffer size, so the bound does not protect
> the destination at all.
>
> This cannot overflow today - dbgfs_attr_source[] has two entries,
> "normal" and "ramp-16bit", 18 bytes formatted - but the bound stops
> protecting the stack the day the table grows. Use sizeof(buf) so the
> bound matches the destination.
>
> Found by smatch:
>
> drivers/iio/dac/ad3552r-hs.c:593 ad3552r_hs_show_data_source_avail()
> error: scnprintf() 'buf[len]' too small (128 vs 4096)
>
> Fixes: b1c5d68ea66e ("iio: dac: ad3552r-hs: add support for internal ramp")
> Assisted-by: Claude:claude-sonnet-5
> Signed-off-by: Babanpreet Singh <bbnpreetsingh@gmail.com>
Applied to the fixes-togreg branch of iio.git and marked for stable.
Thanks,
Jonathan
> ---
> v3:
> - Return to the v1 fix: bound scnprintf() with sizeof(buf) instead
> of switching to sysfs_emit_at(). As Andy noticed in the v2 review,
> this is a custom debugfs read handler formatting a kernel buffer
> for simple_read_from_buffer(), not a sysfs show callback, so
> sysfs_emit_at() does not apply here:
> https://lore.kernel.org/r/als6lRi-H4DM7ra6@ashevche-desk.local
> - Dropped Suggested-by and restored the v1 title; the diff is
> identical to v1.
>
> v2: https://lore.kernel.org/r/20260718044244.7-1-bbnpreetsingh@gmail.com
> v1: https://lore.kernel.org/r/20260717040024.7-1-bbnpreetsingh@gmail.com
>
> drivers/iio/dac/ad3552r-hs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c
> index 02a124ac4855..f865843aa439 100644
> --- a/drivers/iio/dac/ad3552r-hs.c
> +++ b/drivers/iio/dac/ad3552r-hs.c
> @@ -590,7 +590,7 @@ static ssize_t ad3552r_hs_show_data_source_avail(struct file *f,
> int i;
>
> for (i = 0; i < ARRAY_SIZE(dbgfs_attr_source); i++) {
> - len += scnprintf(buf + len, PAGE_SIZE - len, "%s ",
> + len += scnprintf(buf + len, sizeof(buf) - len, "%s ",
> dbgfs_attr_source[i]);
> }
> buf[len - 1] = '\n';
>
> base-commit: fce2dfa773ced15f27dd27cd0b482a7473cdcf2a
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-18 22:00 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-18 18:22 [PATCH v3] iio: dac: ad3552r-hs: fix scnprintf() buffer bound in data source show Babanpreet Singh
2026-07-18 22:00 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox