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

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]

Date:   Thu, 2 Mar 2023 19:22:17 +0100
From:   Mateusz Guzik <mjguzik@...il.com>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Christian Brauner <brauner@...nel.org>, serge@...lyn.com,
        paul@...l-moore.com, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: [PATCH v3 2/2] vfs: avoid duplicating creds in faccessat if possible

On 3/2/23, Al Viro <viro@...iv.linux.org.uk> wrote:
> On Thu, Mar 02, 2023 at 07:14:24PM +0100, Mateusz Guzik wrote:
>> On 3/2/23, Linus Torvalds <torvalds@...ux-foundation.org> wrote:
>> > On Thu, Mar 2, 2023 at 12:30 AM Christian Brauner <brauner@...nel.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?
>

Ops, I meant "names_cache", here:
	names_cachep = kmem_cache_create_usercopy("names_cache", PATH_MAX, 0,
			SLAB_HWCACHE_ALIGN|SLAB_PANIC, 0, PATH_MAX, NULL);

it is fs/dcache.c and I brainfarted into the above.

>> 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.
>

allright

-- 
Mateusz Guzik <mjguzik gmail.com>