Re: [PATCH bpf-next 1/2] bpf: fix crash in bpf_path_d_path() for exiting tasks

[Matt Bobrowski <matt@bobrowski.net>]

On Wed, Oct 21, 2026 at 09:58:09PM +1100, Matt Bobrowski wrote:
> On Tue, Oct 20, 2026 at 06:24:42PM +0000, Alexei Starovoitov wrote:
> > On Tue, Oct 20, 2026 at 03:50 AM Matt Bobrowski <matt@bobrowski.net> wrote:
> > > Fix this by returning -EINVAL from bpf_path_d_path() when current->fs
> > > is NULL. Only the task itself clears its fs, in exit_fs(), so the check
> > > cannot race with that.
> > 
> > The crash is gone, but so is the path. file_release progs will get
> > -EINVAL for every file that a task still has open when it exits.
> > 
> > It also breaks what works today. The d_dname branch in d_path() returns
> > before it looks at current->fs, so at task exit a pipe or a socket
> > resolves to "pipe:[123]" now and to -EINVAL with this patch.
> > 
> > And it's not specific to bpf. The comment in fs/cachefiles/namei.c says
> > that current->fs == NULL in do_exit() breaks d_path() called from ext4
> > open.
> > 
> > can we fix it in d_path() instead ?
> > With an empty root prepend_path() goes up to the global root, the same
> > way d_absolute_path() does. Then the check in bpf_path_d_path() is not
> > needed and the selftest can expect the path. A chrooted task gets the
> > path from the global root, but it doesn't have a root anymore.
> > 
> > Something like this (compile tested only):
> 
> Prior to sending out the fix, I also considered addressing this directly
> within d_path(), much like you've proposed here, but decided against
> it. d_path()'s contract is to build a pathname relative to the current
> root. It does fall back to a root-independent name when the path isn't
> reachable from that root, but that fallback still presupposes the caller
> has a root to measure against. An exiting task past exit_fs() has no
> root, so d_path() has no correct answer to give in that state, and I'm
> not convinced we should invent one for it.  That's what
> d_absolute_path() is for.
> 
> You're right that my patch regresses the d_dname case though, since
> pipes and sockets resolve today without ever looking at current->fs.
> Rather than replicating the d_dname check in bpf_path_d_path(), which
> would mean duplicating VFS logic such as the mounted pseudo-inode
> special case, I think the cleaner fix is to move the "no root, no
> answer" check into d_path() itself, after the d_dname branch:
> 
> 	if (unlikely(!current->fs))
> 		return ERR_PTR(-ENOENT);
> 
> That keeps d_path()'s semantics intact, keeps pipe:[N] and socket:[N]
> working at exit, and turns the oops into an error for every caller,
> including the ext4 open path you mentioned. Nothing that works today
> changes behaviour; only calls that currently oops are affected.  Callers
> that want a name in that state then have to choose one explicitly, which
> I take to be Al's point.
> 
> For BPF, that choice could be a new BPF kfunc wrapping
> d_absolute_path(), so programs can pick whichever semantics they actually
> want, and file_release programs that need a name at exit get one whose
> format doesn't depend on how far the task got through do_exit(). I'd
> prefer that over silently falling back to d_absolute_path() inside
> bpf_path_d_path(), which would return chroot-relative names before
> exit_fs() and absolute ones after, and still wouldn't help pipes and
> sockets since d_absolute_path() rejects internal mounts.
> 
> Al, would a check like the above in d_path() be acceptable to you?

Al, what do you think about what I've proposed above?

> > diff --git a/fs/d_path.c b/fs/d_path.c
> > index c253090..ea49d33 100644
> > --- a/fs/d_path.c
> > +++ b/fs/d_path.c
> > @@ -265,7 +265,7 @@ static void get_fs_root_rcu(struct fs_struct *fs, struct path *root)
> >  char *d_path(const struct path *path, char *buf, int buflen)
> >  {
> >  	DECLARE_BUFFER(b, buf, buflen);
> > -	struct path root;
> > +	struct path root = {};
> > 
> >  	/*
> >  	 * We have various synthetic filesystems that never get mounted.  On
> > @@ -284,7 +284,9 @@ char *d_path(const struct path *path, char *buf, int buflen)
> >  		return path->dentry->d_op->d_dname(path->dentry, buf, buflen);
> > 
> >  	rcu_read_lock();
> > -	get_fs_root_rcu(current->fs, &root);
> > +	/* Past exit_fs() there is no root to stop at, go up to the global one. */
> > +	if (current->fs)
> > +		get_fs_root_rcu(current->fs, &root);
> >  	if (unlikely(d_unlinked(path->dentry)))
> >  		prepend(&b, " (deleted)", 11);
> >  	else
> > 
> > pw-bot: cr