Re: [PATCH bpf-next v3 1/2] 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] bpf_init_inode_xattr incorrectly uses GFP_KERNEL when allocating xattr_value inside the inode_init_security hook, which can trigger filesystem reclaim deadlocks.
--

commit 90409fcd43737306bcca3704874f44073960a65f
Author: David Windsor <dwindsor@gmail.com>

bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling

This commit adds the bpf_init_inode_xattr() kfunc for BPF LSM programs to
atomically set xattrs via the inode_init_security hook. It updates the hook
interface to use a single xattr_ctx context object rather than separate
arrays and counts.

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

> diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> index 768aca2dc0f03..7abc3f3d1a678 100644
> --- a/fs/bpf_fs_kfuncs.c
> +++ b/fs/bpf_fs_kfuncs.c
[ ... ]
> @@ -374,6 +375,97 @@ __bpf_kfunc struct inode *bpf_real_inode(struct dentry *dentry)
[ ... ]
> +static int __bpf_init_inode_xattr(struct xattr_ctx *xattr_ctx,
> +				  const char *name__str,
> +				  const struct bpf_dynptr *value_p)
> +{
[ ... ]
> +	value = __bpf_dynptr_data(value_ptr, value_len);
> +	if (!value)
> +		return -EINVAL;
> +
> +	/* Combine xattr value + name into one allocation. */
> +	xattr_value = kmalloc(value_len + name_len + 1, GFP_KERNEL);

[Severity: High]
Can this cause a filesystem transaction deadlock?

The inode_init_security hook is called during inode creation, which often
happens while filesystems like ext4 or btrfs hold critical filesystem locks
or an active journal transaction. 

If kmalloc is called with GFP_KERNEL here, the memory allocator can enter
direct reclaim. The reclaim path may re-enter the filesystem to write back
dirty pages or shrink caches, attempting to acquire the same locks that are
already held by the active file creation path.

Other LSMs and the core security framework explicitly use GFP_NOFS during
the inode_init_security hook to avoid these reclaim deadlocks. Should this
allocation use GFP_NOFS instead?

> +	if (!xattr_value)
> +		return -ENOMEM;
> +
> +	memcpy(xattr_value, value, value_len);

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