linux-kernel - Re: [PATCH v2] fs: hide names_cache behind runtime const machinery

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: <CAGudoHFD6bWhp-8821Pb6cDAEnR9N8UFEj9qT7G-_v0FOS+_vg@mail.gmail.com>
Date: Tue, 2 Dec 2025 07:18:16 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: Al Viro <viro@...iv.linux.org.uk>
Cc: brauner@...nel.org, jack@...e.cz, linux-kernel@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v2] fs: hide names_cache behind runtime const machinery

On Tue, Dec 2, 2025 at 6:52 AM Al Viro <viro@...iv.linux.org.uk> wrote:
>
> On Tue, Dec 02, 2025 at 06:10:36AM +0100, Mateusz Guzik wrote:
>
> > So IIUC whatever APIs aside, the crux of this idea is to have
> > kmem_cache objs defined instead of having pointers to them, as in:
> > -struct kmem_cache *names_cachep __ro_after_init;
> > +struct kmem_cache names_cachep __ro_after_init;
>
> Huh?  __ro_after_init will break instantly - the contents changes with
> each allocation, after all.  What I want is
> static struct kmem_cache_store names_cache;
>

c'mon man, I copy pasted the existing line and removed the asterisk to
de-pointer it to make for illustrative purposes. You went straight to
description how to make your idea happen, so I wanted to make sure we
are on the same page on what it is.

> As for the many places to modify...
>
> fs/file.c:390:  newf = kmem_cache_alloc(files_cachep, GFP_KERNEL);
> fs/file.c:422:                  kmem_cache_free(files_cachep, newf);
> fs/file.c:514:          kmem_cache_free(files_cachep, files);
> include/linux/fdtable.h:116:extern struct kmem_cache *files_cachep;
> kernel/fork.c:429:struct kmem_cache *files_cachep;
> kernel/fork.c:2987:     files_cachep = kmem_cache_create("files_cache",
> samples/kmemleak/kmemleak-test.c:52:    pr_info("kmem_cache_alloc(files_cachep) = 0x%px\n",
> samples/kmemleak/kmemleak-test.c:53:            kmem_cache_alloc(files_cachep, GFP_KERNEL));
> samples/kmemleak/kmemleak-test.c:54:    pr_info("kmem_cache_alloc(files_cachep) = 0x%px\n",
> samples/kmemleak/kmemleak-test.c:55:            kmem_cache_alloc(files_cachep, GFP_KERNEL));
>
> I would argue for making it static in fs/file.c, where we have the grand
> total of 3 places using the sucker, between two functions.
>

The claim was not that your idea results in insurmountable churn. The
claim was *both* your idea and runtime const require churn on per kmem
cache basis. Then the question is if one is going to churn it
regardless, why this way over runtime const. I do think the runtime
thing is a little bit less churn and less work on the mm side to get
it going, but then the runtime thing *itself* needs productizing
(which I'm not signing up to do).

Per the previous e-mail I don't have a strong opinion myself and it is
the mm folk who need either idea sold to anyway.