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

[Matt Bobrowski <mattbobrowski@google.com>]

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