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

[Matt Bobrowski <matt@bobrowski.net>]

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?

> 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