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

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 <[email protected]> wrote:
> > > > > > > On Mon, May 25, 2026 at 2:17 AM Dan Carpenter <[email protected]> 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