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: <20251202055258.GB1712166@ZenIV>
Date: Tue, 2 Dec 2025 05:52:58 +0000
From: Al Viro <viro@...iv.linux.org.uk>
To: Mateusz Guzik <mjguzik@...il.com>
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 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;

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.

dentry_cache:
fs/dcache.c:345:        kmem_cache_free(dentry_cache, dentry); 
fs/dcache.c:352:        kmem_cache_free(dentry_cache, dentry);
fs/dcache.c:1690:       dentry = kmem_cache_alloc_lru(dentry_cache, &sb->s_dentry_lru,
fs/dcache.c:1711:                       kmem_cache_free(dentry_cache, dentry); 
fs/dcache.c:1748:                       kmem_cache_free(dentry_cache, dentry);

5 lines, between 3 functions (__d_free(), __d_free_external(), __d_allock()).

mnt_cache:
fs/namespace.c:293:     struct mount *mnt = kmem_cache_zalloc(mnt_cache, GFP_KERNEL);
fs/namespace.c:342:     kmem_cache_free(mnt_cache, mnt);
fs/namespace.c:737:     kmem_cache_free(mnt_cache, mnt);

3 lines, alloc_vfsmnt() and free_vfsmnt()

sock_inode_cachep:
net/socket.c:322:       ei = alloc_inode_sb(sb, sock_inode_cachep, GFP_KERNEL);
net/socket.c:343:       kmem_cache_free(sock_inode_cachep, ei);

2 lines, sock_alloc_inode() and sock_free_inode() (sockets are coallocated with
inodes).

struct filename: two lines after that series.

task_struct_cachep:
kernel/fork.c:184:      return kmem_cache_alloc_node(task_struct_cachep, GFP_KERNEL, node);
kernel/fork.c:189:      kmem_cache_free(task_struct_cachep, tsk);

and so it goes; that's the sane pattern - you want few places where objects of given
type are allocated and freed, so that tracing the callchains would be feasible.
names_cachep used to be shitty in that respect, what with its abuse by weird __getname()
callers.  It's not the common situation, thankfully.

The delicate part is headers, indeed - we don't want to expose struct kmem_cache guts
anywhere outside of mm/*, and not the entire mm/* either.  But that's not hard to
deal with - see include/generate/bounds.h, include/generate/rq-offsets.h, etc.
Exact same technics can be used to get sizeof(struct kmem_cache) calculated and
put into generated header.  Then we get something like struct kmem_cache_store with
the right size and alignment, and _that_ would be what the variables would be.
With static inline struct kmem_cache *to_kmem_cache(struct kmem_cache_store *)
returning a cast and e.g.

static inline void free_filename(struct __filename *p)
{
        kmem_cache_free(to_kmem_cache(&names_cache), p);
}

as an example of use.

Anyway, for now I've applied your patch pretty much as-is; conversion of the
sort described above can be done afterwards just fine.