Re: [PATCH v3 2/2] vfs: avoid duplicating creds in faccessat if possible

From: Al Viro <viro@zeniv.linux.org.uk>
To: Mateusz Guzik <mjguzik@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Christian Brauner <brauner@kernel.org>,
	serge@hallyn.com, paul@paul-moore.com,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org
Subject: Re: [PATCH v3 2/2] vfs: avoid duplicating creds in faccessat if possible
Date: Thu, 2 Mar 2023 18:18:32 +0000	[thread overview]
Message-ID: <ZADoeOiJs6BRLUSd@ZenIV> (raw)
In-Reply-To: <CAGudoHF9WKoKhKRHOH_yMsPnX+8Lh0fXe+y-K26mVR0gajEhaQ@mail.gmail.com>

On Thu, Mar 02, 2023 at 07:14:24PM +0100, Mateusz Guzik wrote:
> On 3/2/23, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > On Thu, Mar 2, 2023 at 12:30 AM Christian Brauner <brauner@kernel.org>
> > wrote:
> >>
> >> Fwiw, as long as you, Al, and others are fine with it and I'm aware of
> >> it I'm happy to pick up more stuff like this. I've done it before and
> >> have worked in this area so I'm happy to help with some of the load.
> >
> > Yeah, that would work. We've actually had discussions of vfs
> > maintenance in general.
> >
> > In this case it really wasn't an issue, with this being just two
> > fairly straightforward patches for code that I was familiar with.
> >
> 
> Well on that note I intend to write a patch which would add a flag to
> the dentry cache.
> 
> There is this thing named CONFIG_INIT_ON_ALLOC_DEFAULT_ON which is
> enabled at least on debian, ubuntu and arch. It results in mandatory
> memset on the obj before it gets returned from kmalloc, for hardening
> purposes.
> 
> Now the problem is that dentry cache allocates bufs 4096 bytes in
> size, so this is an equivalent of a clear_page call and it happens
> *every time* there is a path lookup.

Huh?  Quite a few path lookups don't end up allocating any dentries;
what exactly are you talking about?

> Given how dentry cache is used, I'm confident there is 0 hardening
> benefit for it.
> 
> So the plan would be to add a flag on cache creation to exempt it from
> the mandatory memset treatment and use it with dentry.
> 
> I don't have numbers handy but as you can imagine it gave me a nice bump :)
> 
> Whatever you think about the idea aside, the q is: can something like
> the above go in without Al approving it?

That one I would really like to take a look at.