[PATCH] tomoyo: Fix NULL pointer dereference in tomoyo_init_request_info() when domain is NULL

Jiakai Xu posted 1 patch 1 week, 6 days ago
security/tomoyo/util.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] tomoyo: Fix NULL pointer dereference in tomoyo_init_request_info() when domain is NULL
Posted by Jiakai Xu 1 week, 6 days ago
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 >>>>>>>>>>>>>>>
Re: [PATCH] tomoyo: Fix NULL pointer dereference in tomoyo_init_request_info() when domain is NULL
Posted by Tetsuo Handa 1 week, 6 days ago
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.
Re: [PATCH] tomoyo: Fix NULL pointer dereference in tomoyo_init_request_info() when domain is NULL
Posted by Jiakai Xu 1 week, 6 days ago
> 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
Re: [PATCH] tomoyo: Fix NULL pointer dereference in tomoyo_init_request_info() when domain is NULL
Posted by Tetsuo Handa 1 week, 6 days ago
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.
Re: [PATCH] tomoyo: Fix NULL pointer dereference in tomoyo_init_request_info() when domain is NULL
Posted by Jiakai Xu 1 week, 5 days ago
> >> 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