Re: [PATCH v4 bpf-next 2/3] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Marking `bpf_lsm_inode_init_security` as a sleepable hook allows BPF programs to trigger page faults while holding a filesystem transaction handle, leading to deadlocks.
--

commit 78be3e5567e8821124f5c2f9f979f1274f553583
Author: David Windsor <dwindsor@gmail.com>

bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling

This patch adds the bpf_init_inode_xattr kfunc to allow BPF LSM programs
to atomically set xattrs via the inode_init_security hook. The kfunc uses
lsm_get_xattr_slot and restricts usage to the bpf.* namespace.

Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55

> diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> index 768aca2dc0f03..c4023c82f21e4 100644
> --- a/fs/bpf_fs_kfuncs.c
> +++ b/fs/bpf_fs_kfuncs.c
[ ... ]
> @@ -385,6 +463,7 @@ BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE)
>  BTF_ID_FLAGS(func, bpf_set_dentry_xattr, KF_SLEEPABLE)
>  BTF_ID_FLAGS(func, bpf_remove_dentry_xattr, KF_SLEEPABLE)
>  BTF_ID_FLAGS(func, bpf_real_inode, KF_SLEEPABLE | KF_RET_NULL)
> +BTF_ID_FLAGS(func, bpf_init_inode_xattr, KF_SLEEPABLE)

[Severity: High]
Does marking this kfunc as sleepable create a risk of deadlocks?

Since the kfunc is only callable from the inode_init_security hook, marking
the hook as sleepable allows BPF programs to trigger page faults
(e.g., via bpf_copy_from_user). When a file is created (e.g., ext4_create),
the filesystem acquires a transaction handle before invoking the hook. If the
BPF program triggers a page fault, it will attempt to take mmap_read_lock.

Concurrently, another thread doing a write page fault (or mmap/mprotect)
might acquire mmap_lock and attempt to start a transaction (e.g., in
ext4_page_mkwrite), which blocks on the transaction handle held by the first
process. This creates an AB-BA deadlock between the filesystem transaction and
mmap_lock.

>  BTF_KFUNCS_END(bpf_fs_kfunc_set_ids)
>  
>  static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id)
[ ... ]
> diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> index 564071a92d7d2..1c3f84a924203 100644
> --- a/kernel/bpf/bpf_lsm.c
> +++ b/kernel/bpf/bpf_lsm.c
> @@ -315,6 +315,7 @@ BTF_ID(func, bpf_lsm_inode_create)
>  BTF_ID(func, bpf_lsm_inode_free_security)
>  BTF_ID(func, bpf_lsm_inode_getattr)
>  BTF_ID(func, bpf_lsm_inode_getxattr)
> +BTF_ID(func, bpf_lsm_inode_init_security)

[Severity: High]
Is it safe to add bpf_lsm_inode_init_security to the sleepable hooks list?

As mentioned above, marking this hook as sleepable permits BPF programs to
take page faults from within a filesystem transaction, which violates
filesystem locking rules and can freeze filesystem operations.

>  BTF_ID(func, bpf_lsm_inode_mknod)
>  BTF_ID(func, bpf_lsm_inode_need_killpriv)
>  BTF_ID(func, bpf_lsm_inode_post_setxattr)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630183956.281293-1-dwindsor@gmail.com?part=2