[PATCH 0/1] rust: configfs: Fix incorrect subsystem root check in `get_group_data`

[PATCH 0/1] rust: configfs: Fix incorrect subsystem root check in `get_group_data`

[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:

In `get_group_data` to determine whether `this` is 
subsystem root, the following check is performed:
`(*this).cg_subsys.is_null()
`get_group_data` then tries to read the `data` field of either
`Group` or `Subsystem` structs depending on the result of the check.

But the check always evaluates to false in `get_group_data`, since
`link_group` in C always populates `group->cg_subsys` with a valid, 
non-null pointer.
Thus, if `this` points to a group representing the subsytem root,
the incorrect branch in `get_group_data` may be taken, 
leading to attempting to access `Subsystem.data` as `Group.data`.
This is type confusion that can lead to data corruption.

If the `data` fields of both structs are placed at the same offset,
we will not observe this issue, since the attempt to access the 
`data` field will be valid regardless of the type. But this layout 
is not guaranteed by the compiler.

In particular, if the kernel is built with a randomized layout 
(e.g with KRUSTFLAGS='-Zrandomize-layout=yes -Zlayout-seed=1'), 
We observe incorrect behavior when using the rust_configfs 
module from samples.

$ sudo insmod samples/rust/rust_configfs.ko
$ sudo cat /sys/kernel/config/rust_configfs/message
Segmentation fault (core dumped)

[  365.684128] rust_kernel: panicked at samples/rust/rust_configfs.rs:97:13:
               range end index 18446744072104944063 out of range for slice of length 4096
[  365.684458] ------------[ cut here ]------------
[  365.684468] kernel BUG at rust/helpers/bug.c:7!
[  365.684525] Oops: invalid opcode: 0000 [#1] SMP KASAN PTI
[  365.690063] CPU: 10 UID: 0 PID: 4686 Comm: cat Tainted: G            E    N  7.3.0-rc3+ #54 PREEMPT(lazy) 
[  365.691501] Tainted: [E]=UNSIGNED_MODULE, [N]=TEST
[  365.692323] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
[  365.693700] RIP: 0010:rust_helper_BUG+0x8/0x10
[  365.694437] Code: ff 5d c3 cc cc cc cc cc 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 55 48 89 e5 <0f> 0b 66 0f 1f 44 00 00 90 90 90 90 90 90 90 9090 90 90 90 90 90
[  365.697372] RSP: 0018:ffff88810fc4fad8 EFLAGS: 00010286
[  365.698177] RAX: 0000000000000088 RBX: ffff88810fc4fb00 RCX: 79cfa39b35b7ef00
[  365.699252] RDX: 0000000000000001 RSI: 0000000000000004 RDI: 00000000ffffffff
[  365.700371] RBP: ffff88810fc4fad8 R08: ffff8883ad7fd553 R09: 1ffff11075affaaa
[  365.701440] R10: dffffc0000000000 R11: ffffed1075affaab R12: dffffc0000000000
[  365.702524] R13: ffffffffa07400c0 R14: dffffc0000000000 R15: ffffffffa05ba190
[  365.703642] FS:  00007f30f23c3740(0000) GS:ffff888426a46000(0000) knlGS:0000000000000000
[  365.704994] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  365.705890] CR2: 00007f30f23a1000 CR3: 00000002b2d16001 CR4: 0000000000372ef0
[  365.707001] Call Trace:
[  365.707404]  <TASK>
[  365.707797]  _RNvCsGIExRX8pES_7___rustc17rust_begin_unwind+0x8b/0x90
[  365.708795]  ? __pfx__RNvXs6_NtCsbuTvttuFvbr_6kernel3fmtINtB5_7AdapterRRNtNtNtCsjYlAz7NZ3Sw_4core5panic10panic_info9PanicInfoENtNtBS_3fmt7Display3fmtB7_+0x10/0x10
[  365.711020]  _RNvNtCsjYlAz7NZ3Sw_4core9panicking9panic_fmt+0x5a/0x60
[  365.712125]  ? vpanic+0x6c1/0x710
[  365.712635]  _RNvNtNtCsjYlAz7NZ3Sw_4core5slice5index16slice_index_fail+0x1cc/0x1d0
[  365.713776]  ? __pfx__RNvXsi_NtNtNtCsjYlAz7NZ3Sw_4core3fmt3num3impjNtB9_7Display3fmt+0x10/0x10
[  365.715149]  ? __pfx__RNvXsi_NtNtNtCsjYlAz7NZ3Sw_4core3fmt3num3impjNtB9_7Display3fmt+0x10/0x10
[  365.716487]  ? _RNvMs4_NtCsbuTvttuFvbr_6kernel8configfsINtB5_21GroupOperationsVTableNtCs1nKPhsfELFc_13rust_configfs13ConfigurationNtB16_5ChildE9drop_itemB16_+0x1f/0x20 [rust_configfs]
[  365.718991]  ? _RNvMs4_NtCsbuTvttuFvbr_6kernel8configfsINtB5_21GroupOperationsVTableNtCs1nKPhsfELFc_13rust_configfs13ConfigurationNtB16_5ChildE9drop_itemB16_+0x1f/0x20 [rust_configfs]
[  365.721617]  _RNvMs9_NtCsbuTvttuFvbr_6kernel8configfsINtB5_9AttributeKy0_NtCs1nKPhsfELFc_13rust_configfs13ConfigurationBV_E4showBX_+0xf1/0x110 [rust_configfs]
[  365.723789]  configfs_read_iter+0x285/0x580
[  365.724621]  ? __pfx__RNvMs9_NtCsbuTvttuFvbr_6kernel8configfsINtB5_9AttributeKy0_NtCs1nKPhsfELFc_13rust_configfs13ConfigurationBV_E4showBX_+0x10/0x10 [rust_configfs]
[  365.726836]  vfs_read+0x6e0/0x8e0
[  365.727402]  ksys_read+0xbc/0x170
[  365.728049]  __x64_sys_read+0x7f/0x90
[  365.728644]  x64_sys_call+0x2180/0x2600
[  365.729255]  do_syscall_64+0x11c/0x3f0
[  365.729848]  ? __this_cpu_preempt_check+0x17/0x20
[  365.730565]  ? debug_smp_processor_id+0x1b/0x30
[  365.731251]  ? debug_smp_processor_id+0x1b/0x30
[  365.732026]  ? rcu_is_watching+0x1a/0xc0
[  365.732646]  ? trace_irq_disable+0x3c/0x130
[  365.733302]  ? do_syscall_64+0x6d/0x3f0
[  365.733876]  ? entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  365.734657]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  365.735401] RIP: 0033:0x7f30f2114932
[  365.736044] Code: c0 e9 b2 fe ff ff 50 48 8d 3d 5a b3 0c 00 e8 e5 1c 02 00 0f 1f 44 00 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 0f 05 <48> 3d 00 f0 ff ff 77 56 c3 0f 1f 44 00 00 48 83ec 28 48 89 54 24
[  365.738750] RSP: 002b:00007ffc90983bb8 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
[  365.739953] RAX: ffffffffffffffda RBX: 0000000000020000 RCX: 00007f30f2114932
[  365.741121] RDX: 0000000000020000 RSI: 00007f30f23a2000 RDI: 0000000000000003
[  365.742409] RBP: 00007f30f23a2000 R08: 00007f30f23a1010 R09: 00007f30f23a1010
[  365.743448] R10: 0000000000000022 R11: 0000000000000246 R12: 0000000000022000
[  365.744690] R13: 0000000000000003 R14: 0000000000020000 R15: 0000000000020000
[  365.745771]  </TASK>
[  365.746104] Modules linked in: rust_configfs(E) binfmt_misc intel_rapl_msr intel_rapl_common kvm_intel kvm rapl joydev i2c_piix4 e1000 i2c_smbus floppy qemu_fw_cfg sch_fq_codel dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua msr efi_pstore autofs4 raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor xor async_tx raid6_pq raid1 raid0 aesni_intel gf128mul psmouse input_leds serio_raw mac_hid pata_acpi
[  365.751834] ---[ end trace 0000000000000000 ]---
[  365.752720] RIP: 0010:rust_helper_BUG+0x8/0x10
[  365.752746] Code: ff 5d c3 cc cc cc cc cc 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 55 48 89 e5 <0f> 0b 66 0f 1f 44 00 00 90 90 90 90 90 90 90 9090 90 90 90 90 90
[  365.752751] RSP: 0018:ffff88810fc4fad8 EFLAGS: 00010286
[  365.752759] RAX: 0000000000000088 RBX: ffff88810fc4fb00 RCX: 79cfa39b35b7ef00
[  365.752763] RDX: 0000000000000001 RSI: 0000000000000004 RDI: 00000000ffffffff
[  365.752765] RBP: ffff88810fc4fad8 R08: ffff8883ad7fd553 R09: 1ffff11075affaaa
[  365.752768] R10: dffffc0000000000 R11: ffffed1075affaab R12: dffffc0000000000
[  365.752770] R13: ffffffffa07400c0 R14: dffffc0000000000 R15: ffffffffa05ba190
[  365.752773] FS:  00007f30f23c3740(0000) GS:ffff888426a46000(0000) knlGS:0000000000000000
[  365.752776] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  365.752778] CR2: 00007f30f23a1000 CR3: 00000002b2d16001 CR4: 0000000000372ef0


Priya Bala Govindasamy (1):
  rust: configfs: Fix incorrect subsystem root check in `get_group_data`

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

-- 
2.34.1

[PATCH 1/1] rust: configfs: Fix incorrect subsystem root check in `get_group_data`

[Priya Bala Govindasamy]

In `get_group_data` to determine whether `this` is 
subsystem root, the following check is performed:
`(*this).cg_subsys.is_null()
`get_group_data` then tries to read the `data` field of either
`Group` or `Subsystem` structs depending on the result of the check.

But the check always evaluates to false in `get_group_data`, since
`link_group` in C always populates group->cg_subsys with a valid, 
non-null pointer.
Thus, if `this` points to a group representing the subsytem root,
the incorrect branch in `get_group_data` may be taken, 
leading to attempting to access `Subsystem.data` as `Group.data`.
This is type confusion that can lead to data corruption.

Fix this by comparing the group's address with its subsystem's su_group
and updating the safety contract of `get_group_data`.

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

diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
index cd082b83e9e7..c5fdafde0107 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -295,7 +295,8 @@ unsafe fn container_of(group: *const bindings::config_group) -> *const Self {
 
 /// # Safety
 ///
-/// `this` must be a valid pointer.
+/// `this` must be a valid pointer to a group linked into a registered configfs
+/// subsystem. Its `cg_subsys` must be non-null and valid.
 ///
 /// If `this` does not represent the root group of a configfs subsystem,
 /// `this` must be a pointer to a `bindings::config_group` embedded in a
@@ -304,9 +305,15 @@ unsafe fn container_of(group: *const bindings::config_group) -> *const Self {
 /// Otherwise, `this` must be a pointer to a `bindings::config_group` that
 /// is embedded in a `bindings::configfs_subsystem` that is embedded in a
 /// `Subsystem<Parent>`.
+///
+/// The containing `Group<Parent>` or `Subsystem<Parent>` and its initialized
+/// `data` must remain valid for shared access for `'a`.
 unsafe fn get_group_data<'a, Parent>(this: *mut bindings::config_group) -> &'a Parent {
-    // SAFETY: `this` is a valid pointer.
-    let is_root = unsafe { (*this).cg_subsys.is_null() };
+    // SAFETY: `this` and its `cg_subsys` are valid by the function's safety requirements.
+    let is_root = unsafe {
+        let subsys = (*this).cg_subsys;
+        core::ptr::eq(this, &raw const (*subsys).su_group)
+    };
 
     if !is_root {
         // SAFETY: By C API contact,`this` was returned from a call to
-- 
2.34.1

Re: [PATCH 0/1] rust: configfs: Fix incorrect subsystem root check in `get_group_data`

[Andreas Hindborg]

"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:
>
> In `get_group_data` to determine whether `this` is
> subsystem root, the following check is performed:
> `(*this).cg_subsys.is_null()
> `get_group_data` then tries to read the `data` field of either
> `Group` or `Subsystem` structs depending on the result of the check.
>
> But the check always evaluates to false in `get_group_data`, since
> `link_group` in C always populates `group->cg_subsys` with a valid,
> non-null pointer.
> Thus, if `this` points to a group representing the subsytem root,
> the incorrect branch in `get_group_data` may be taken,
> leading to attempting to access `Subsystem.data` as `Group.data`.
> This is type confusion that can lead to data corruption.
>
> If the `data` fields of both structs are placed at the same offset,
> we will not observe this issue, since the attempt to access the
> `data` field will be valid regardless of the type. But this layout
> is not guaranteed by the compiler.
>
> In particular, if the kernel is built with a randomized layout
> (e.g with KRUSTFLAGS='-Zrandomize-layout=yes -Zlayout-seed=1'),
> We observe incorrect behavior when using the rust_configfs
> module from samples.

Thank you for the patch. I think it was already reported here [1].

Best regards,
Andreas Hindborg

[1] https://lore.kernel.org/all/20260608-configfs-fix-offset-v1-1-7f01b8fb9e5f@kernel.org/