linux-kernel - Re: [PATCH v4 bpf-next 1/5] namei: Introduce new helper function path_walk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPhsuW7V9MWXBqiEFbFipUVASwysbB1pX3Lz0NCncFJ9Gjpo5w@mail.gmail.com>
Date: Fri, 13 Jun 2025 15:27:33 -0700
From: Song Liu <song@...nel.org>
To: NeilBrown <neil@...wn.name>
Cc: bpf@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org, 
	kernel-team@...a.com, andrii@...nel.org, eddyz87@...il.com, ast@...nel.org, 
	daniel@...earbox.net, martin.lau@...ux.dev, viro@...iv.linux.org.uk, 
	brauner@...nel.org, jack@...e.cz, kpsingh@...nel.org, 
	mattbobrowski@...gle.com, amir73il@...il.com, repnop@...gle.com, 
	jlayton@...nel.org, josef@...icpanda.com, mic@...ikod.net, gnoack@...gle.com, 
	m@...wtm.org
Subject: Re: [PATCH v4 bpf-next 1/5] namei: Introduce new helper function path_walk_parent()

On Thu, Jun 12, 2025 at 5:11 PM NeilBrown <neil@...wn.name> wrote:
[...]
> > +
> > +false_out:
> > +     path_put(path);
> > +     memset(path, 0, sizeof(*path));
> > +     return false;
> > +}
>
> I think the public function should return 0 on success and -error on
> failure.  That is a well established pattern.

Yeah, I think we can use this pattern.

> I also think you
> shouldn't assume that all callers will want the same flags.

__path_walk_parent() only handles two LOOKUP_ flags, so
it is a bit weird to allow all the flags. But if folks think this is a
good idea, I don't have strong objections to taking various flags.

>
> And it isn't clear to me why you want to path_put() on failure.

In earlier versions, we would keep "path" unchanged when the
walk stopped. However, this is not the case in this version
(choose_mountpoint() => in_root => return -EXDEV). So I
decided to just release it, so that we will not leak a path that
the walk should not get to.

>
> I wonder if there might be other potential users in the kernel.
> If so we should consider how well the interface meets their needs.
>
> autofs, devpts, nfsd, landlock all call follow_up...
> maybe they should be using the new interface...
> nfsd is the most likely to benefit - particularly nfsd_lookup_parent().

AFAICT, autofs and devpts can just use follow_up().
For nfsd, nfsd_lookup_parent() and nfsd4_encode_pathname4() can
use path_walk_parent. And 2/5 covers landlock.

I think we can update nfsd in a follow up patch, just to keep this set
simpler.

Thanks,
Song

> Just a thought..

[...]