[PATCH bpf-next v2] bpf: fix crash in bpf_[set|remove]_dentry_xattr for negative dentries

[PATCH bpf-next v2] bpf: fix crash in bpf_[set|remove]_dentry_xattr for negative dentries

[Matt Bobrowski]

bpf_set_dentry_xattr and bpf_remove_dentry_xattr BPF kfuncs attempt to
lock the inode of the supplied dentry without checking if it is
NULL. If a negative dentry is passed (e.g. from
security_inode_create), d_inode(dentry) returns NULL, and
inode_lock(inode) will cause a NULL pointer dereference.

Trivially fix this by adding a NULL check for inode before attempting
to lock it, returning -EINVAL if it is NULL.

Additionally, drop WARN_ON(!inode) in bpf_xattr_read_permission() and
bpf_xattr_write_permission(). These warnings could be triggered by
passing a negative dentry to bpf_get_dentry_xattr() or the _locked
variants of the xattr kfuncs, potentially causing a Denial of Service
on systems with panic_on_warn enabled. Instead, simply return -EINVAL.

Reported-by: Quan Sun <2022090917019@std.uestc.edu.cn>
Closes: https://lore.kernel.org/bpf/1587cbf4-1293-4e25-ad24-c970836a1686@std.uestc.edu.cn/
Fixes: 56467292794b ("bpf: fs/xattr: Add BPF kfuncs to set and remove xattrs")
Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
---
 fs/bpf_fs_kfuncs.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
index 9d27be058494..8642a754b37d 100644
--- a/fs/bpf_fs_kfuncs.c
+++ b/fs/bpf_fs_kfuncs.c
@@ -100,7 +100,7 @@ static bool match_security_bpf_prefix(const char *name__str)
 
 static int bpf_xattr_read_permission(const char *name, struct inode *inode)
 {
-	if (WARN_ON(!inode))
+	if (!inode)
 		return -EINVAL;
 
 	/* Allow reading xattr with user. and security.bpf. prefix */
@@ -170,7 +170,7 @@ __bpf_kfunc_end_defs();
 
 static int bpf_xattr_write_permission(const char *name, struct inode *inode)
 {
-	if (WARN_ON(!inode))
+	if (!inode)
 		return -EINVAL;
 
 	/* Only allow setting and removing security.bpf. xattrs */
@@ -289,6 +289,9 @@ __bpf_kfunc int bpf_set_dentry_xattr(struct dentry *dentry, const char *name__st
 	struct inode *inode = d_inode(dentry);
 	int ret;
 
+	if (!inode)
+		return -EINVAL;
+
 	inode_lock(inode);
 	ret = bpf_set_dentry_xattr_locked(dentry, name__str, value_p, flags);
 	inode_unlock(inode);
@@ -314,6 +317,9 @@ __bpf_kfunc int bpf_remove_dentry_xattr(struct dentry *dentry, const char *name_
 	struct inode *inode = d_inode(dentry);
 	int ret;
 
+	if (!inode)
+		return -EINVAL;
+
 	inode_lock(inode);
 	ret = bpf_remove_dentry_xattr_locked(dentry, name__str);
 	inode_unlock(inode);
-- 
2.54.0.545.g6539524ca2-goog

Re: [PATCH bpf-next v2] bpf: fix crash in bpf_[set|remove]_dentry_xattr for negative dentries

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2339 bytes --]

> diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> index 9d27be058494..8642a754b37d 100644
> --- a/fs/bpf_fs_kfuncs.c
> +++ b/fs/bpf_fs_kfuncs.c

[ ... ]

> @@ -289,6 +289,9 @@ __bpf_kfunc int bpf_set_dentry_xattr(struct dentry *dentry, const char *name__st
>  	struct inode *inode = d_inode(dentry);
>  	int ret;
>
> +	if (!inode)
> +		return -EINVAL;
> +
>  	inode_lock(inode);
>  	ret = bpf_set_dentry_xattr_locked(dentry, name__str, value_p, flags);

An earlier review comment from sashiko-bot raised concerns about the
BPF verifier's unconditional rewriting of bpf_set_dentry_xattr() calls
to bpf_set_dentry_xattr_locked() in LSM hooks that already hold inode
locks.

The concern is that the verifier doesn't check whether the dentry
argument provided to the kfunc is the specific context argument that
the VFS already locked. Could a BPF program obtain a trusted pointer to
an unrelated, unlocked dentry and pass it to the kfunc, causing the
substituted _locked variant to execute __vfs_setxattr() without
acquiring the proper VFS locks?

This concern was acknowledged as valid and worth reviewing separately,
but remains unaddressed in v2.

Reference: https://lore.kernel.org/bpf/20260429221005.6D1C6C19425@smtp.kernel.org/

[ ... ]

In bpf_set_dentry_xattr_locked():

>  	ret = __vfs_setxattr(&nop_mnt_idmap, dentry, inode, name__str,
>  			     value, value_len, flags);
>  	if (!ret) {

An earlier review comment from sashiko-bot pointed out that the
standard setxattr system call sanitizes inputs in setxattr_copy(),
rejecting sizes larger than XATTR_SIZE_MAX and masking flags.

Since bpf_set_dentry_xattr() and its _locked variant directly invoke
__vfs_setxattr(), a BPF program can supply arbitrary unvalidated flags
and massive values up to 4GB.

Could this bypass lead to huge allocations, integer overflows, or
out-of-bounds accesses in drivers that assume VFS-enforced bounds?

This concern was acknowledged as valid and to be addressed in a
follow-up patch, but remains unaddressed in v2.

Reference: https://lore.kernel.org/bpf/20260429221005.6D1C6C19425@smtp.kernel.org/


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25153819489

Re: [PATCH bpf-next v2] bpf: fix crash in bpf_[set|remove]_dentry_xattr for negative dentries

[Matt Bobrowski]

On Thu, Apr 30, 2026 at 08:14:07AM +0000, bot+bpf-ci@kernel.org wrote:
> > diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> > index 9d27be058494..8642a754b37d 100644
> > --- a/fs/bpf_fs_kfuncs.c
> > +++ b/fs/bpf_fs_kfuncs.c
> 
> [ ... ]
> 
> > @@ -289,6 +289,9 @@ __bpf_kfunc int bpf_set_dentry_xattr(struct dentry *dentry, const char *name__st
> >  	struct inode *inode = d_inode(dentry);
> >  	int ret;
> >
> > +	if (!inode)
> > +		return -EINVAL;
> > +
> >  	inode_lock(inode);
> >  	ret = bpf_set_dentry_xattr_locked(dentry, name__str, value_p, flags);
> 
> An earlier review comment from sashiko-bot raised concerns about the
> BPF verifier's unconditional rewriting of bpf_set_dentry_xattr() calls
> to bpf_set_dentry_xattr_locked() in LSM hooks that already hold inode
> locks.
> 
> The concern is that the verifier doesn't check whether the dentry
> argument provided to the kfunc is the specific context argument that
> the VFS already locked. Could a BPF program obtain a trusted pointer to
> an unrelated, unlocked dentry and pass it to the kfunc, causing the
> substituted _locked variant to execute __vfs_setxattr() without
> acquiring the proper VFS locks?
> 
> This concern was acknowledged as valid and worth reviewing separately,
> but remains unaddressed in v2.
> 
> Reference: https://lore.kernel.org/bpf/20260429221005.6D1C6C19425@smtp.kernel.org/
> 
> [ ... ]
> 
> In bpf_set_dentry_xattr_locked():
> 
> >  	ret = __vfs_setxattr(&nop_mnt_idmap, dentry, inode, name__str,
> >  			     value, value_len, flags);
> >  	if (!ret) {
> 
> An earlier review comment from sashiko-bot pointed out that the
> standard setxattr system call sanitizes inputs in setxattr_copy(),
> rejecting sizes larger than XATTR_SIZE_MAX and masking flags.
> 
> Since bpf_set_dentry_xattr() and its _locked variant directly invoke
> __vfs_setxattr(), a BPF program can supply arbitrary unvalidated flags
> and massive values up to 4GB.
> 
> Could this bypass lead to huge allocations, integer overflows, or
> out-of-bounds accesses in drivers that assume VFS-enforced bounds?
> 
> This concern was acknowledged as valid and to be addressed in a
> follow-up patch, but remains unaddressed in v2.
> 
> Reference: https://lore.kernel.org/bpf/20260429221005.6D1C6C19425@smtp.kernel.org/

Yeah, I think this is a legitimate concern and we should be abiding
whatever the VFS enforces with regards to xattr name/value length
related limitations. I've addressed this within a separate follow up
patch series here:

* https://lore.kernel.org/bpf/20260503200819.1530328-1-mattbobrowski@google.com/T/#t

> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25153819489

Re: [PATCH bpf-next v2] bpf: fix crash in bpf_[set|remove]_dentry_xattr for negative dentries

[Christian Brauner]

On Thu, 30 Apr 2026 07:38:36 +0000, Matt Bobrowski wrote:
> bpf_set_dentry_xattr and bpf_remove_dentry_xattr BPF kfuncs attempt to
> lock the inode of the supplied dentry without checking if it is
> NULL. If a negative dentry is passed (e.g. from
> security_inode_create), d_inode(dentry) returns NULL, and
> inode_lock(inode) will cause a NULL pointer dereference.
> 
> Trivially fix this by adding a NULL check for inode before attempting
> to lock it, returning -EINVAL if it is NULL.
> 
> [...]

Applied to the vfs-7.2.kfunc branch of the vfs/vfs.git tree.
Patches in the vfs-7.2.kfunc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-7.2.kfunc

[1/1] bpf: fix crash in bpf_[set|remove]_dentry_xattr for negative dentries
      https://git.kernel.org/vfs/vfs/c/07410646f6ff