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

[Matt Bobrowski <matt@bobrowski.net>]

On Thu, Nov 05, 2026 at 04:44:34PM +0100, Christian Brauner wrote:
> On Tue, Oct 27, 2026 at 08:55:57AM +1100, Matt Bobrowski wrote:
> > 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 seems ok albeit fugly.

Yeah, not super nice, I agree...

Anyway, I'll respin this series with what I've proposed above.