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

[Jiakai Xu]

> 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