Re: [PATCH v5 01/13] iio: core: validate file offset in iio_debugfs_write_reg()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `*ppos != 0` check introduces a functional regression for sequential writes, masking the true bug which is the inappropriate use of `simple_write_to_buffer()`.
- [High] Unpatched uninitialized stack memory reads in other IIO drivers caused by the identical anti-pattern.
--

commit 7c5ddbc34738b9e84e226b9906348d61e3569bf4
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>

iio: core: validate file offset in iio_debugfs_write_reg()

This patch adds a check to ensure that the file offset is zero in
iio_debugfs_write_reg() before calling simple_write_to_buffer(). This is
intended to prevent leaving the start of the buffer uninitialized if a write
occurs at a non-zero offset.

> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index bd6f4f9f45330..e587aca79b8e7 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -419,7 +419,7 @@ static ssize_t iio_debugfs_write_reg(struct file *file,
>  	char buf[80];
>  	int ret;
>  
> -	if (count >= sizeof(buf))
> +	if (count >= sizeof(buf) || *ppos != 0)

Does this cause a functional regression for sequential writes?

If a user writes multiple commands to the same open file descriptor, for
example using (echo "cmd1"; echo "cmd2") > direct_reg_access, the first
write advances the file position. This means *ppos will be non-zero on the
second write, causing it to fail with -EINVAL.

Before simple_write_to_buffer() was introduced, the code used
copy_from_user() and ignored *ppos, correctly treating every write() call
as a distinct command starting at the beginning of the buffer.

Since simple_write_to_buffer() is designed for stateful linear memory buffers
rather than stateless command endpoints using local stack arrays, would it be
better to revert the use of simple_write_to_buffer() entirely and use
copy_from_user() or strndup_user() to parse each write independently?

Additionally, does this leave a similar pattern unpatched in other IIO
drivers?

While this patch addresses the issue in iio_debugfs_write_reg(), the same
pattern of using simple_write_to_buffer() into a local stack array still
exists in other locations:

1. iio_backend_debugfs_write_reg() in drivers/iio/industrialio-backend.c
2. ad3552r_hs_write_data_source() in drivers/iio/dac/ad3552r-hs.c

Both functions use simple_write_to_buffer() into uninitialized local stack
arrays without checking *ppos.

Furthermore, ad9467_chan_test_mode_write() in drivers/iio/adc/ad9467.c
uses the same pattern. While the buffer is zero-initialized there,
sequential writes will still be functionally broken.

>  		return -EINVAL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517-ad9910-iio-driver-v5-0-31599c88314a@analog.com?part=1