mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dan Carpenter <error27@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: Jonathan Cameron <jic23@kernel.org>,
	Maxwell Doose <m32285159@gmail.com>, Nuno Sa <nuno.sa@analog.com>,
	Olivier Moysan <olivier.moysan@foss.st.com>,
	David Lechner <dlechner@baylibre.com>,
	Andy Shevchenko <andy@kernel.org>,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] iio: backend: fix uninitialized data in debugfs
Date: Thu, 4 Jun 2026 13:38:50 +0300	[thread overview]
Message-ID: <aiFVuvVtaWbjfVzA@stanley.mountain> (raw)
In-Reply-To: <aiE3N6zErq2WcCvS@ashevche-desk.local>

[-- 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);

  reply	other threads:[~2026-06-04 10:38 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-25  7:16 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 [this message]
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-08 20:31                       ` Nuno Sá
2026-06-08 21:51                         ` Maxwell Doose
2026-06-09  8:10                           ` Nuno Sá
2026-06-09 12:16                             ` Andy Shevchenko
2026-06-09 13:09                             ` Jonathan Cameron
2026-06-09  6:21                         ` Dan Carpenter
2026-06-09  6:19                       ` Dan Carpenter
2026-06-10 10:41                         ` Andy Shevchenko
2026-06-10 12:19                           ` Dan Carpenter
2026-06-10 13:44                             ` Andy Shevchenko
2026-06-04 14:52               ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aiFVuvVtaWbjfVzA@stanley.mountain \
    --to=error27@gmail.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=andy@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m32285159@gmail.com \
    --cc=nuno.sa@analog.com \
    --cc=olivier.moysan@foss.st.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox