linux-kernel - Re: [PATCH] lib/genalloc: use try_cmpxchg in {set,clear}_bits

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] [day] [month] [year] [list]

Message-ID: <CAHk-=wih--CcOho+zAp9BM4a2GXVE6QfT1t50=G8ZPS99OVDZQ@mail.gmail.com>
Date:   Fri, 27 Jan 2023 11:25:31 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     Mateusz Guzik <mjguzik@...il.com>, Uros Bizjak <ubizjak@...il.com>,
        David Laight <David.Laight@...lab.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] lib/genalloc: use try_cmpxchg in {set,clear}_bits_ll

On Thu, Jan 26, 2023 at 7:54 PM Al Viro <viro@...iv.linux.org.uk> wrote:
>
> >
> > And extending the LOOKUP_RCU window all the way over the stat info
> > gathering would require a lot of care, and force us to expose the
> > kinds of things we do for LOOKUP_RCU in namei.c to fs/stat.c too.
>
> Interesting...  So basically a variant of filename_lookup() that
> fills struct kstat instead of doing that to struct path?

Well, kinda. You still want the fallback to struct path in case you
can't fill in the kstat without it (because the filesystem needs to do
something under RCU).

So I think what you'd really want is something like a special version
of filename_lookup() that is then given an optimistic "call this
function under RCU before you finalize the path".

And then, *if* the filesystem can do the kstat lookup etc under RCU,
it can just fill it in there, and instead of finalizing the path, we
can just do terminate the walk without ever doing the try_to_unlazy()
that legitimizes the path.

I suspect it's fairly close to how we do d_revalidate() for the path
component, except we'd do this not per-component, but as a "for the
final result, let's do one last thing under RCU, and if it succeeded
there, we don't actually need the path at all after all, because we've
already done everything we needed".

I think the only really relevant situation this is the case is
basically the stat() family of functions, since those don't actually
care about the path after the operation.

But there are other cases that have that

        error = filename_lookup(dfd, filename, lookup_flags, &path, NULL);
        ... do something simple once ...
        path_put(&path);

pattern where we just do the 'put' on the path and don't have any
other use for it.

The io_uring xattr code matches that same pattern, for example - but
may simply not be worth worrying about.

So either some generic "callback under RCU before we finalize it", or
we could make it very specific for just "fill in kstat and don't
bother finalizing the path when this flag is set"

> Looks like the main obstacle is how to deal with duplication between
> that thing and vfs_getattr{,_nosec}();

I think we'd need something like a new ->rcu_getattr() function, and
filesystems that can do getattr under RCU would just set that function
pointer.

I dunno. I didn't look at it all, but it *feels* like you could have
something that just says "if I have this function, and calling it
returns success, then we can just do "terminate_walk()" without ever
doing "try_to_unlazy()".

                 Linus