security/tomoyo/util.c | 2 ++ 1 file changed, 2 insertions(+)
tomoyo_domain() can return NULL when the current task has no TOMOYO
domain_info set. When this happens, tomoyo_init_request_info() sets
r->domain = NULL and then dereferences the NULL domain via
domain->profile and later domain->acl_info_list in tomoyo_check_acl(),
causing a kernel page fault.
Add a NULL check after tomoyo_domain() and return TOMOYO_CONFIG_DISABLED
when domain is NULL. All callers that can reach this path already check
for TOMOYO_CONFIG_DISABLED and bail out, so this prevents the crash
without changing the control flow for those callers.
Fixes: c3ef1500ec8338 ("TOMOYO: Split files into some pieces.")
Signed-off-by: Jiakai Xu <xujiakai24@mails.ucas.ac.cn>
---
security/tomoyo/util.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/security/tomoyo/util.c b/security/tomoyo/util.c
index 6799b1122c9d8..cdc085390949c 100644
--- a/security/tomoyo/util.c
+++ b/security/tomoyo/util.c
@@ -1024,6 +1024,8 @@ int tomoyo_init_request_info(struct tomoyo_request_info *r,
memset(r, 0, sizeof(*r));
if (!domain)
domain = tomoyo_domain();
+ if (!domain)
+ return TOMOYO_CONFIG_DISABLED;
r->domain = domain;
profile = domain->profile;
r->profile = profile;
--
2.34.1
Found by fuzzing. Here is the report:
Unable to handle kernel paging request at virtual address dfffffff00000003
Current syz-executor pgtable: 4K pagesize, 57-bit VAs, pgdp=0x000000012edec000
[dfffffff00000003] pgd=000000005fffe401, p4d=000000005fffe001, pud=0000000000000000
Oops [#1]
Modules linked in:
CPU: 0 UID: 0 PID: 3126 Comm: syz-executor Tainted: G W 7.1.0-rc1-gdb909bd7986c #1 PREEMPT
Tainted: [W]=WARN
Hardware name: riscv-virtio,qemu (DT)
epc : tomoyo_check_acl+0x90/0x4bc security/tomoyo/domain.c:173
ra : tomoyo_check_acl+0x86/0x4bc security/tomoyo/domain.c:173
epc : ffffffff8149cf64 ra : ffffffff8149cf5a sp : ff200000040c7a90
gp : ffffffff8a395420 tp : ff60000089d05040 t0 : ff200000040c7960
t1 : 000000000000000f t2 : ffffffff86c068b0 s0 : ff200000040c7b10
s1 : 0000000000000000 a0 : 0000000000000018 a1 : 0000000000000000
a2 : 0000000000000002 a3 : ffffffff8149cf5a a4 : 0000000000000000
a5 : dfffffff00000003 a6 : 0000000000000003 a7 : 000000003dfe34af
s2 : dfffffff00000000 s3 : ff200000040c7b80 s4 : ff600000872a1510
s5 : ffe3ffff00818f79 s6 : 0000000000000000 s7 : ffffffff814a2e18
s8 : ff600000872a1500 s9 : ff200000040c7bc8 s10: 0000000000000002
s11: 0000000000000000 t3 : 6a92f41f00000000 t4 : 0000000000001fff
t5 : 00000000000000c8 t6 : 0000000000000002 ssp : 0000000000000000
status: 0000000200000120 badaddr: dfffffff00000003 cause: 000000000000000d
[<ffffffff8149cf64>] tomoyo_check_acl+0x90/0x4bc security/tomoyo/domain.c:173
[<ffffffff814a4108>] tomoyo_path_number_perm+0x384/0x5a4 security/tomoyo/file.c:738
[<ffffffff814b0cc4>] tomoyo_file_ioctl+0x28/0x34 security/tomoyo/tomoyo.c:350
[<ffffffff81454e8c>] security_file_ioctl+0xaa/0x2c2 security/security.c:2512
[<ffffffff80d45c5e>] __do_sys_ioctl fs/ioctl.c:591 [inline]
[<ffffffff80d45c5e>] __se_sys_ioctl fs/ioctl.c:583 [inline]
[<ffffffff80d45c5e>] __riscv_sys_ioctl+0xae/0x1e4 fs/ioctl.c:583
[<ffffffff80078fb2>] syscall_handler+0x94/0x118 arch/riscv/include/asm/syscall.h:112
[<ffffffff866fa9ea>] do_trap_ecall_u+0x43e/0x5de arch/riscv/kernel/traps.c:342
[<ffffffff867267f6>] handle_exception+0x15e/0x16a arch/riscv/kernel/entry.S:232
Code: 2544 1097 ff0a 80e7 e9c0 8513 0184 5793 0035 97ca (8703) 0007
---[ end trace 0000000000000000 ]---
----------------
Code disassembly (best guess):
0: 2544 fld fs1,136(a0)
2: ff0a1097 auipc ra,0xff0a1
6: e9c080e7 jalr -356(ra) # 0xffffffffff0a0e9e
a: 01848513 addi a0,s1,24
e: 00355793 srli a5,a0,0x3
12: 97ca add a5,a5,s2
* 14: 00078703 lb a4,0(a5) <-- trapping instruction
<<<<<<<<<<<<<<< tail report >>>>>>>>>>>>>>>
Thank you for a patch, but I don't think we need this change. TOMOYO's initial domain is &tomoyo_kernel_domain, and each thread belongs to a non-NULL domain. Therefore, tomoyo_domain() is not supposed to return NULL. > Found by fuzzing. Here is the report: > > Unable to handle kernel paging request at virtual address dfffffff00000003 Is this a NULL pointer dereference? It seems to me that this is just a random memory corruption.
> Thank you for a patch, but I don't think we need this change.
Thanks for your review! I understand your perspective, but I believe
the crash is a real NULL pointer dereference, and I'd like to explain
why the defensive check is warranted.
> TOMOYO's initial domain is &tomoyo_kernel_domain, and each thread belongs to
> a non-NULL domain. Therefore, tomoyo_domain() is not supposed to return NULL.
While tomoyo_domain() is not supposed to return NULL under normal
operation, there are code paths that leave s->domain_info == NULL:
a) Pre-init window (security/tomoyo/tomoyo.c, lines 598-612):
The task security blob is zero-allocated via kzalloc(), and
security_add_hooks() at line 603 is called BEFORE
s->domain_info = &tomoyo_kernel_domain at line 606. If any LSM
hook fires during that window, tomoyo_domain() returns NULL.
b) tomoyo_task_free() (tomoyo.c, lines 533-545) explicitly sets
s->domain_info = NULL after decrementing the refcount.
c) tomoyo_find_next_domain() (domain.c, lines 876-883) writes
s->domain_info = NULL when the domain transition fails.
> > Found by fuzzing. Here is the report:
> >
> > Unable to handle kernel paging request at virtual address dfffffff00000003
>
> Is this a NULL pointer dereference?
> It seems to me that this is just a random memory corruption.
This address is the KASAN shadow byte for memory access at offset 0x18
(24), not a random corrupted value. On RISC-V with sv57 page table,
KASAN_SHADOW_BASE is `0xdfffffff00000000`, and the shadow address is
computed as:
shadow_addr = (access_addr >> 3) + KASAN_SHADOW_BASE
= (24 >> 3) + 0xdfffffff00000000
= 0xdfffffff00000003
In `struct tomoyo_domain_info` (security/tomoyo/common.h, lines
680-693), the layout is:
offset 0: struct list_head list; // 16 bytes
offset 16: struct list_head acl_info_list; // 16 bytes (next at 16, prev at 24)
offset 32: domainname; // 8 bytes
...
Offset 24 from NULL is `domain->acl_info_list.prev`, which is
dereferenced by the `list_for_each_entry_rcu()` loop in
`tomoyo_check_acl()` at security/tomoyo/domain.c:171 when `domain` is
NULL. This is KASAN catching a NULL pointer dereference in action, not
random memory corruption.
I think adding a NULL check makes the code more robust. What do you
think?
Best regards,
Jiakai
On 2026/05/26 22:58, Jiakai Xu wrote: >> Thank you for a patch, but I don't think we need this change. > > Thanks for your review! I understand your perspective, but I believe > the crash is a real NULL pointer dereference, and I'd like to explain > why the defensive check is warranted. > >> TOMOYO's initial domain is &tomoyo_kernel_domain, and each thread belongs to >> a non-NULL domain. Therefore, tomoyo_domain() is not supposed to return NULL. > > While tomoyo_domain() is not supposed to return NULL under normal > operation, there are code paths that leave s->domain_info == NULL: > > a) Pre-init window (security/tomoyo/tomoyo.c, lines 598-612): > The task security blob is zero-allocated via kzalloc(), and > security_add_hooks() at line 603 is called BEFORE > s->domain_info = &tomoyo_kernel_domain at line 606. If any LSM > hook fires during that window, tomoyo_domain() returns NULL. This code is executed during early boot stage. Other LSM hooks are not supposed to fire. > > b) tomoyo_task_free() (tomoyo.c, lines 533-545) explicitly sets > s->domain_info = NULL after decrementing the refcount. This code is executed when a "struct task_struct" is about to be released. Nobody can find this "struct task_struct". Also, this "struct task_struct" cannot be the current thread. > > c) tomoyo_find_next_domain() (domain.c, lines 876-883) writes > s->domain_info = NULL when the domain transition fails. I couldn't catch, but old_domain is initialized as struct tomoyo_domain_info *old_domain = tomoyo_domain(); which cannot be NULL. domain is guaranteed to be non-NULL because old_domain cannot be NULL. if (!domain) domain = old_domain; Therefore, s->domain_info is guaranteed to be non-NULL because domain cannot be NULL. s->domain_info = domain; If domain were NULL, the kernel should have already crashed at line 884. > > I think adding a NULL check makes the code more robust. What do you > think? Then, this will be NULL pointer dereference. But fixing the location that is setting NULL is the correct approach.
> >> Thank you for a patch, but I don't think we need this change. > > > > Thanks for your review! I understand your perspective, but I believe > > the crash is a real NULL pointer dereference, and I'd like to explain > > why the defensive check is warranted. > > > >> TOMOYO's initial domain is &tomoyo_kernel_domain, and each thread belongs to > >> a non-NULL domain. Therefore, tomoyo_domain() is not supposed to return NULL. > > > > While tomoyo_domain() is not supposed to return NULL under normal > > operation, there are code paths that leave s->domain_info == NULL: > > > > a) Pre-init window (security/tomoyo/tomoyo.c, lines 598-612): > > The task security blob is zero-allocated via kzalloc(), and > > security_add_hooks() at line 603 is called BEFORE > > s->domain_info = &tomoyo_kernel_domain at line 606. If any LSM > > hook fires during that window, tomoyo_domain() returns NULL. > > This code is executed during early boot stage. Other LSM hooks are not > supposed to fire. > > > > > b) tomoyo_task_free() (tomoyo.c, lines 533-545) explicitly sets > > s->domain_info = NULL after decrementing the refcount. > > This code is executed when a "struct task_struct" is about to be released. > Nobody can find this "struct task_struct". Also, this "struct task_struct" > cannot be the current thread. > > > > > c) tomoyo_find_next_domain() (domain.c, lines 876-883) writes > > s->domain_info = NULL when the domain transition fails. > > I couldn't catch, but old_domain is initialized as > > struct tomoyo_domain_info *old_domain = tomoyo_domain(); > > which cannot be NULL. > > domain is guaranteed to be non-NULL because old_domain cannot be NULL. > > if (!domain) > domain = old_domain; > > Therefore, s->domain_info is guaranteed to be non-NULL because domain cannot be NULL. > > s->domain_info = domain; > > If domain were NULL, the kernel should have already crashed at line 884. Thank you for the thorough explanation! You are absolutely right, and I really appreciate you taking the time to walk through each path. > > > > I think adding a NULL check makes the code more robust. What do you > > think? > > Then, this will be NULL pointer dereference. > But fixing the location that is setting NULL is the correct approach. I fully agree. The NULL check I proposed would only mask the symptom. The real bug is that something corrupted the task_struct's security blob and zeroed out domain_info before the ioctl hook fired. Unfortunately, I don't have a reliable reproducer. The fuzzer triggered this only once on riscv, so I can't easily track down the source of the corruption. Either way, thank you again for the review. I learned a lot about TOMOYO's domain lifecycle from your explanation. Best regards, Jiakai
© 2016 - 2026 Red Hat, Inc.