Re: [PATCH v3 -next 09/18] LSM: rename inode_getattr to path_getattr and pass struct file

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] bpf, lsm: missing __nullable on file arg in bpf_lsm_path_getattr()
- [Low] tomoyo: missing @file kernel-doc in tomoyo_path_getattr()
--

commit b5ecf79579b44f0d69e46ecb8c8c91e302299f23
Author: Cai Xinchen <caixinchen1@huawei.com>

LSM: rename inode_getattr to path_getattr and pass struct file

This commit renames the inode_getattr LSM hook and security_inode_getattr()
wrapper to path_getattr and security_path_getattr(), and adds a struct file
parameter passed by file descriptor-based stat operations while path-based
callers pass NULL. It also updates LSM implementations, BPF LSM/tracing BTF
IDs, and BPF selftests accordingly.

[ ... ]

> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 96ae1b548e8d4..0f7712f516444 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -153,7 +153,8 @@ LSM_HOOK(int, 0, inode_setattr, const struct mnt_idmap *idmap,
>  	 const struct path *path, struct iattr *attr, struct file *file)
>  LSM_HOOK(void, LSM_RET_VOID, inode_post_setattr, const struct mnt_idmap *idmap,
>  	 struct dentry *dentry, int ia_valid)
> -LSM_HOOK(int, 0, inode_getattr, const struct path *path)
> +LSM_HOOK(int, 0, path_getattr, const struct path *path,
> +	 struct file *file)

[Severity: High]
Can this trigger a NULL pointer dereference when a BPF LSM program attaches
to lsm/path_getattr or lsm.s/path_getattr?

In fs/stat.c, both vfs_getattr() and vfs_statx() -> vfs_statx_path() pass
NULL for the file argument when calling security_path_getattr():

    retval = security_path_getattr(path, NULL);

and:

    error = vfs_statx_path(&path, flags, stat, request_mask, NULL);

In kernel/bpf/bpf_lsm.c, LSM_HOOK() generates the weak hook stub
bpf_lsm_path_getattr(const struct path *path, struct file *file). Because
bpf_lsm_path_getattr is not in untrusted_lsm_hooks, btf_ctx_access() in
kernel/bpf/btf.c marks its pointer arguments as PTR_TO_BTF_ID | PTR_TRUSTED
without PTR_MAYBE_NULL unless the parameter name has a __nullable suffix:

    info->reg_type = PTR_TO_BTF_ID;
    if (prog_args_trusted(prog))
        info->reg_type |= PTR_TRUSTED;

    if (btf_param_match_suffix(btf, &args[arg], "__nullable"))
        info->reg_type |= PTR_MAYBE_NULL;

Without PTR_MAYBE_NULL on the file argument:

1. bpf_may_fault_on_deref(PTR_TO_BTF_ID | PTR_TRUSTED) returns false in
   include/linux/bpf_verifier.h, so direct loads from file (such as
   file->f_inode) are permitted without a NULL check and are not rewritten
   to BPF_PROBE_MEM by convert_ctx_accesses().

2. Because bpf_lsm_path_getattr is in sleepable_lsm_hooks, a sleepable
   lsm.s/path_getattr program can pass file without a NULL check to kfuncs
   expecting a non-NULL trusted struct file * (such as bpf_get_file_xattr()
   in fs/bpf_fs_kfuncs.c), which immediately dereference file.

Should bpf_lsm_path_getattr() have a strong override in
kernel/bpf/bpf_lsm_proto.c with struct file *file__nullable, similar to
bpf_lsm_mmap_file()?

[ ... ]

> diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
> index c66e02ed8ee3b..c67455e060f74 100644
> --- a/security/tomoyo/tomoyo.c
> +++ b/security/tomoyo/tomoyo.c
> @@ -112,13 +112,14 @@ static int tomoyo_bprm_check_security(struct linux_binprm *bprm)
>  }
>  
>  /**
> - * tomoyo_inode_getattr - Target for security_inode_getattr().
> + * tomoyo_path_getattr - Target for security_path_getattr().
>   *
>   * @path: Pointer to "struct path".
>   *
>   * Returns 0 on success, negative value otherwise.
>   */
> -static int tomoyo_inode_getattr(const struct path *path)
> +static int tomoyo_path_getattr(const struct path *path,
> +			       struct file *file)

[Severity: Low]
Does the kernel-doc comment for tomoyo_path_getattr() also need a @file
entry to avoid a kernel-doc warning under W=1 builds?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027121151.1887650-1-caixinchen1@huawei.com?part=9