* [PATCH] iio: backend: fix uninitialized data in debugfs
@ 2026-05-25 7:16 Dan Carpenter
2026-05-25 13:20 ` Maxwell Doose
0 siblings, 1 reply; 15+ messages in thread
From: Dan Carpenter @ 2026-05-25 7:16 UTC (permalink / raw)
To: Nuno Sa
Cc: Olivier Moysan, Jonathan Cameron, David Lechner, Andy Shevchenko,
linux-iio, linux-kernel, kernel-janitors
If the *ppos value is non-zero then simple_write_to_buffer() will not
initialize the start of the buf[] buffer. Non-zero ppos values aren't
going to work at all. Check for that at the start of the function and
return -EINVAL.
Fixes: cdf01e0809a4 ("iio: backend: add debugFs interface")
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
drivers/iio/industrialio-backend.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index 138ebebc9c0d..4763e224ebc6 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -156,7 +156,7 @@ static ssize_t iio_backend_debugfs_write_reg(struct file *file,
ssize_t rc;
int ret;
- if (count >= sizeof(buf))
+ if (*ppos != 0 || count >= sizeof(buf))
return -ENOSPC;
rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] iio: backend: fix uninitialized data in debugfs
2026-05-25 7:16 [PATCH] iio: backend: fix uninitialized data in debugfs Dan Carpenter
@ 2026-05-25 13:20 ` Maxwell Doose
2026-05-26 18:19 ` Jonathan Cameron
0 siblings, 1 reply; 15+ messages in thread
From: Maxwell Doose @ 2026-05-25 13:20 UTC (permalink / raw)
To: Dan Carpenter
Cc: Nuno Sa, Olivier Moysan, Jonathan Cameron, David Lechner,
Andy Shevchenko, linux-iio, linux-kernel, kernel-janitors
On Mon, May 25, 2026 at 2:17 AM Dan Carpenter <error27@gmail.com> wrote:
>
> If the *ppos value is non-zero then simple_write_to_buffer() will not
> initialize the start of the buf[] buffer. Non-zero ppos values aren't
> going to work at all. Check for that at the start of the function and
> return -EINVAL.
>
Commit message is incorrect, it looks like you're returning -ENOSPC here...
>
> Fixes: cdf01e0809a4 ("iio: backend: add debugFs interface")
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> ---
> drivers/iio/industrialio-backend.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
> index 138ebebc9c0d..4763e224ebc6 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -156,7 +156,7 @@ static ssize_t iio_backend_debugfs_write_reg(struct file *file,
> ssize_t rc;
> int ret;
>
> - if (count >= sizeof(buf))
> + if (*ppos != 0 || count >= sizeof(buf))
> return -ENOSPC;
>
See above comment.
best regards,
max
>
> rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
> --
> 2.53.0
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] iio: backend: fix uninitialized data in debugfs
2026-05-25 13:20 ` Maxwell Doose
@ 2026-05-26 18:19 ` Jonathan Cameron
2026-06-04 7:30 ` Andy Shevchenko
0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2026-05-26 18:19 UTC (permalink / raw)
To: Maxwell Doose
Cc: Dan Carpenter, Nuno Sa, Olivier Moysan, David Lechner,
Andy Shevchenko, linux-iio, linux-kernel, kernel-janitors
On Mon, 25 May 2026 08:20:31 -0500
Maxwell Doose <m32285159@gmail.com> wrote:
> On Mon, May 25, 2026 at 2:17 AM Dan Carpenter <error27@gmail.com> wrote:
> >
> > If the *ppos value is non-zero then simple_write_to_buffer() will not
> > initialize the start of the buf[] buffer. Non-zero ppos values aren't
> > going to work at all. Check for that at the start of the function and
> > return -EINVAL.
> >
>
> Commit message is incorrect, it looks like you're returning -ENOSPC here...
Tweaked and applied to the fixes-togreg branch of iio.git + marked for stable.
thanks,
J
>
> >
> > Fixes: cdf01e0809a4 ("iio: backend: add debugFs interface")
> > Signed-off-by: Dan Carpenter <error27@gmail.com>
> > ---
> > drivers/iio/industrialio-backend.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
> > index 138ebebc9c0d..4763e224ebc6 100644
> > --- a/drivers/iio/industrialio-backend.c
> > +++ b/drivers/iio/industrialio-backend.c
> > @@ -156,7 +156,7 @@ static ssize_t iio_backend_debugfs_write_reg(struct file *file,
> > ssize_t rc;
> > int ret;
> >
> > - if (count >= sizeof(buf))
> > + if (*ppos != 0 || count >= sizeof(buf))
> > return -ENOSPC;
> >
>
> See above comment.
>
> best regards,
> max
>
>
> >
> > rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
> > --
> > 2.53.0
> >
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] iio: backend: fix uninitialized data in debugfs
2026-05-26 18:19 ` Jonathan Cameron
@ 2026-06-04 7:30 ` Andy Shevchenko
2026-06-04 7:38 ` Andy Shevchenko
2026-06-04 7:45 ` Dan Carpenter
0 siblings, 2 replies; 15+ messages in thread
From: Andy Shevchenko @ 2026-06-04 7:30 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Maxwell Doose, Dan Carpenter, Nuno Sa, Olivier Moysan,
David Lechner, Andy Shevchenko, linux-iio, linux-kernel,
kernel-janitors
On Tue, May 26, 2026 at 07:19:46PM +0100, Jonathan Cameron wrote:
> On Mon, 25 May 2026 08:20:31 -0500
> Maxwell Doose <m32285159@gmail.com> wrote:
> > On Mon, May 25, 2026 at 2:17 AM Dan Carpenter <error27@gmail.com> wrote:
> > >
> > > If the *ppos value is non-zero then simple_write_to_buffer() will not
> > > initialize the start of the buf[] buffer. Non-zero ppos values aren't
> > > going to work at all. Check for that at the start of the function and
> > > return -EINVAL.
> >
> > Commit message is incorrect, it looks like you're returning -ENOSPC here...
> Tweaked and applied to the fixes-togreg branch of iio.git + marked for stable.
I was stumbled over these patches by Dan. Since the file_operations for the code
in question do not define .llseek, the seek is not supported and will return
-ESPIPE. I'm not sure why we have these patches to begin with. Dan, can you
elaborate on the case where the *ppos is not 0, please?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] iio: backend: fix uninitialized data in debugfs
2026-06-04 7:30 ` Andy Shevchenko
@ 2026-06-04 7:38 ` Andy Shevchenko
2026-06-04 7:45 ` Dan Carpenter
1 sibling, 0 replies; 15+ messages in thread
From: Andy Shevchenko @ 2026-06-04 7:38 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Maxwell Doose, Dan Carpenter, Nuno Sa, Olivier Moysan,
David Lechner, Andy Shevchenko, linux-iio, linux-kernel,
kernel-janitors
On Thu, Jun 04, 2026 at 10:30:40AM +0300, Andy Shevchenko wrote:
> On Tue, May 26, 2026 at 07:19:46PM +0100, Jonathan Cameron wrote:
> > On Mon, 25 May 2026 08:20:31 -0500
> > Maxwell Doose <m32285159@gmail.com> wrote:
> > > On Mon, May 25, 2026 at 2:17 AM Dan Carpenter <error27@gmail.com> wrote:
> > > >
> > > > If the *ppos value is non-zero then simple_write_to_buffer() will not
> > > > initialize the start of the buf[] buffer. Non-zero ppos values aren't
> > > > going to work at all. Check for that at the start of the function and
> > > > return -EINVAL.
> > >
> > > Commit message is incorrect, it looks like you're returning -ENOSPC here...
> > Tweaked and applied to the fixes-togreg branch of iio.git + marked for stable.
>
> I was stumbled over these patches by Dan. Since the file_operations for the code
> in question do not define .llseek, the seek is not supported and will return
> -ESPIPE. I'm not sure why we have these patches to begin with. Dan, can you
> elaborate on the case where the *ppos is not 0, please?
FWIW, I also left a comment in that blog post.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] iio: backend: fix uninitialized data in debugfs
2026-06-04 7:30 ` Andy Shevchenko
2026-06-04 7:38 ` Andy Shevchenko
@ 2026-06-04 7:45 ` Dan Carpenter
2026-06-04 8:03 ` Andy Shevchenko
1 sibling, 1 reply; 15+ messages in thread
From: Dan Carpenter @ 2026-06-04 7:45 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, Maxwell Doose, Nuno Sa, Olivier Moysan,
David Lechner, Andy Shevchenko, linux-iio, linux-kernel,
kernel-janitors
On Thu, Jun 04, 2026 at 10:30:34AM +0300, Andy Shevchenko wrote:
> On Tue, May 26, 2026 at 07:19:46PM +0100, Jonathan Cameron wrote:
> > On Mon, 25 May 2026 08:20:31 -0500
> > Maxwell Doose <m32285159@gmail.com> wrote:
> > > On Mon, May 25, 2026 at 2:17 AM Dan Carpenter <error27@gmail.com> wrote:
> > > >
> > > > If the *ppos value is non-zero then simple_write_to_buffer() will not
> > > > initialize the start of the buf[] buffer. Non-zero ppos values aren't
> > > > going to work at all. Check for that at the start of the function and
> > > > return -EINVAL.
> > >
> > > Commit message is incorrect, it looks like you're returning -ENOSPC here...
> > Tweaked and applied to the fixes-togreg branch of iio.git + marked for stable.
>
> I was stumbled over these patches by Dan. Since the file_operations for the code
> in question do not define .llseek, the seek is not supported and will return
> -ESPIPE. I'm not sure why we have these patches to begin with. Dan, can you
> elaborate on the case where the *ppos is not 0, please?
The simple_write_to_buffer() will update *ppos so partial writes are
supported in that way.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] iio: backend: fix uninitialized data in debugfs
2026-06-04 7:45 ` Dan Carpenter
@ 2026-06-04 8:03 ` Andy Shevchenko
2026-06-04 8:28 ` Andy Shevchenko
0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2026-06-04 8:03 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jonathan Cameron, Maxwell Doose, Nuno Sa, Olivier Moysan,
David Lechner, Andy Shevchenko, linux-iio, linux-kernel,
kernel-janitors
On Thu, Jun 04, 2026 at 10:45:36AM +0300, Dan Carpenter wrote:
> On Thu, Jun 04, 2026 at 10:30:34AM +0300, Andy Shevchenko wrote:
> > On Tue, May 26, 2026 at 07:19:46PM +0100, Jonathan Cameron wrote:
> > > On Mon, 25 May 2026 08:20:31 -0500
> > > Maxwell Doose <m32285159@gmail.com> wrote:
> > > > On Mon, May 25, 2026 at 2:17 AM Dan Carpenter <error27@gmail.com> wrote:
> > > > >
> > > > > If the *ppos value is non-zero then simple_write_to_buffer() will not
> > > > > initialize the start of the buf[] buffer. Non-zero ppos values aren't
> > > > > going to work at all. Check for that at the start of the function and
> > > > > return -EINVAL.
> > > >
> > > > Commit message is incorrect, it looks like you're returning -ENOSPC here...
> > > Tweaked and applied to the fixes-togreg branch of iio.git + marked for stable.
> >
> > I was stumbled over these patches by Dan. Since the file_operations for the code
> > in question do not define .llseek, the seek is not supported and will return
> > -ESPIPE. I'm not sure why we have these patches to begin with. Dan, can you
> > elaborate on the case where the *ppos is not 0, please?
> The simple_write_to_buffer() will update *ppos so partial writes are
> supported in that way.
Can you show the step-by-step scenario? I'm still fail to see how it may be happen.
Is it somewhere inside the kernel loop? Which VFS function(s) is responsible for
that in such a case?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] iio: backend: fix uninitialized data in debugfs
2026-06-04 8:03 ` Andy Shevchenko
@ 2026-06-04 8:28 ` Andy Shevchenko
2026-06-04 10:38 ` Dan Carpenter
0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2026-06-04 8:28 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jonathan Cameron, Maxwell Doose, Nuno Sa, Olivier Moysan,
David Lechner, Andy Shevchenko, linux-iio, linux-kernel,
kernel-janitors
On Thu, Jun 04, 2026 at 11:03:15AM +0300, Andy Shevchenko wrote:
> On Thu, Jun 04, 2026 at 10:45:36AM +0300, Dan Carpenter wrote:
> > On Thu, Jun 04, 2026 at 10:30:34AM +0300, Andy Shevchenko wrote:
> > > On Tue, May 26, 2026 at 07:19:46PM +0100, Jonathan Cameron wrote:
> > > > On Mon, 25 May 2026 08:20:31 -0500
> > > > Maxwell Doose <m32285159@gmail.com> wrote:
> > > > > On Mon, May 25, 2026 at 2:17 AM Dan Carpenter <error27@gmail.com> wrote:
> > > > > >
> > > > > > If the *ppos value is non-zero then simple_write_to_buffer() will not
> > > > > > initialize the start of the buf[] buffer. Non-zero ppos values aren't
> > > > > > going to work at all. Check for that at the start of the function and
> > > > > > return -EINVAL.
> > > > >
> > > > > Commit message is incorrect, it looks like you're returning -ENOSPC here...
> > > > Tweaked and applied to the fixes-togreg branch of iio.git + marked for stable.
> > >
> > > I was stumbled over these patches by Dan. Since the file_operations for the code
> > > in question do not define .llseek, the seek is not supported and will return
> > > -ESPIPE. I'm not sure why we have these patches to begin with. Dan, can you
> > > elaborate on the case where the *ppos is not 0, please?
>
> > The simple_write_to_buffer() will update *ppos so partial writes are
> > supported in that way.
>
> Can you show the step-by-step scenario? I'm still fail to see how it may be happen.
> Is it somewhere inside the kernel loop? Which VFS function(s) is responsible for
> that in such a case?
Even if ppos is advanced, the simple_write_to_buffer()
https://elixir.bootlin.com/linux/v7.1-rc6/source/fs/libfs.c#L1188
won't write more than available in the buffer. Is the available also
being advanced somehow?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] iio: backend: fix uninitialized data in debugfs
2026-06-04 8:28 ` Andy Shevchenko
@ 2026-06-04 10:38 ` Dan Carpenter
2026-06-04 10:42 ` Dan Carpenter
2026-06-04 14:52 ` Andy Shevchenko
0 siblings, 2 replies; 15+ messages in thread
From: Dan Carpenter @ 2026-06-04 10:38 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, Maxwell Doose, Nuno Sa, Olivier Moysan,
David Lechner, Andy Shevchenko, linux-iio, linux-kernel,
kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 4309 bytes --]
On Thu, Jun 04, 2026 at 11:28:39AM +0300, Andy Shevchenko wrote:
> On Thu, Jun 04, 2026 at 11:03:15AM +0300, Andy Shevchenko wrote:
> > On Thu, Jun 04, 2026 at 10:45:36AM +0300, Dan Carpenter wrote:
> > > On Thu, Jun 04, 2026 at 10:30:34AM +0300, Andy Shevchenko wrote:
> > > > On Tue, May 26, 2026 at 07:19:46PM +0100, Jonathan Cameron wrote:
> > > > > On Mon, 25 May 2026 08:20:31 -0500
> > > > > Maxwell Doose <m32285159@gmail.com> wrote:
> > > > > > On Mon, May 25, 2026 at 2:17 AM Dan Carpenter <error27@gmail.com> wrote:
> > > > > > >
> > > > > > > If the *ppos value is non-zero then simple_write_to_buffer() will not
> > > > > > > initialize the start of the buf[] buffer. Non-zero ppos values aren't
> > > > > > > going to work at all. Check for that at the start of the function and
> > > > > > > return -EINVAL.
> > > > > >
> > > > > > Commit message is incorrect, it looks like you're returning -ENOSPC here...
> > > > > Tweaked and applied to the fixes-togreg branch of iio.git + marked for stable.
> > > >
> > > > I was stumbled over these patches by Dan. Since the file_operations for the code
> > > > in question do not define .llseek, the seek is not supported and will return
> > > > -ESPIPE. I'm not sure why we have these patches to begin with. Dan, can you
> > > > elaborate on the case where the *ppos is not 0, please?
> >
> > > The simple_write_to_buffer() will update *ppos so partial writes are
> > > supported in that way.
> >
> > Can you show the step-by-step scenario? I'm still fail to see how it may be happen.
> > Is it somewhere inside the kernel loop? Which VFS function(s) is responsible for
> > that in such a case?
>
> Even if ppos is advanced, the simple_write_to_buffer()
> https://elixir.bootlin.com/linux/v7.1-rc6/source/fs/libfs.c#L1188
> won't write more than available in the buffer. Is the available also
> being advanced somehow?
It's not a buffer overflow, it's an uninitialized data bug.
I used Google AI to create a test case but my qemu system is arm64
and the test case is in assembly so I'm not sure how useful it is.
(Also I modified the Google AI code and the test case is garbage).
My test case writes 10 bytes of data at a time. I've attached the
test debugfs kernel module.
Here is the code from the kernel:
drivers/iio/industrialio-backend.c
149 static ssize_t iio_backend_debugfs_write_reg(struct file *file,
150 const char __user *userbuf,
151 size_t count, loff_t *ppos)
152 {
153 struct iio_backend *back = file->private_data;
154 unsigned int val;
155 char buf[80];
buf is uninitialized. In my test code, I initialized it to all U
characters which stands for uninitialized.
156 ssize_t rc;
157 int ret;
158
159 if (*ppos != 0 || count >= sizeof(buf))
^^^^^^^^^^
I added this check on *ppos but imagine it's not there and *ppos is
non-zero.
160 return -ENOSPC;
161
162 rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
The simple_write_to_buffer() function is designed to support partial
writes so it leaves the first 0 to *ppos characters alone.
163 if (rc < 0)
164 return rc;
165
166 buf[rc] = '\0';
The return is the number of characters written (starting at *ppos). I'm
doing 10 character writes so it is 10. The first 10 characters are
uninitialized. In my test output you can see it prints ten U characters.
167
168 ret = sscanf(buf, "%i %i", &back->cached_reg_addr, &val);
^^^
Uninitialized variable.
169
root@test:/home/ubuntu/mnt/progs/tmp/sysfs# ./a.out
Enter a line of text to save: 123456789012345678901234567890
[ 4266.853201] pos=0 count=10
[ 4266.853451] rc=10 buf 1234567890
[ 4266.854774] pos=10 count=10
Writing to file character by character...
[ 4266.858582] rc=10 buf UUUUUUUUUU
[ 4266.858698] pos=20 count=10
[ 4266.858740] rc=10 buf UUUUUUUUUU
[ 4266.858798] pos=30 count=10
[ 4266.858834] rc=10 buf UUUUUUUUUU
[ 4266.858888] pos=40 count=10
[ 4266.858924] rc=10 buf UUUUUUUUUU
[ 4266.859015] pos=50 count=10
regards,
dan carpenter
[-- Attachment #2: hello_debugfs.c --]
[-- Type: text/x-csrc, Size: 1989 bytes --]
#include <linux/init.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/debugfs.h>
#include <linux/fs.h>
#include <linux/uaccess.h>
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Developer");
MODULE_DESCRIPTION("A Hello World Module with debugfs functionality");
MODULE_VERSION("1.0");
static struct dentry *dir_ret = NULL;
static struct dentry *file_ret = NULL;
// Triggered when a userspace process reads the debugfs file
static ssize_t hello_write(struct file *fp, const char __user *user_buffer, size_t count, loff_t *position)
{
char buf[60] = "UUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUU";
int rc;
pr_info("pos=%lld count=%lu\n", *position, count);
rc = simple_write_to_buffer(buf, sizeof(buf), position, user_buffer, count);
if (rc < 0)
return rc;
buf[rc] = '\0';
pr_info("rc=%d buf %s\n", rc, buf);
return count;
}
// Map the custom read logic to file operations structure
static const struct file_operations hello_fops = {
.owner = THIS_MODULE,
.write = hello_write,
};
// Module Initialization
static int __init hello_init(void)
{
pr_info("hello_debugfs: Module loaded successfully\n");
// 1. Create top-level parent directory in debugfs root
dir_ret = debugfs_create_dir("hello_dir", NULL);
if (IS_ERR(dir_ret)) {
pr_err("hello_debugfs: Failed to create parent directory\n");
return PTR_ERR(dir_ret);
}
// 2. Create target communication file inside the parent directory (Read-only for all: 0444)
file_ret = debugfs_create_file("hello_file", 0444, dir_ret, NULL, &hello_fops);
if (IS_ERR(file_ret)) {
pr_err("hello_debugfs: Failed to create communication file\n");
debugfs_remove_recursive(dir_ret);
return PTR_ERR(file_ret);
}
return 0;
}
// Module Cleanup
static void __exit hello_exit(void)
{
// Recursively destroys the created files and directory nodes
debugfs_remove_recursive(dir_ret);
pr_info("hello_debugfs: Module unloaded successfully\n");
}
module_init(hello_init);
module_exit(hello_exit);
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] iio: backend: fix uninitialized data in debugfs
2026-06-04 10:38 ` Dan Carpenter
@ 2026-06-04 10:42 ` Dan Carpenter
2026-06-04 14:55 ` Andy Shevchenko
2026-06-04 14:52 ` Andy Shevchenko
1 sibling, 1 reply; 15+ messages in thread
From: Dan Carpenter @ 2026-06-04 10:42 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, Maxwell Doose, Nuno Sa, Olivier Moysan,
David Lechner, Andy Shevchenko, linux-iio, linux-kernel,
kernel-janitors
On Thu, Jun 04, 2026 at 01:38:50PM +0300, Dan Carpenter wrote:
> 168 ret = sscanf(buf, "%i %i", &back->cached_reg_addr, &val);
> ^^^
> Uninitialized variable.
s/variable/data/.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] iio: backend: fix uninitialized data in debugfs
2026-06-04 10:38 ` Dan Carpenter
2026-06-04 10:42 ` Dan Carpenter
@ 2026-06-04 14:52 ` Andy Shevchenko
1 sibling, 0 replies; 15+ messages in thread
From: Andy Shevchenko @ 2026-06-04 14:52 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jonathan Cameron, Maxwell Doose, Nuno Sa, Olivier Moysan,
David Lechner, Andy Shevchenko, linux-iio, linux-kernel,
kernel-janitors
On Thu, Jun 04, 2026 at 01:38:50PM +0300, Dan Carpenter wrote:
> On Thu, Jun 04, 2026 at 11:28:39AM +0300, Andy Shevchenko wrote:
> > On Thu, Jun 04, 2026 at 11:03:15AM +0300, Andy Shevchenko wrote:
> > > On Thu, Jun 04, 2026 at 10:45:36AM +0300, Dan Carpenter wrote:
> > > > On Thu, Jun 04, 2026 at 10:30:34AM +0300, Andy Shevchenko wrote:
> > > > > On Tue, May 26, 2026 at 07:19:46PM +0100, Jonathan Cameron wrote:
> > > > > > On Mon, 25 May 2026 08:20:31 -0500
> > > > > > Maxwell Doose <m32285159@gmail.com> wrote:
> > > > > > > On Mon, May 25, 2026 at 2:17 AM Dan Carpenter <error27@gmail.com> wrote:
> > > > > > > >
> > > > > > > > If the *ppos value is non-zero then simple_write_to_buffer() will not
> > > > > > > > initialize the start of the buf[] buffer. Non-zero ppos values aren't
> > > > > > > > going to work at all. Check for that at the start of the function and
> > > > > > > > return -EINVAL.
> > > > > > >
> > > > > > > Commit message is incorrect, it looks like you're returning -ENOSPC here...
> > > > > > Tweaked and applied to the fixes-togreg branch of iio.git + marked for stable.
> > > > >
> > > > > I was stumbled over these patches by Dan. Since the file_operations for the code
> > > > > in question do not define .llseek, the seek is not supported and will return
> > > > > -ESPIPE. I'm not sure why we have these patches to begin with. Dan, can you
> > > > > elaborate on the case where the *ppos is not 0, please?
> > >
> > > > The simple_write_to_buffer() will update *ppos so partial writes are
> > > > supported in that way.
> > >
> > > Can you show the step-by-step scenario? I'm still fail to see how it may be happen.
> > > Is it somewhere inside the kernel loop? Which VFS function(s) is responsible for
> > > that in such a case?
> >
> > Even if ppos is advanced, the simple_write_to_buffer()
> > https://elixir.bootlin.com/linux/v7.1-rc6/source/fs/libfs.c#L1188
> > won't write more than available in the buffer. Is the available also
> > being advanced somehow?
>
> It's not a buffer overflow, it's an uninitialized data bug.
>
> I used Google AI to create a test case but my qemu system is arm64
> and the test case is in assembly so I'm not sure how useful it is.
> (Also I modified the Google AI code and the test case is garbage).
> My test case writes 10 bytes of data at a time. I've attached the
> test debugfs kernel module.
>
> Here is the code from the kernel:
>
> drivers/iio/industrialio-backend.c
> 149 static ssize_t iio_backend_debugfs_write_reg(struct file *file,
> 150 const char __user *userbuf,
> 151 size_t count, loff_t *ppos)
> 152 {
> 153 struct iio_backend *back = file->private_data;
> 154 unsigned int val;
> 155 char buf[80];
>
> buf is uninitialized. In my test code, I initialized it to all U
> characters which stands for uninitialized.
>
> 156 ssize_t rc;
> 157 int ret;
> 158
> 159 if (*ppos != 0 || count >= sizeof(buf))
> ^^^^^^^^^^
> I added this check on *ppos but imagine it's not there and *ppos is
> non-zero.
>
> 160 return -ENOSPC;
> 161
> 162 rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
>
> The simple_write_to_buffer() function is designed to support partial
> writes so it leaves the first 0 to *ppos characters alone.
If you wrote as many bytes as required, *ppos left on the 'count', and next
writes go after that. So, why *ppos is not 0? Maybe the issue that we need
somewhere reset *ppos to 0?
> 163 if (rc < 0)
> 164 return rc;
> 165
> 166 buf[rc] = '\0';
>
> The return is the number of characters written (starting at *ppos). I'm
> doing 10 character writes so it is 10. The first 10 characters are
> uninitialized. In my test output you can see it prints ten U characters.
>
> 167
> 168 ret = sscanf(buf, "%i %i", &back->cached_reg_addr, &val);
> ^^^
> Uninitialized variable.
>
> 169
> # ./a.out
> Enter a line of text to save: 123456789012345678901234567890
> [ 4266.853201] pos=0 count=10
> [ 4266.853451] rc=10 buf 1234567890
> [ 4266.854774] pos=10 count=10
> Writing to file character by character...
> [ 4266.858582] rc=10 buf UUUUUUUUUU
> [ 4266.858698] pos=20 count=10
> [ 4266.858740] rc=10 buf UUUUUUUUUU
> [ 4266.858798] pos=30 count=10
> [ 4266.858834] rc=10 buf UUUUUUUUUU
> [ 4266.858888] pos=40 count=10
> [ 4266.858924] rc=10 buf UUUUUUUUUU
> [ 4266.859015] pos=50 count=10
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] iio: backend: fix uninitialized data in debugfs
2026-06-04 10:42 ` Dan Carpenter
@ 2026-06-04 14:55 ` Andy Shevchenko
2026-06-05 6:12 ` Dan Carpenter
0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2026-06-04 14:55 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jonathan Cameron, Maxwell Doose, Nuno Sa, Olivier Moysan,
David Lechner, Andy Shevchenko, linux-iio, linux-kernel,
kernel-janitors
On Thu, Jun 04, 2026 at 01:42:11PM +0300, Dan Carpenter wrote:
> On Thu, Jun 04, 2026 at 01:38:50PM +0300, Dan Carpenter wrote:
> > 168 ret = sscanf(buf, "%i %i", &back->cached_reg_addr, &val);
> > ^^^
> > Uninitialized variable.
>
> s/variable/data/.
With what I asked in the previous reply and what you explained there
(thanks, btw!) I still think your patches are not fully correct. They
will require to atomically write all or nothing. If we want support
partial writes we need to go with that differently (reset ppos when
we got enough or more than enough data).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] iio: backend: fix uninitialized data in debugfs
2026-06-04 14:55 ` Andy Shevchenko
@ 2026-06-05 6:12 ` Dan Carpenter
2026-06-05 8:28 ` Andy Shevchenko
0 siblings, 1 reply; 15+ messages in thread
From: Dan Carpenter @ 2026-06-05 6:12 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, Maxwell Doose, Nuno Sa, Olivier Moysan,
David Lechner, Andy Shevchenko, linux-iio, linux-kernel,
kernel-janitors
On Thu, Jun 04, 2026 at 05:55:08PM +0300, Andy Shevchenko wrote:
> On Thu, Jun 04, 2026 at 01:42:11PM +0300, Dan Carpenter wrote:
> > On Thu, Jun 04, 2026 at 01:38:50PM +0300, Dan Carpenter wrote:
> > > 168 ret = sscanf(buf, "%i %i", &back->cached_reg_addr, &val);
> > > ^^^
> > > Uninitialized variable.
> >
> > s/variable/data/.
>
> With what I asked in the previous reply and what you explained there
> (thanks, btw!) I still think your patches are not fully correct. They
> will require to atomically write all or nothing. If we want support
> partial writes we need to go with that differently (reset ppos when
> we got enough or more than enough data).
Requiring writes to syfs and debugfs be atomic is pretty normal and
works well in practice. These are very small writes.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] iio: backend: fix uninitialized data in debugfs
2026-06-05 6:12 ` Dan Carpenter
@ 2026-06-05 8:28 ` Andy Shevchenko
2026-06-05 14:38 ` Maxwell Doose
0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2026-06-05 8:28 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jonathan Cameron, Maxwell Doose, Nuno Sa, Olivier Moysan,
David Lechner, Andy Shevchenko, linux-iio, linux-kernel,
kernel-janitors
On Fri, Jun 05, 2026 at 09:12:38AM +0300, Dan Carpenter wrote:
> On Thu, Jun 04, 2026 at 05:55:08PM +0300, Andy Shevchenko wrote:
> > On Thu, Jun 04, 2026 at 01:42:11PM +0300, Dan Carpenter wrote:
> > > On Thu, Jun 04, 2026 at 01:38:50PM +0300, Dan Carpenter wrote:
> > > > 168 ret = sscanf(buf, "%i %i", &back->cached_reg_addr, &val);
> > > > ^^^
> > > > Uninitialized variable.
> > >
> > > s/variable/data/.
> >
> > With what I asked in the previous reply and what you explained there
> > (thanks, btw!) I still think your patches are not fully correct. They
> > will require to atomically write all or nothing. If we want support
> > partial writes we need to go with that differently (reset ppos when
> > we got enough or more than enough data).
>
> Requiring writes to syfs and debugfs be atomic is pretty normal and
> works well in practice. These are very small writes.
Perhaps. In any case your patch will break existing partial writes, right?
I'm still considering that resetting ppos is the right thing to do. Just
need to find where the best place is to do that.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] iio: backend: fix uninitialized data in debugfs
2026-06-05 8:28 ` Andy Shevchenko
@ 2026-06-05 14:38 ` Maxwell Doose
0 siblings, 0 replies; 15+ messages in thread
From: Maxwell Doose @ 2026-06-05 14:38 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Dan Carpenter, Jonathan Cameron, Nuno Sa, Olivier Moysan,
David Lechner, Andy Shevchenko, linux-iio, linux-kernel,
kernel-janitors
On Fri, Jun 5, 2026 at 3:28 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Fri, Jun 05, 2026 at 09:12:38AM +0300, Dan Carpenter wrote:
> > On Thu, Jun 04, 2026 at 05:55:08PM +0300, Andy Shevchenko wrote:
> > > On Thu, Jun 04, 2026 at 01:42:11PM +0300, Dan Carpenter wrote:
> > > > On Thu, Jun 04, 2026 at 01:38:50PM +0300, Dan Carpenter wrote:
> > > > > 168 ret = sscanf(buf, "%i %i", &back->cached_reg_addr, &val);
> > > > > ^^^
> > > > > Uninitialized variable.
> > > >
> > > > s/variable/data/.
> > >
> > > With what I asked in the previous reply and what you explained there
> > > (thanks, btw!) I still think your patches are not fully correct. They
> > > will require to atomically write all or nothing. If we want support
> > > partial writes we need to go with that differently (reset ppos when
> > > we got enough or more than enough data).
> >
> > Requiring writes to syfs and debugfs be atomic is pretty normal and
> > works well in practice. These are very small writes.
>
> Perhaps. In any case your patch will break existing partial writes, right?
> I'm still considering that resetting ppos is the right thing to do. Just
> need to find where the best place is to do that.
>
I'm certainly no expert on simple_write_to_buffer() but something in
me tells me that writing in the middle of a buffer is a code smell.
But you all seem much more informed in that regard so don't trust me
:)
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-06-05 14:40 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-25 7:16 [PATCH] iio: backend: fix uninitialized data in debugfs Dan Carpenter
2026-05-25 13:20 ` Maxwell Doose
2026-05-26 18:19 ` Jonathan Cameron
2026-06-04 7:30 ` Andy Shevchenko
2026-06-04 7:38 ` Andy Shevchenko
2026-06-04 7:45 ` Dan Carpenter
2026-06-04 8:03 ` Andy Shevchenko
2026-06-04 8:28 ` Andy Shevchenko
2026-06-04 10:38 ` Dan Carpenter
2026-06-04 10:42 ` Dan Carpenter
2026-06-04 14:55 ` Andy Shevchenko
2026-06-05 6:12 ` Dan Carpenter
2026-06-05 8:28 ` Andy Shevchenko
2026-06-05 14:38 ` Maxwell Doose
2026-06-04 14:52 ` Andy Shevchenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.