[PATCH 0/2] iio: small fixes and improvements

[PATCH 0/2] iio: small fixes and improvements

[Nuno Sá via B4 Relay]

The first patch is a fix for the backend code dealing with
direct_register_access. We need to properly terminate the received
string before passing it to sscanf().

The second patch is about using the simple_write_to_buffer() which makes
the direct_register_access write side more symmetric with the read side.

---
Nuno Sá (2):
      iio: backend: make sure to NULL terminate stack buffer
      iio: core: make use of simple_write_to_buffer()

 drivers/iio/industrialio-backend.c | 4 +++-
 drivers/iio/industrialio-core.c    | 9 +++++----
 2 files changed, 8 insertions(+), 5 deletions(-)
---
base-commit: c0f115a8d97599623294c8e9ec28530e19c1e85b
change-id: 20250218-dev-iio-misc-d78425a94985
--

Thanks!
- Nuno Sá

[PATCH 1/2] iio: backend: make sure to NULL terminate stack buffer

[Nuno Sá via B4 Relay]

From: Nuno Sá <nuno.sa@analog.com>

Make sure to NULL terminate the buffer in
iio_backend_debugfs_write_reg() before passing it to sscanf(). It is a
stack variable so we should not assume it will 0 initialized.

Fixes: cdf01e0809a4 ("iio: backend: add debugFs interface")
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/industrialio-backend.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index d4ad36f54090204bf3bef08457d4aa55aa7c11fc..a43c8d1bb3d0f4dda4277cac94b0ea9232c071e4 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -155,10 +155,12 @@ static ssize_t iio_backend_debugfs_write_reg(struct file *file,
 	ssize_t rc;
 	int ret;
 
-	rc = simple_write_to_buffer(buf, sizeof(buf), ppos, userbuf, count);
+	rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
 	if (rc < 0)
 		return rc;
 
+	buf[count] = '\0';
+
 	ret = sscanf(buf, "%i %i", &back->cached_reg_addr, &val);
 
 	switch (ret) {

-- 
2.48.1

Re: [PATCH 1/2] iio: backend: make sure to NULL terminate stack buffer

[David Lechner]

On 2/18/25 4:31 AM, Nuno Sá via B4 Relay wrote:
> From: Nuno Sá <nuno.sa@analog.com>
> 
> Make sure to NULL terminate the buffer in
> iio_backend_debugfs_write_reg() before passing it to sscanf(). It is a
> stack variable so we should not assume it will 0 initialized.
> 
> Fixes: cdf01e0809a4 ("iio: backend: add debugFs interface")
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
>  drivers/iio/industrialio-backend.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
> index d4ad36f54090204bf3bef08457d4aa55aa7c11fc..a43c8d1bb3d0f4dda4277cac94b0ea9232c071e4 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -155,10 +155,12 @@ static ssize_t iio_backend_debugfs_write_reg(struct file *file,
>  	ssize_t rc;
>  	int ret;
>  
> -	rc = simple_write_to_buffer(buf, sizeof(buf), ppos, userbuf, count);
> +	rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
>  	if (rc < 0)
>  		return rc;
>  
> +	buf[count] = '\0';

Does this need to be count++? Later we return count.

> +
>  	ret = sscanf(buf, "%i %i", &back->cached_reg_addr, &val);
>  
>  	switch (ret) {
> 

Re: [PATCH 1/2] iio: backend: make sure to NULL terminate stack buffer

[Nuno Sá]

On Tue, 2025-02-18 at 09:52 -0600, David Lechner wrote:
> On 2/18/25 4:31 AM, Nuno Sá via B4 Relay wrote:
> > From: Nuno Sá <nuno.sa@analog.com>
> > 
> > Make sure to NULL terminate the buffer in
> > iio_backend_debugfs_write_reg() before passing it to sscanf(). It is a
> > stack variable so we should not assume it will 0 initialized.
> > 
> > Fixes: cdf01e0809a4 ("iio: backend: add debugFs interface")
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > ---
> >  drivers/iio/industrialio-backend.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
> > backend.c
> > index
> > d4ad36f54090204bf3bef08457d4aa55aa7c11fc..a43c8d1bb3d0f4dda4277cac94b0ea9232
> > c071e4 100644
> > --- a/drivers/iio/industrialio-backend.c
> > +++ b/drivers/iio/industrialio-backend.c
> > @@ -155,10 +155,12 @@ static ssize_t iio_backend_debugfs_write_reg(struct
> > file *file,
> >  	ssize_t rc;
> >  	int ret;
> >  
> > -	rc = simple_write_to_buffer(buf, sizeof(buf), ppos, userbuf,
> > count);
> > +	rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf,
> > count);
> >  	if (rc < 0)
> >  		return rc;
> >  
> > +	buf[count] = '\0';
> 
> Does this need to be count++? Later we return count.
> 

Don't think so... count comes down from userspace. The termination is local so
we do not want to return count + 1 when userspace only requested to write count.
Same deal as in iio_debugfs_write_reg()

Also note that we pass sizeof(buf) - 1 into simple_write_to_buffer()

- Nuno Sá
> 

Re: [PATCH 1/2] iio: backend: make sure to NULL terminate stack buffer

[David Lechner]

On 2/18/25 10:36 AM, Nuno Sá wrote:
> On Tue, 2025-02-18 at 09:52 -0600, David Lechner wrote:
>> On 2/18/25 4:31 AM, Nuno Sá via B4 Relay wrote:
>>> From: Nuno Sá <nuno.sa@analog.com>
>>>
>>> Make sure to NULL terminate the buffer in
>>> iio_backend_debugfs_write_reg() before passing it to sscanf(). It is a
>>> stack variable so we should not assume it will 0 initialized.
>>>
>>> Fixes: cdf01e0809a4 ("iio: backend: add debugFs interface")
>>> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
>>> ---
>>>  drivers/iio/industrialio-backend.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
>>> backend.c
>>> index
>>> d4ad36f54090204bf3bef08457d4aa55aa7c11fc..a43c8d1bb3d0f4dda4277cac94b0ea9232
>>> c071e4 100644
>>> --- a/drivers/iio/industrialio-backend.c
>>> +++ b/drivers/iio/industrialio-backend.c
>>> @@ -155,10 +155,12 @@ static ssize_t iio_backend_debugfs_write_reg(struct
>>> file *file,
>>>  	ssize_t rc;
>>>  	int ret;
>>>  
>>> -	rc = simple_write_to_buffer(buf, sizeof(buf), ppos, userbuf,
>>> count);
>>> +	rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf,
>>> count);
>>>  	if (rc < 0)
>>>  		return rc;
>>>  
>>> +	buf[count] = '\0';
>>
>> Does this need to be count++? Later we return count.
>>
> 
> Don't think so... count comes down from userspace. The termination is local so
> we do not want to return count + 1 when userspace only requested to write count.
> Same deal as in iio_debugfs_write_reg()
> 
> Also note that we pass sizeof(buf) - 1 into simple_write_to_buffer()
> 
> - Nuno Sá
>>

Ah, right. I get it now. Wasn't thinking so clear earlier.

[PATCH 2/2] iio: core: make use of simple_write_to_buffer()

[Nuno Sá via B4 Relay]

From: Nuno Sá <nuno.sa@analog.com>

Instead of open coding (kind of) simple_write_to_buffer(), use it.

While at it, use ascii representation to terminate the string as that is
the more common way of doing it.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/industrialio-core.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index a2117ad1337d55468c0f21c274fcbedd352c97ff..b9f4113ae5fc3ee1ef76be6808cc437286690dae 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -410,11 +410,12 @@ static ssize_t iio_debugfs_write_reg(struct file *file,
 	char buf[80];
 	int ret;
 
-	count = min(count, sizeof(buf) - 1);
-	if (copy_from_user(buf, userbuf, count))
-		return -EFAULT;
+	ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf,
+				     count);
+	if (ret < 0)
+		return ret;
 
-	buf[count] = 0;
+	buf[count] = '\0';
 
 	ret = sscanf(buf, "%i %i", &reg, &val);
 

-- 
2.48.1

Re: [PATCH 0/2] iio: small fixes and improvements

[David Lechner]

On 2/18/25 4:31 AM, Nuno Sá via B4 Relay wrote:
> The first patch is a fix for the backend code dealing with
> direct_register_access. We need to properly terminate the received
> string before passing it to sscanf().
> 
> The second patch is about using the simple_write_to_buffer() which makes
> the direct_register_access write side more symmetric with the read side.
> 
> ---

Reviewed-by: David Lechner <dlechner@baylibre.com>

Re: [PATCH 0/2] iio: small fixes and improvements

[Jonathan Cameron]

On Tue, 18 Feb 2025 11:29:08 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 2/18/25 4:31 AM, Nuno Sá via B4 Relay wrote:
> > The first patch is a fix for the backend code dealing with
> > direct_register_access. We need to properly terminate the received
> > string before passing it to sscanf().
> > 
> > The second patch is about using the simple_write_to_buffer() which makes
> > the direct_register_access write side more symmetric with the read side.
> > 
> > ---  
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> 

Applied