rust: configfs: Fix incorrect subsystem root check in `get_group_data`

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

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.

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

index cd082b83e9e7..c5fdafde0107 100644
@@ -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

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

Best regards,
Andreas Hindborg

[1] https://lore.kernel.org/all/[email protected]/