Re: [PATCH bpf-next v1 05/11] bpf: add a path ancestor iterator

[Justin Suess <utilityemal77@gmail.com>]

On Tue, Oct 13, 2026 at 05:11:54PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] bpf: deadlock in bpf_iter_path_ancestors_next due to GFP_KERNEL under VFS locks
> --
> 
> commit 0d24b2f0561971ff2a9d89456692d9ec1ea13279
> Author: Justin Suess <utilityemal77@gmail.com>
> 
> bpf: add a path ancestor iterator
> 
> This commit introduces a sleepable BPF iterator for evaluating a path's
> ancestry using the stepwise vfs_walk_ancestors() engine. It adds kfuncs
> to manage the iterator and hands each position to the program as an
> acquired reference by allocating a copy of the path structure.
> 
> > diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> > index 08f0847c49708..02398f5a46d5a 100644
> > --- a/fs/bpf_fs_kfuncs.c
> > +++ b/fs/bpf_fs_kfuncs.c
> [ ... ]
> > +__bpf_kfunc struct path *
> > +bpf_iter_path_ancestors_next(struct bpf_iter_path_ancestors *it)
> > +{
> > +	struct bpf_path_ancestors_kern *kit = (void *)it;
> > +	struct path *pos = bpf_path_ancestors_step(kit);
> > +	struct path *held;
> > +
> > +	if (!pos)
> > +		return NULL;
> > +	/*
> > +	 * The position must outlive the walk's own view of it, so it gets a
> > +	 * reference and a struct path of its own to live in: struct path is
> > +	 * a value type, with nothing a BPF reference could be taken on
> > +	 * otherwise.  Sleepable, so no atomic allocation.
> > +	 */
> > +	held = kmalloc_obj(*held);
> 
> [Severity: High]
> Can this allocation cause a deadlock if the iterator is used from a sleepable
> LSM hook?
> 
> If a sleepable BPF LSM program is attached to a hook like
> security_inode_setattr or security_inode_unlink, it is called by the VFS
> with the inode's i_rwsem held. 
> 
> The kmalloc_obj() macro appears to default to GFP_KERNEL allocations, which
> includes the __GFP_FS flag. Under memory pressure, this could trigger direct
> memory reclaim.
> 
> If reclaim attempts to write back dirty pages or shrink the dcache/icache,
> it might try to acquire the same i_rwsem, resulting in a deadlock.
>
Direct reclaim never takes i_rwsem. So this is incorrect.

(Also if that were the case, the mkdir selftest would splat with lockdep)

Justin
> > +	if (!held) {
> > +		kit->step = -ENOMEM;
> > +		return NULL;
> > +	}
> > +	*held = *pos;
> > +	path_get(held);
> > +	return held;
> > +}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261013164800.2906577-1-utilityemal77@gmail.com?part=5