[PATCH 0/1] rust: configfs: Fix reference creation from uninitialized data in `Attribute::show`

[PATCH 0/1] rust: configfs: Fix reference creation from uninitialized data in `Attribute::show`

[Priya Bala Govindasamy]

Dear Linux kernel maintainers,

We are developing a tool called FerroLens to detect potential 
unsound behavior in Rust code in the Linux kernel. The tool 
internally uses an LLM to detect bugs.
We then perform manual analysis to verify these reports. 
FerroLens reported the following bug in rust/kernel/configfs.rs:

When a configfs attribute is written and then read through the 
same open file, the read reuses the buffer allocated for the write.
This buffer is not zero-initialized. Writes to the attribute go
through `fill_write_buffer()`, which allocates
the buffer with `kmalloc`, and initializes only the input bytes 
and a trailing NUL byte. 
The read in `Attribute::show` callback creates an `&mut [u8; PAGE_SIZE]`
to the buffer. This is unsound because the reference may include 
uninitialized bytes.

Here is a kernel module and a python script that interacts with it 
to demonstrate the bug:

  // SPDX-License-Identifier: GPL-2.0

  //! Rust configfs sample.

  use kernel::alloc::flags;
  use kernel::configfs;
  use kernel::configfs::configfs_attrs;
  use kernel::new_mutex;
  use kernel::page::PAGE_SIZE;
  use kernel::prelude::*;
  use kernel::sync::Mutex;

  module! {
      type: RustConfigfs,
      name: "rust_configfs",
      authors: ["Rust for Linux Contributors"],
      description: "Rust configfs sample",
      license: "GPL",
  }

  #[pin_data]
  struct RustConfigfs {
      #[pin]
      config: configfs::Subsystem<Configuration>,
  }

  #[pin_data]
  struct Configuration {
      message: &'static CStr,
      #[pin]
      bar: Mutex<(KBox<[u8; PAGE_SIZE]>, usize)>,
  }

  impl Configuration {
      fn new() -> impl PinInit<Self, Error> {
          try_pin_init!(Self {
              message: c"Hello World\n",
              bar <- new_mutex!((KBox::new([0; PAGE_SIZE], flags::GFP_KERNEL)?, 0)),
          })
      }
  }

  impl kernel::InPlaceModule for RustConfigfs {
      fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
          pr_info!("Rust configfs sample (init)\n");

          // Define a subsystem with the data type `Configuration`, two
          // attributes, `message` and `bar` and child group type `Child`. `mkdir`
          // in the directory representing this subsystem will create directories
          // backed by the `Child` type.
          let item_type = configfs_attrs! {
              container: configfs::Subsystem<Configuration>,
              data: Configuration,
              child: Child,
              attributes: [
                  message: 0,
                  bar: 1,
              ],
          };

          try_pin_init!(Self {
              config <- configfs::Subsystem::new(
                  c"rust_configfs", item_type, Configuration::new()
              ),
          })
      }
  }

  #[vtable]
  impl configfs::GroupOperations for Configuration {
      type Child = Child;

      fn make_group(&self, name: &CStr) -> Result<impl PinInit<configfs::Group<Child>, Error>> {
          // Define a group with data type `Child`, one attribute `baz` and child
          // group type `GrandChild`. `mkdir` in the directory representing this
          // group will create directories backed by the `GrandChild` type.
          let tpe = configfs_attrs! {
              container: configfs::Group<Child>,
              data: Child,
              child: GrandChild,
              attributes: [
                  baz: 0,
              ],
          };

          Ok(configfs::Group::new(name.try_into()?, tpe, Child::new()))
      }
  }

  #[vtable]
  impl configfs::AttributeOperations<0> for Configuration {
      type Data = Configuration;

      fn show(container: &Configuration, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
          pr_info!("Show message\n");
          let data = container.message.to_bytes();
          page[0..data.len()].copy_from_slice(data);
          Ok(data.len())
      }
  }

  #[vtable]
  impl configfs::AttributeOperations<1> for Configuration {
      type Data = Configuration;

      fn show(container: &Configuration, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
          pr_info!(
              "Show bar: page addr = {:p}, capacity = {} bytes\n",
              page.as_ptr(),
              page.len()
          );

          let is_zeroed = page.iter().all(|&b| b == 0);
          pr_info!("Show bar: page is zero-initialized: {}\n", is_zeroed);

          pr_info!("Show bar: first 8 initial bytes = {:x?}\n", &page[..8]);

          let guard = container.bar.lock();
          let data = guard.0.as_slice();
          let len = guard.1;
          
          if len > PAGE_SIZE {
              return Err(kernel::error::code::EINVAL);
          }

          page[..len].copy_from_slice(&data[..len]);

          // 4. Verify post-write state
          pr_info!("Show bar: wrote {} bytes into page\n", len);

          Ok(len)
      }

      fn store(container: &Configuration, page: &[u8]) -> Result {
          pr_info!(
              "Store bar: container={:#x} bar={:#x}\n",
              container as *const Configuration as usize,
              &container.bar as *const _ as usize,
          );
          let mut guard = container.bar.lock();
          pr_info!(
              "Store bar: interpreted buf={:#x} old_len={}\n",
              guard.0.as_slice().as_ptr() as usize,
              guard.1
          );
          guard.0[0..page.len()].copy_from_slice(page);
          guard.1 = page.len();
          Ok(())
      }
  }

  // `pin_data` cannot handle structs without braces.
  #[pin_data]
  struct Child {}

  impl Child {
      fn new() -> impl PinInit<Self, Error> {
          try_pin_init!(Self {})
      }
  }

  #[vtable]
  impl configfs::GroupOperations for Child {
      type Child = GrandChild;

      fn make_group(&self, name: &CStr) -> Result<impl PinInit<configfs::Group<GrandChild>, Error>> {
          // Define a group with data type `GrandChild`, one attribute `gc`. As no
          // child type is specified, it will not be possible to create subgroups
          // in this group, and `mkdir`in the directory representing this group
          // will return an error.
          let tpe = configfs_attrs! {
              container: configfs::Group<GrandChild>,
              data: GrandChild,
              attributes: [
                  gc: 0,
              ],
          };

          Ok(configfs::Group::new(
              name.try_into()?,
              tpe,
              GrandChild::new(),
          ))
      }
  }

  #[vtable]
  impl configfs::AttributeOperations<0> for Child {
      type Data = Child;

      fn show(_container: &Child, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
          pr_info!("Show baz\n");
          let data = c"Hello Baz\n".to_bytes();
          page[0..data.len()].copy_from_slice(data);
          Ok(data.len())
      }
  }

  // `pin_data` cannot handle structs without braces.
  #[pin_data]
  struct GrandChild {}

  impl GrandChild {
      fn new() -> impl PinInit<Self, Error> {
          try_pin_init!(Self {})
      }
  }

  #[vtable]
  impl configfs::AttributeOperations<0> for GrandChild {
      type Data = GrandChild;

      fn show(_container: &GrandChild, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
          pr_info!("Show grand child\n");
          let data = c"Hello GC\n".to_bytes();
          page[0..data.len()].copy_from_slice(data);
          Ok(data.len())
      }
  }
    
poc.py:

```
import os

fd = os.open("/sys/kernel/config/rust_configfs/bar", os.O_RDWR)
try:
    os.write(fd, b"x")
    os.lseek(fd, 0, os.SEEK_SET)
    print(repr(os.read(fd, 4096)))
finally:
    os.close(fd)
```

output:
[  257.079636] rust_configfs: Rust configfs sample (init)
[  269.778979] rust_configfs: Store bar: container=0xffffffffa0740100 bar=0xffffffffa0740110
[  269.779273] rust_configfs: Store bar: interpreted buf=0xffff888129ce4000 old_len=0
[  269.779332] rust_configfs: Show bar: page addr = 0x00000000fca4e938, capacity = 4096 bytes
[  269.779352] rust_configfs: Show bar: page is zero-initialized: false
[  269.779358] rust_configfs: Show bar: first 8 initial bytes = [78, 0, fb, 4f, 83, 88, ff, ff]
[  269.779374] rust_configfs: Show bar: wrote 1 bytes into page

This output demonstrates that the bytes of the page that have not 
been written to by `Attribute::store` are not zero-initialized, 
which is UB when creating a reference to it.

Priya Bala Govindasamy (1):
  rust: configfs: Fix reference creation from uninitialized data in
    `Attribute::show`

 rust/kernel/configfs.rs | 3 +++
 1 file changed, 3 insertions(+)

-- 
2.34.1

[PATCH] rust: configfs: Fix reference creation from uninitialized data in `Attribute::show`

[Priya Bala Govindasamy]

When a configfs attribute is written and then read through the
same open file, the read reuses the buffer allocated for the write.
This buffer is not zero-initialized. Writes to the attribute go
through `fill_write_buffer()`, which allocates
the buffer with `kmalloc`, and initializes only the input bytes
and a trailing NUL byte.
The read in `Attribute::show` callback creates an `&mut [u8; PAGE_SIZE]`
to the buffer. This is unsound because the reference may include
uninitialized bytes.

Fix this by initializing the contents of the buffer to 0 in the
`Attribute::show` callback before creating a
reference to it.

Fixes: 446cafc295bf ("rust: configfs: introduce rust support for configfs")
Reported-by: Dylan Zueck <dzueck@uci.edu>
Assisted-by: LLM
Signed-off-by: Priya Bala Govindasamy <pgovind2@uci.edu>
---
 rust/kernel/configfs.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
index cd082b83e9e7..3a404e0c54da 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -559,6 +559,11 @@ impl<const ID: u64, O, Data> Attribute<ID, O, Data>
         // the conditions for this call.
         let data: &Data = unsafe { get_group_data(c_group) };
 
+        // SAFETY: The callback's page is writable for PAGE_SIZE bytes.
+        unsafe {
+            core::ptr::write(page.cast::<[u8; PAGE_SIZE]>(), [0u8; PAGE_SIZE]);
+        }
+
         // SAFETY: By function safety requirements, `page` is writable for `PAGE_SIZE`.
         let ret = O::show(data, unsafe { &mut *(page.cast::<[u8; PAGE_SIZE]>()) });
 
-- 
2.34.1

Re: [PATCH 0/1] rust: configfs: Fix reference creation from uninitialized data in `Attribute::show`

[Andreas Hindborg]

Andreas Hindborg <a.hindborg@kernel.org> writes:

> Hi Priya,
>
> "Priya Bala Govindasamy" <pgovind2@uci.edu> writes:
>> Dear Linux kernel maintainers,
>>
>> We are developing a tool called FerroLens to detect potential
>> unsound behavior in Rust code in the Linux kernel. The tool
>> internally uses an LLM to detect bugs.
>> We then perform manual analysis to verify these reports.
>> FerroLens reported the following bug in rust/kernel/configfs.rs:
>>
>> When a configfs attribute is written and then read through the
>> same open file, the read reuses the buffer allocated for the write.
>> This buffer is not zero-initialized. Writes to the attribute go
>> through `fill_write_buffer()`, which allocates
>> the buffer with `kmalloc`, and initializes only the input bytes
>> and a trailing NUL byte.
>> The read in `Attribute::show` callback creates an `&mut [u8; PAGE_SIZE]`
>> to the buffer. This is unsound because the reference may include
>> uninitialized bytes.
>>
>> Here is a kernel module and a python script that interacts with it
>> to demonstrate the bug:
>>
>>   // SPDX-License-Identifier: GPL-2.0
>>
>>   //! Rust configfs sample.
>>
>>   use kernel::alloc::flags;
>>   use kernel::configfs;
>>   use kernel::configfs::configfs_attrs;
>>   use kernel::new_mutex;
>>   use kernel::page::PAGE_SIZE;
>>   use kernel::prelude::*;
>>   use kernel::sync::Mutex;
>>
>>   module! {
>>       type: RustConfigfs,
>>       name: "rust_configfs",
>>       authors: ["Rust for Linux Contributors"],
>>       description: "Rust configfs sample",
>>       license: "GPL",
>>   }
>>
>>   #[pin_data]
>>   struct RustConfigfs {
>>       #[pin]
>>       config: configfs::Subsystem<Configuration>,
>>   }
>>
>>   #[pin_data]
>>   struct Configuration {
>>       message: &'static CStr,
>>       #[pin]
>>       bar: Mutex<(KBox<[u8; PAGE_SIZE]>, usize)>,
>>   }
>>
>>   impl Configuration {
>>       fn new() -> impl PinInit<Self, Error> {
>>           try_pin_init!(Self {
>>               message: c"Hello World\n",
>>               bar <- new_mutex!((KBox::new([0; PAGE_SIZE], flags::GFP_KERNEL)?, 0)),
>>           })
>>       }
>>   }
>>
>>   impl kernel::InPlaceModule for RustConfigfs {
>>       fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
>>           pr_info!("Rust configfs sample (init)\n");
>>
>>           // Define a subsystem with the data type `Configuration`, two
>>           // attributes, `message` and `bar` and child group type `Child`. `mkdir`
>>           // in the directory representing this subsystem will create directories
>>           // backed by the `Child` type.
>>           let item_type = configfs_attrs! {
>>               container: configfs::Subsystem<Configuration>,
>>               data: Configuration,
>>               child: Child,
>>               attributes: [
>>                   message: 0,
>>                   bar: 1,
>>               ],
>>           };
>>
>>           try_pin_init!(Self {
>>               config <- configfs::Subsystem::new(
>>                   c"rust_configfs", item_type, Configuration::new()
>>               ),
>>           })
>>       }
>>   }
>>
>>   #[vtable]
>>   impl configfs::GroupOperations for Configuration {
>>       type Child = Child;
>>
>>       fn make_group(&self, name: &CStr) -> Result<impl PinInit<configfs::Group<Child>, Error>> {
>>           // Define a group with data type `Child`, one attribute `baz` and child
>>           // group type `GrandChild`. `mkdir` in the directory representing this
>>           // group will create directories backed by the `GrandChild` type.
>>           let tpe = configfs_attrs! {
>>               container: configfs::Group<Child>,
>>               data: Child,
>>               child: GrandChild,
>>               attributes: [
>>                   baz: 0,
>>               ],
>>           };
>>
>>           Ok(configfs::Group::new(name.try_into()?, tpe, Child::new()))
>>       }
>>   }
>>
>>   #[vtable]
>>   impl configfs::AttributeOperations<0> for Configuration {
>>       type Data = Configuration;
>>
>>       fn show(container: &Configuration, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
>>           pr_info!("Show message\n");
>>           let data = container.message.to_bytes();
>>           page[0..data.len()].copy_from_slice(data);
>>           Ok(data.len())
>>       }
>>   }
>>
>>   #[vtable]
>>   impl configfs::AttributeOperations<1> for Configuration {
>>       type Data = Configuration;
>>
>>       fn show(container: &Configuration, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
>>           pr_info!(
>>               "Show bar: page addr = {:p}, capacity = {} bytes\n",
>>               page.as_ptr(),
>>               page.len()
>>           );
>>
>>           let is_zeroed = page.iter().all(|&b| b == 0);
>>           pr_info!("Show bar: page is zero-initialized: {}\n", is_zeroed);
>>
>>           pr_info!("Show bar: first 8 initial bytes = {:x?}\n", &page[..8]);
>>
>>           let guard = container.bar.lock();
>>           let data = guard.0.as_slice();
>>           let len = guard.1;
>>
>>           if len > PAGE_SIZE {
>>               return Err(kernel::error::code::EINVAL);
>>           }
>>
>>           page[..len].copy_from_slice(&data[..len]);
>>
>>           // 4. Verify post-write state
>>           pr_info!("Show bar: wrote {} bytes into page\n", len);
>>
>>           Ok(len)
>>       }
>>
>>       fn store(container: &Configuration, page: &[u8]) -> Result {
>>           pr_info!(
>>               "Store bar: container={:#x} bar={:#x}\n",
>>               container as *const Configuration as usize,
>>               &container.bar as *const _ as usize,
>>           );
>>           let mut guard = container.bar.lock();
>>           pr_info!(
>>               "Store bar: interpreted buf={:#x} old_len={}\n",
>>               guard.0.as_slice().as_ptr() as usize,
>>               guard.1
>>           );
>>           guard.0[0..page.len()].copy_from_slice(page);
>>           guard.1 = page.len();
>>           Ok(())
>>       }
>>   }
>>
>>   // `pin_data` cannot handle structs without braces.
>>   #[pin_data]
>>   struct Child {}
>>
>>   impl Child {
>>       fn new() -> impl PinInit<Self, Error> {
>>           try_pin_init!(Self {})
>>       }
>>   }
>>
>>   #[vtable]
>>   impl configfs::GroupOperations for Child {
>>       type Child = GrandChild;
>>
>>       fn make_group(&self, name: &CStr) -> Result<impl PinInit<configfs::Group<GrandChild>, Error>> {
>>           // Define a group with data type `GrandChild`, one attribute `gc`. As no
>>           // child type is specified, it will not be possible to create subgroups
>>           // in this group, and `mkdir`in the directory representing this group
>>           // will return an error.
>>           let tpe = configfs_attrs! {
>>               container: configfs::Group<GrandChild>,
>>               data: GrandChild,
>>               attributes: [
>>                   gc: 0,
>>               ],
>>           };
>>
>>           Ok(configfs::Group::new(
>>               name.try_into()?,
>>               tpe,
>>               GrandChild::new(),
>>           ))
>>       }
>>   }
>>
>>   #[vtable]
>>   impl configfs::AttributeOperations<0> for Child {
>>       type Data = Child;
>>
>>       fn show(_container: &Child, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
>>           pr_info!("Show baz\n");
>>           let data = c"Hello Baz\n".to_bytes();
>>           page[0..data.len()].copy_from_slice(data);
>>           Ok(data.len())
>>       }
>>   }
>>
>>   // `pin_data` cannot handle structs without braces.
>>   #[pin_data]
>>   struct GrandChild {}
>>
>>   impl GrandChild {
>>       fn new() -> impl PinInit<Self, Error> {
>>           try_pin_init!(Self {})
>>       }
>>   }
>>
>>   #[vtable]
>>   impl configfs::AttributeOperations<0> for GrandChild {
>>       type Data = GrandChild;
>>
>>       fn show(_container: &GrandChild, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
>>           pr_info!("Show grand child\n");
>>           let data = c"Hello GC\n".to_bytes();
>>           page[0..data.len()].copy_from_slice(data);
>>           Ok(data.len())
>>       }
>>   }
>>
>> poc.py:
>>
>> ```
>> import os
>>
>> fd = os.open("/sys/kernel/config/rust_configfs/bar", os.O_RDWR)
>> try:
>>     os.write(fd, b"x")
>>     os.lseek(fd, 0, os.SEEK_SET)
>>     print(repr(os.read(fd, 4096)))
>> finally:
>>     os.close(fd)
>> ```
>>
>> output:
>> [  257.079636] rust_configfs: Rust configfs sample (init)
>> [  269.778979] rust_configfs: Store bar: container=0xffffffffa0740100 bar=0xffffffffa0740110
>> [  269.779273] rust_configfs: Store bar: interpreted buf=0xffff888129ce4000 old_len=0
>> [  269.779332] rust_configfs: Show bar: page addr = 0x00000000fca4e938, capacity = 4096 bytes
>> [  269.779352] rust_configfs: Show bar: page is zero-initialized: false
>> [  269.779358] rust_configfs: Show bar: first 8 initial bytes = [78, 0, fb, 4f, 83, 88, ff, ff]
>> [  269.779374] rust_configfs: Show bar: wrote 1 bytes into page
>>
>> This output demonstrates that the bytes of the page that have not
>> been written to by `Attribute::store` are not zero-initialized,
>> which is UB when creating a reference to it.
>
> The buffer is zeroed on allocation during file open, so all the bytes in
> the page are initialized.
>
> The fact that bytes are not zeroed in a write -> seek -> read sequence
> does not mean they are not initialized. They are just not zero.

Actually there may be a problem, but I think it may be in C configfs. If
you take a look at the function that reads data from the iov_iter:

  static int fill_write_buffer(struct configfs_buffer *buffer,
            struct iov_iter *from)
  {
    int copied;

    if (!buffer->page)
      buffer->page = kmalloc(PAGE_SIZE, GFP_KERNEL); // <- HERE
    if (!buffer->page)
      return -ENOMEM;

    copied = copy_from_iter(buffer->page, SIMPLE_ATTR_SIZE - 1, from);
    buffer->needs_read_fill = 1;
    /* if buf is assumed to contain a string, terminate it by \0,
    * so e.g. sscanf() can scan the string easily */
    buffer->page[copied] = 0;
    return copied ? : -EFAULT;
  }

This function does not zero the page that is written into. This,
combined with the buffer being per file handle means that you can read
the original data in the page. The `kmalloc` should be probably be
replaced with a `kzalloc`. This should be a problem for C modules as
well.

Cc: Breno Leitao <leitao@debian.org>

Looks like I only replied Priya when replying, so I'm adding back
everyone from the original email.

Best regards,
Andreas Hindborg

Re: [PATCH 0/1] rust: configfs: Fix reference creation from uninitialized data in `Attribute::show`

[Gary Guo]

On Fri Oct 9, 2026 at 1:49 PM BST, Andreas Hindborg wrote:
> Andreas Hindborg <a.hindborg@kernel.org> writes:
>
> Actually there may be a problem, but I think it may be in C configfs. If
> you take a look at the function that reads data from the iov_iter:
>
>   static int fill_write_buffer(struct configfs_buffer *buffer,
>             struct iov_iter *from)
>   {
>     int copied;
>
>     if (!buffer->page)
>       buffer->page = kmalloc(PAGE_SIZE, GFP_KERNEL); // <- HERE
>     if (!buffer->page)
>       return -ENOMEM;
>
>     copied = copy_from_iter(buffer->page, SIMPLE_ATTR_SIZE - 1, from);
>     buffer->needs_read_fill = 1;
>     /* if buf is assumed to contain a string, terminate it by \0,
>     * so e.g. sscanf() can scan the string easily */
>     buffer->page[copied] = 0;
>     return copied ? : -EFAULT;
>   }
>
> This function does not zero the page that is written into. This,
> combined with the buffer being per file handle means that you can read
> the original data in the page. The `kmalloc` should be probably be
> replaced with a `kzalloc`. This should be a problem for C modules as
> well.

Why is this an issue? The extra uninitialized bytes should not be used.

Best,
Gary

>
> Cc: Breno Leitao <leitao@debian.org>
>
> Looks like I only replied Priya when replying, so I'm adding back
> everyone from the original email.
>
> Best regards,
> Andreas Hindborg

Re: [PATCH 0/1] rust: configfs: Fix reference creation from uninitialized data in `Attribute::show`

[Andreas Hindborg]

"Priya Bala Govindasamy" <pgovind2@uci.edu> writes:

> Hi Gary,
>
> `Attribute::show` creates an `&mut [u8; PAGE_SIZE]` over a page that may be only partially initialized. Rust's array
> documentation requires all elements of an array to be initialized [1]. So this doesn't meet the documented requirements.
> Moreover, `AttributeOperations::show` is a safe trait method and could be implemented to read any bytes from that array
> without using `unsafe`. Reading an uninitialized byte would be undefined behavior. 
>
> [1] https://doc.rust-lang.org/reference/types/array.html

This was HTML, so the list will drop it.


Best regards,
Andreas Hindborg

Re: [PATCH 0/1] rust: configfs: Fix reference creation from uninitialized data in `Attribute::show`

[Andreas Hindborg]

"Gary Guo" <gary@garyguo.net> writes:

> On Fri Oct 9, 2026 at 1:49 PM BST, Andreas Hindborg wrote:
>> Andreas Hindborg <a.hindborg@kernel.org> writes:
>>
>> Actually there may be a problem, but I think it may be in C configfs. If
>> you take a look at the function that reads data from the iov_iter:
>>
>>   static int fill_write_buffer(struct configfs_buffer *buffer,
>>             struct iov_iter *from)
>>   {
>>     int copied;
>>
>>     if (!buffer->page)
>>       buffer->page = kmalloc(PAGE_SIZE, GFP_KERNEL); // <- HERE
>>     if (!buffer->page)
>>       return -ENOMEM;
>>
>>     copied = copy_from_iter(buffer->page, SIMPLE_ATTR_SIZE - 1, from);
>>     buffer->needs_read_fill = 1;
>>     /* if buf is assumed to contain a string, terminate it by \0,
>>     * so e.g. sscanf() can scan the string easily */
>>     buffer->page[copied] = 0;
>>     return copied ? : -EFAULT;
>>   }
>>
>> This function does not zero the page that is written into. This,
>> combined with the buffer being per file handle means that you can read
>> the original data in the page. The `kmalloc` should be probably be
>> replaced with a `kzalloc`. This should be a problem for C modules as
>> well.
>
> Why is this an issue? The extra uninitialized bytes should not be used.

I guess you are right. In C, a driver just promise to not read beyond
the write count. At any rate, it is a cheap defensive mechanism to
kzalloc this buffer. It would prevent leaking uninitialized data to user
space under a wrong length returned by the `show` implementation after a
`store` operation.

In rust it is an issue because the *driver* _may_ read the uninitialized
bytes.

So we can fix it on the Rust side with this patch from Priya. Or we can
initialize the buffer on allocation in C.

For the record, I did not notice that the buffer is per file open, which
is why the bug is here in the first place.

@Breno, should we zero this buffer on initialization on the C side, or
do we just zero the buffer before calling into Rust driver code?

Best regards,
Andreas Hindborg

Re: [PATCH 0/1] rust: configfs: Fix reference creation from uninitialized data in `Attribute::show`

[Gary Guo]

On Fri Oct 9, 2026 at 9:32 PM BST, Andreas Hindborg wrote:
> "Gary Guo" <gary@garyguo.net> writes:
>
>> On Fri Oct 9, 2026 at 1:49 PM BST, Andreas Hindborg wrote:
>>> Andreas Hindborg <a.hindborg@kernel.org> writes:
>>>
>>> Actually there may be a problem, but I think it may be in C configfs. If
>>> you take a look at the function that reads data from the iov_iter:
>>>
>>>   static int fill_write_buffer(struct configfs_buffer *buffer,
>>>             struct iov_iter *from)
>>>   {
>>>     int copied;
>>>
>>>     if (!buffer->page)
>>>       buffer->page = kmalloc(PAGE_SIZE, GFP_KERNEL); // <- HERE
>>>     if (!buffer->page)
>>>       return -ENOMEM;
>>>
>>>     copied = copy_from_iter(buffer->page, SIMPLE_ATTR_SIZE - 1, from);
>>>     buffer->needs_read_fill = 1;
>>>     /* if buf is assumed to contain a string, terminate it by \0,
>>>     * so e.g. sscanf() can scan the string easily */
>>>     buffer->page[copied] = 0;
>>>     return copied ? : -EFAULT;
>>>   }
>>>
>>> This function does not zero the page that is written into. This,
>>> combined with the buffer being per file handle means that you can read
>>> the original data in the page. The `kmalloc` should be probably be
>>> replaced with a `kzalloc`. This should be a problem for C modules as
>>> well.
>>
>> Why is this an issue? The extra uninitialized bytes should not be used.
>
> I guess you are right. In C, a driver just promise to not read beyond
> the write count. At any rate, it is a cheap defensive mechanism to
> kzalloc this buffer. It would prevent leaking uninitialized data to user
> space under a wrong length returned by the `show` implementation after a
> `store` operation.
>
> In rust it is an issue because the *driver* _may_ read the uninitialized
> bytes.

Well, the driver shouldn't read the bytes at all. It is supposed to *write* to
the slice. We probably want something like
https://docs.rs/tokio/latest/tokio/io/struct.ReadBuf.html 
instead of a raw slice.

(Also, the name "page" for variable name is a bad one, it leaks the
implementation detail of configfs. This is just a buffer to avoid having drivers
dealing with iov itself.)

>
> So we can fix it on the Rust side with this patch from Priya. Or we can
> initialize the buffer on allocation in C.

Not really, we can just "declare" all bytes allocated by kmalloc is initialized,
but to unpredictable values.

>
> For the record, I did not notice that the buffer is per file open, which
> is why the bug is here in the first place.
>
> @Breno, should we zero this buffer on initialization on the C side, or
> do we just zero the buffer before calling into Rust driver code?

Of course, leaking any bytes to userspace is bad because this is kernel memory.
So one can argue that zeroing it would be a good hardening measure.

Best,
Gary

Re: [PATCH 0/1] rust: configfs: Fix reference creation from uninitialized data in `Attribute::show`

[Priya Bala Govindasamy]

Resending in plain text. Sorry for the duplicate.

Hi Gary,

`Attribute::show` creates an `&mut [u8; PAGE_SIZE]` over a page that
may be only partially initialized. Rust's array documentation requires
all elements of an array to be initialized [1]. So this doesn't meet
the documented requirements. Moreover, `AttributeOperations::show` is
a safe trait method and could be implemented to read any bytes from
that array without using `unsafe`. Reading an uninitialized byte would
be undefined behavior.

[1] https://doc.rust-lang.org/reference/types/array.html


On Fri, Oct 9, 2026 at 5:54 AM Gary Guo <gary@garyguo.net> wrote:
>
> On Fri Oct 9, 2026 at 1:49 PM BST, Andreas Hindborg wrote:
> > Andreas Hindborg <a.hindborg@kernel.org> writes:
> >
> > Actually there may be a problem, but I think it may be in C configfs. If
> > you take a look at the function that reads data from the iov_iter:
> >
> >   static int fill_write_buffer(struct configfs_buffer *buffer,
> >             struct iov_iter *from)
> >   {
> >     int copied;
> >
> >     if (!buffer->page)
> >       buffer->page = kmalloc(PAGE_SIZE, GFP_KERNEL); // <- HERE
> >     if (!buffer->page)
> >       return -ENOMEM;
> >
> >     copied = copy_from_iter(buffer->page, SIMPLE_ATTR_SIZE - 1, from);
> >     buffer->needs_read_fill = 1;
> >     /* if buf is assumed to contain a string, terminate it by \0,
> >     * so e.g. sscanf() can scan the string easily */
> >     buffer->page[copied] = 0;
> >     return copied ? : -EFAULT;
> >   }
> >
> > This function does not zero the page that is written into. This,
> > combined with the buffer being per file handle means that you can read
> > the original data in the page. The `kmalloc` should be probably be
> > replaced with a `kzalloc`. This should be a problem for C modules as
> > well.
>
> Why is this an issue? The extra uninitialized bytes should not be used.
>
> Best,
> Gary
>
> >
> > Cc: Breno Leitao <leitao@debian.org>
> >
> > Looks like I only replied Priya when replying, so I'm adding back
> > everyone from the original email.
> >
> > Best regards,
> > Andreas Hindborg
>
>