iio: core: fix uninitialized data in debugfs

[PATCH] iio: core: fix uninitialized data in debugfs

Posted by Dan Carpenter 2 weeks ago

If *ppos is non-zero then simple_write_to_buffer() will not initialize
the start of buf[].  Non zero values for *ppos aren't going to work
anyway.  Test for them at the start of the function and return -EINVAL.

Fixes: 6d5dd486c715 ("iio: core: make use of simple_write_to_buffer()")
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
The original copy_from_user() code was better:
https://staticthinking.wordpress.com/2026/05/23/simple_write_to_buffer-is-complicated/
---
 drivers/iio/industrialio-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index bd6f4f9f4533..1308a534582b 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 (*ppos != 0 || count >= sizeof(buf))
 		return -EINVAL;
 
 	ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf,
-- 
2.53.0

Re: [PATCH] iio: core: fix uninitialized data in debugfs

Posted by Maxwell Doose 2 weeks ago

Hi Dan,

On Mon, May 25, 2026 at 2:19 AM Dan Carpenter <error27@gmail.com> wrote:
>
> If *ppos is non-zero then simple_write_to_buffer() will not initialize
> the start of buf[].  Non zero values for *ppos aren't going to work
> anyway.  Test for them at the start of the function and return -EINVAL.
>
> Fixes: 6d5dd486c715 ("iio: core: make use of simple_write_to_buffer()")
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> ---
> The original copy_from_user() code was better:
> https://staticthinking.wordpress.com/2026/05/23/simple_write_to_buffer-is-complicated/
> ---
>  drivers/iio/industrialio-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Codewise looks good but often times people will prefer

if (*ppos || count >= sizeof(buf))

over

if (*ppos != 0 || count >= sizeof(buf))

Regardless,

Reviewed-by: Maxwell Doose <m32285159@gmail.com>

best regards,
max

Re: [PATCH] iio: core: fix uninitialized data in debugfs

Posted by Dan Carpenter 2 weeks ago

On Mon, May 25, 2026 at 08:17:28AM -0500, Maxwell Doose wrote:
> Hi Dan,
> 
> On Mon, May 25, 2026 at 2:19 AM Dan Carpenter <error27@gmail.com> wrote:
> >
> > If *ppos is non-zero then simple_write_to_buffer() will not initialize
> > the start of buf[].  Non zero values for *ppos aren't going to work
> > anyway.  Test for them at the start of the function and return -EINVAL.
> >
> > Fixes: 6d5dd486c715 ("iio: core: make use of simple_write_to_buffer()")
> > Signed-off-by: Dan Carpenter <error27@gmail.com>
> > ---
> > The original copy_from_user() code was better:
> > https://staticthinking.wordpress.com/2026/05/23/simple_write_to_buffer-is-complicated/
> > ---
> >  drivers/iio/industrialio-core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> 
> Codewise looks good but often times people will prefer
> 
> if (*ppos || count >= sizeof(buf))
> 
> over
> 
> if (*ppos != 0 || count >= sizeof(buf))

In this context, I feel like either is acceptable since zero
represents the number zero.  I have a blog about that which I have
been trying to promote.  #SEO

https://staticthinking.wordpress.com/2024/02/20/when-to-use-0/

> 
> Regardless,
> 
> Reviewed-by: Maxwell Doose <m32285159@gmail.com>
> 

Thanks.  And you're other comments with regards to -EINVAL and
-ENOSPC are obviously correct.

regards,
dan carpenter

Re: [PATCH] iio: core: fix uninitialized data in debugfs

Posted by Maxwell Doose 2 weeks ago

On Mon, May 25, 2026 at 11:13 AM Dan Carpenter <error27@gmail.com> wrote:
>
> On Mon, May 25, 2026 at 08:17:28AM -0500, Maxwell Doose wrote:
> > Hi Dan,
> >
> > On Mon, May 25, 2026 at 2:19 AM Dan Carpenter <error27@gmail.com> wrote:
> > >
> > > If *ppos is non-zero then simple_write_to_buffer() will not initialize
> > > the start of buf[].  Non zero values for *ppos aren't going to work
> > > anyway.  Test for them at the start of the function and return -EINVAL.
> > >
> > > Fixes: 6d5dd486c715 ("iio: core: make use of simple_write_to_buffer()")
> > > Signed-off-by: Dan Carpenter <error27@gmail.com>
> > > ---
> > > The original copy_from_user() code was better:
> > > https://staticthinking.wordpress.com/2026/05/23/simple_write_to_buffer-is-complicated/
> > > ---
> > >  drivers/iio/industrialio-core.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> >
> > Codewise looks good but often times people will prefer
> >
> > if (*ppos || count >= sizeof(buf))
> >
> > over
> >
> > if (*ppos != 0 || count >= sizeof(buf))
>
> In this context, I feel like either is acceptable since zero
> represents the number zero.  I have a blog about that which I have
> been trying to promote.  #SEO
>
> https://staticthinking.wordpress.com/2024/02/20/when-to-use-0/
>

Interesting article, and I do agree that either is appropriate, just
wanted to give you a heads up in case Jonathan or Andy ask you to
change it.

best regards,
max


>
> >
> > Regardless,
> >
> > Reviewed-by: Maxwell Doose <m32285159@gmail.com>
> >
>
> Thanks.  And you're other comments with regards to -EINVAL and
> -ENOSPC are obviously correct.
>
> regards,
> dan carpenter
>

Re: [PATCH] iio: core: fix uninitialized data in debugfs

Posted by Jonathan Cameron 1 week, 6 days ago

On Mon, 25 May 2026 15:08:14 -0500
Maxwell Doose <m32285159@gmail.com> wrote:

> On Mon, May 25, 2026 at 11:13 AM Dan Carpenter <error27@gmail.com> wrote:
> >
> > On Mon, May 25, 2026 at 08:17:28AM -0500, Maxwell Doose wrote:  
> > > Hi Dan,
> > >
> > > On Mon, May 25, 2026 at 2:19 AM Dan Carpenter <error27@gmail.com> wrote:  
> > > >
> > > > If *ppos is non-zero then simple_write_to_buffer() will not initialize
> > > > the start of buf[].  Non zero values for *ppos aren't going to work
> > > > anyway.  Test for them at the start of the function and return -EINVAL.
> > > >
> > > > Fixes: 6d5dd486c715 ("iio: core: make use of simple_write_to_buffer()")
> > > > Signed-off-by: Dan Carpenter <error27@gmail.com>
> > > > ---
> > > > The original copy_from_user() code was better:
> > > > https://staticthinking.wordpress.com/2026/05/23/simple_write_to_buffer-is-complicated/
> > > > ---
> > > >  drivers/iio/industrialio-core.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >  
> > >
> > > Codewise looks good but often times people will prefer
> > >
> > > if (*ppos || count >= sizeof(buf))
> > >
> > > over
> > >
> > > if (*ppos != 0 || count >= sizeof(buf))  
> >
> > In this context, I feel like either is acceptable since zero
> > represents the number zero.  I have a blog about that which I have
> > been trying to promote.  #SEO
> >
> > https://staticthinking.wordpress.com/2024/02/20/when-to-use-0/
> >  
> 
> Interesting article, and I do agree that either is appropriate, just
> wanted to give you a heads up in case Jonathan or Andy ask you to
> change it.
> 
> best regards,
> max
> 
Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks,

Jonathan

> >  
> > >
> > > Regardless,
> > >
> > > Reviewed-by: Maxwell Doose <m32285159@gmail.com>
> > >  
> >
> > Thanks.  And you're other comments with regards to -EINVAL and
> > -ENOSPC are obviously correct.
> >
> > regards,
> > dan carpenter
> >

Re: [PATCH] iio: core: fix uninitialized data in debugfs

Posted by Andy Shevchenko 4 days, 16 hours ago

On Tue, May 26, 2026 at 07:21:41PM +0100, Jonathan Cameron wrote:
> On Mon, 25 May 2026 15:08:14 -0500
> Maxwell Doose <m32285159@gmail.com> wrote:

...

> Applied to the fixes-togreg branch of iio.git and marked for stable.

Same Q as per other patch.

-- 
With Best Regards,
Andy Shevchenko