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

[PATCH bpf-next] 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.

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 | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
index 9d27be058494..a0d58d5e3022 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);
 	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] bpf: fix crash in bpf_[set|remove]_dentry_xattr for negative dentries

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Incomplete Fix: Negative dentries still trigger WARN_ON splats in bpf_get_dentry_xattr and _locked variants
- [High] BPF verifier blindly substitutes locked kfuncs, allowing arbitrary VFS lock bypass
- [High] Missing VFS parameter sanitization allows out-of-bounds xattr sizes and arbitrary flags
--

commit 07ec989f2fe3141fcb87e29833232a3f0591690d
Author: Matt Bobrowski <mattbobrowski@google.com>

bpf: fix crash in bpf_[set|remove]_dentry_xattr for negative dentries

This commit adds a NULL check for the inode in bpf_set_dentry_xattr() and
bpf_remove_dentry_xattr() to prevent a NULL pointer dereference when a
negative dentry is passed. If the inode is NULL, the functions now return
-EINVAL instead of attempting to lock it.

Closes: https://lore.kernel.org/bpf/1587cbf4-1293-4e25-ad24-c970836a1686@std.uestc.edu.cn/

> diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> index 9d27be058494..a0d58d5e3022 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;

Are similar checks needed for bpf_get_dentry_xattr() and the _locked variants
(bpf_set_dentry_xattr_locked() and bpf_remove_dentry_xattr_locked())?

If a negative dentry is passed to those functions, they will extract a NULL
inode and pass it to bpf_xattr_write_permission() or
bpf_xattr_read_permission(), which will trigger a WARN_ON(!inode) splat:

fs/bpf_fs_kfuncs.c:bpf_set_dentry_xattr_locked() {
	...
	struct inode *inode = d_inode(dentry);
	...
	ret = bpf_xattr_write_permission(name__str, inode);
	...
}

Could a user-controlled BPF program intentionally pass a negative dentry to
trigger this warning, causing a denial of service on systems with
panic_on_warn enabled?

> +
>  	inode_lock(inode);
>  	ret = bpf_set_dentry_xattr_locked(dentry, name__str, value_p, flags);

This isn't a regression from this patch, but does the BPF verifier safely
handle the substitution of these locked kfuncs?

If a program is attached to an LSM hook that holds an inode lock, the
verifier unconditionally rewrites calls to bpf_set_dentry_xattr() to
bpf_set_dentry_xattr_locked(). The verifier doesn't check if the dentry
argument provided to the kfunc is the specific context argument that the
VFS already locked.

Can 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?

Additionally, do these kfuncs bypass necessary VFS parameter sanitization?

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?

>  	inode_unlock(inode);
>  	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260429205438.2601592-1-mattbobrowski@google.com?part=1

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

[Matt Bobrowski]

On Wed, Apr 29, 2026 at 10:10:00PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] Incomplete Fix: Negative dentries still trigger WARN_ON splats in bpf_get_dentry_xattr and _locked variants
> - [High] BPF verifier blindly substitutes locked kfuncs, allowing arbitrary VFS lock bypass
> - [High] Missing VFS parameter sanitization allows out-of-bounds xattr sizes and arbitrary flags
> --
> 
> commit 07ec989f2fe3141fcb87e29833232a3f0591690d
> Author: Matt Bobrowski <mattbobrowski@google.com>
> 
> bpf: fix crash in bpf_[set|remove]_dentry_xattr for negative dentries
> 
> This commit adds a NULL check for the inode in bpf_set_dentry_xattr() and
> bpf_remove_dentry_xattr() to prevent a NULL pointer dereference when a
> negative dentry is passed. If the inode is NULL, the functions now return
> -EINVAL instead of attempting to lock it.
> 
> Closes: https://lore.kernel.org/bpf/1587cbf4-1293-4e25-ad24-c970836a1686@std.uestc.edu.cn/
> 
> > diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> > index 9d27be058494..a0d58d5e3022 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;
> 
> Are similar checks needed for bpf_get_dentry_xattr() and the _locked variants
> (bpf_set_dentry_xattr_locked() and bpf_remove_dentry_xattr_locked())?
> 
> If a negative dentry is passed to those functions, they will extract a NULL
> inode and pass it to bpf_xattr_write_permission() or
> bpf_xattr_read_permission(), which will trigger a WARN_ON(!inode) splat:
> 
> fs/bpf_fs_kfuncs.c:bpf_set_dentry_xattr_locked() {
> 	...
> 	struct inode *inode = d_inode(dentry);
> 	...
> 	ret = bpf_xattr_write_permission(name__str, inode);
> 	...
> }
> 
> Could a user-controlled BPF program intentionally pass a negative dentry to
> trigger this warning, causing a denial of service on systems with
> panic_on_warn enabled?

Yes, dropping the WARN_ON(!inode) makes sense to me. By definition,
attempting to set an extended attribute on a negative dentry is
considered as an invalid operation, so it should be handled
gracefully.

> > +
> >  	inode_lock(inode);
> >  	ret = bpf_set_dentry_xattr_locked(dentry, name__str, value_p, flags);
> 
> This isn't a regression from this patch, but does the BPF verifier safely
> handle the substitution of these locked kfuncs?
> 
> If a program is attached to an LSM hook that holds an inode lock, the
> verifier unconditionally rewrites calls to bpf_set_dentry_xattr() to
> bpf_set_dentry_xattr_locked(). The verifier doesn't check if the dentry
> argument provided to the kfunc is the specific context argument that the
> VFS already locked.
> 
> Can 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?

Probably a valid concern and the current approach does seem a little
fragile. This too should be reviewed in further depth separately
though.

> Additionally, do these kfuncs bypass necessary VFS parameter sanitization?
> 
> 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?

I believe this is a valid concern and should be addressed separately
in a follow up patch. Thank you for bringing this to our attention.

> >  	inode_unlock(inode);
> >  	return ret;
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260429205438.2601592-1-mattbobrowski@google.com?part=1