Re: [RFC][PATCH 10/13] get rid of audit_reusename()

[Linus Torvalds <torvalds@linux-foundation.org>]

[-- Attachment #1: Type: text/plain, Size: 1504 bytes --]

On Sun, 9 Nov 2025 at 11:18, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Hmm? Comments?

Oh, and while double-checking bad users, I note that ntfs3 does

        uni = kmem_cache_alloc(names_cachep, GFP_NOWAIT);
...
        kmem_cache_free(names_cachep, uni);

which is all complete and utter bogosity. I have no idea why anybody
ever thought that was acceptable. It's garbage.

That "uni" isn't even a filename - of either kind. It's neither a
"struct filename" nor a PATH_MAX buffer. It's a "struct cpu_str *"
which is a UTF16 thing that has absolutely nothing to do with
names_cachep, and should never have been allocated that way. It's pure
random insanity.

It should just do a "kmalloc/kfree", with the size being 512 (255
UTF16 characters plus two bytes for len/unused).

Anyway, slightly updated patch that makes "names_cachep" local to
fs/namei.c just because there is absolutely _no_ reason for anybody
else to ever use it. Except for that insane legacy one of __getname(),
that is now just a kmalloc.

I also made EMBEDDED_NAME_MAX be 128 as per Mateusz' comment, although
to avoid double allocations it should probably be even bigger. A
"small" value is good for testing that the new logic works, though.

I haven't actually dared trying to boot into this, so it's still
entirely untested. But I've at least looked through that patch a bit
more and tried to search for other insane patterns, and so far that
oddity in ntfs3 was the only related thing I've found.

        Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 7162 bytes --]

 fs/dcache.c        |  8 +----
 fs/namei.c         | 86 ++++++++++++++++++++++++++++--------------------------
 fs/ntfs3/namei.c   |  4 +--
 include/linux/fs.h | 11 +++----
 4 files changed, 54 insertions(+), 55 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 035cccbc9276..bd4432f46d15 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -3246,10 +3246,6 @@ static void __init dcache_init(void)
 	runtime_const_init(ptr, dentry_hashtable);
 }
 
-/* SLAB cache for __getname() consumers */
-struct kmem_cache *names_cachep __ro_after_init;
-EXPORT_SYMBOL(names_cachep);
-
 void __init vfs_caches_init_early(void)
 {
 	int i;
@@ -3263,9 +3259,7 @@ void __init vfs_caches_init_early(void)
 
 void __init vfs_caches_init(void)
 {
-	names_cachep = kmem_cache_create_usercopy("names_cache", PATH_MAX, 0,
-			SLAB_HWCACHE_ALIGN|SLAB_PANIC, 0, PATH_MAX, NULL);
-
+	namei_init();
 	dcache_init();
 	inode_init();
 	files_init();
diff --git a/fs/namei.c b/fs/namei.c
index 7377020a2cba..aaede1892133 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -123,7 +123,26 @@
  * PATH_MAX includes the nul terminator --RR.
  */
 
-#define EMBEDDED_NAME_MAX	(PATH_MAX - offsetof(struct filename, iname))
+/* SLAB cache for alloc_filename() consumers */
+static struct kmem_cache *names_cachep __ro_after_init;
+
+void __init namei_init(void)
+{
+	names_cachep = kmem_cache_create_usercopy("names_cache",
+			sizeof(struct filename), 0, SLAB_PANIC,
+			offsetof(struct filename, iname), EMBEDDED_NAME_MAX,
+			NULL);
+}
+
+static inline struct filename *alloc_filename(void)
+{
+	return kmem_cache_alloc(names_cachep, GFP_KERNEL);
+}
+
+static inline void free_filename(struct filename *name)
+{
+	kmem_cache_free(names_cachep, name);
+}
 
 static inline void initname(struct filename *name, const char __user *uptr)
 {
@@ -143,7 +162,7 @@ getname_flags(const char __user *filename, int flags)
 	if (result)
 		return result;
 
-	result = __getname();
+	result = alloc_filename();
 	if (unlikely(!result))
 		return ERR_PTR(-ENOMEM);
 
@@ -160,55 +179,42 @@ getname_flags(const char __user *filename, int flags)
 	 */
 	if (unlikely(len <= 0)) {
 		if (unlikely(len < 0)) {
-			__putname(result);
+			free_filename(result);
 			return ERR_PTR(len);
 		}
 
 		/* The empty path is special. */
 		if (!(flags & LOOKUP_EMPTY)) {
-			__putname(result);
+			free_filename(result);
 			return ERR_PTR(-ENOENT);
 		}
 	}
 
 	/*
 	 * Uh-oh. We have a name that's approaching PATH_MAX. Allocate a
-	 * separate struct filename so we can dedicate the entire
-	 * names_cache allocation for the pathname, and re-do the copy from
-	 * userland.
+	 * separate pathname, copy the partial result we already did, and
+	 * then copy the rest of the pathname from user space.
 	 */
 	if (unlikely(len == EMBEDDED_NAME_MAX)) {
-		const size_t size = offsetof(struct filename, iname[1]);
-		kname = (char *)result;
-
-		/*
-		 * size is chosen that way we to guarantee that
-		 * result->iname[0] is within the same object and that
-		 * kname can't be equal to result->iname, no matter what.
-		 */
-		result = kzalloc(size, GFP_KERNEL);
-		if (unlikely(!result)) {
-			__putname(kname);
+		kname = kmalloc(PATH_MAX, GFP_KERNEL);
+		if (unlikely(!kname)) {
+			free_filename(result);
 			return ERR_PTR(-ENOMEM);
 		}
-		result->name = kname;
-		len = strncpy_from_user(kname, filename, PATH_MAX);
+		memcpy(kname, result->iname, EMBEDDED_NAME_MAX);
+
+		// Copy remaining part of the name
+		len = strncpy_from_user(kname + EMBEDDED_NAME_MAX,
+			filename + EMBEDDED_NAME_MAX,
+			PATH_MAX-EMBEDDED_NAME_MAX);
+		if (unlikely(len == PATH_MAX-EMBEDDED_NAME_MAX))
+			len = -ENAMETOOLONG;
 		if (unlikely(len < 0)) {
-			__putname(kname);
-			kfree(result);
+			free_filename(result);
+			kfree(kname);
 			return ERR_PTR(len);
 		}
-		/* The empty path is special. */
-		if (unlikely(!len) && !(flags & LOOKUP_EMPTY)) {
-			__putname(kname);
-			kfree(result);
-			return ERR_PTR(-ENOENT);
-		}
-		if (unlikely(len == PATH_MAX)) {
-			__putname(kname);
-			kfree(result);
-			return ERR_PTR(-ENAMETOOLONG);
-		}
+		result->name = kname;
 	}
 	initname(result, filename);
 	audit_getname(result);
@@ -246,7 +252,7 @@ struct filename *getname_kernel(const char * filename)
 	struct filename *result;
 	int len = strlen(filename) + 1;
 
-	result = __getname();
+	result = alloc_filename();
 	if (unlikely(!result))
 		return ERR_PTR(-ENOMEM);
 
@@ -258,13 +264,13 @@ struct filename *getname_kernel(const char * filename)
 
 		tmp = kmalloc(size, GFP_KERNEL);
 		if (unlikely(!tmp)) {
-			__putname(result);
+			free_filename(result);
 			return ERR_PTR(-ENOMEM);
 		}
 		tmp->name = (char *)result;
 		result = tmp;
 	} else {
-		__putname(result);
+		free_filename(result);
 		return ERR_PTR(-ENAMETOOLONG);
 	}
 	memcpy((char *)result->name, filename, len);
@@ -290,11 +296,9 @@ void putname(struct filename *name)
 			return;
 	}
 
-	if (name->name != name->iname) {
-		__putname(name->name);
-		kfree(name);
-	} else
-		__putname(name);
+	if (name->name != name->iname)
+		kfree(name->name);
+	free_filename(name);
 }
 EXPORT_SYMBOL(putname);
 
diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
index 82c8ae56beee..5ddbfe17d8e3 100644
--- a/fs/ntfs3/namei.c
+++ b/fs/ntfs3/namei.c
@@ -407,7 +407,7 @@ static int ntfs_d_hash(const struct dentry *dentry, struct qstr *name)
 	/*
 	 * Try slow way with current upcase table
 	 */
-	uni = kmem_cache_alloc(names_cachep, GFP_NOWAIT);
+	uni = kmalloc(2*(NTFS_NAME_LEN + 1), GFP_NOWAIT);
 	if (!uni)
 		return -ENOMEM;
 
@@ -429,7 +429,7 @@ static int ntfs_d_hash(const struct dentry *dentry, struct qstr *name)
 	err = 0;
 
 out:
-	kmem_cache_free(names_cachep, uni);
+	kfree(uni);
 	return err;
 }
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c9588d555f73..9d4707bbc83a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -81,6 +81,7 @@ struct fs_parameter_spec;
 struct file_kattr;
 struct iomap_ops;
 
+extern void __init namei_init(void);
 extern void __init inode_init(void);
 extern void __init inode_init_early(void);
 extern void __init files_init(void);
@@ -2834,12 +2835,13 @@ extern struct kobject *fs_kobj;
 
 /* fs/open.c */
 struct audit_names;
+#define EMBEDDED_NAME_MAX	128
 struct filename {
 	const char		*name;	/* pointer to actual string */
 	const __user char	*uptr;	/* original userland pointer */
 	atomic_t		refcnt;
 	struct audit_names	*aname;
-	const char		iname[];
+	const char		iname[EMBEDDED_NAME_MAX];
 };
 static_assert(offsetof(struct filename, iname) % sizeof(long) == 0);
 
@@ -2959,10 +2961,9 @@ static inline int finish_open_simple(struct file *file, int error)
 extern void __init vfs_caches_init_early(void);
 extern void __init vfs_caches_init(void);
 
-extern struct kmem_cache *names_cachep;
-
-#define __getname()		kmem_cache_alloc(names_cachep, GFP_KERNEL)
-#define __putname(name)		kmem_cache_free(names_cachep, (void *)(name))
+// Crazy old legacy uses for pathname allocations
+#define __getname() kmalloc(PATH_MAX, GFP_KERNEL)
+#define __putname(name) kfree((void *)(name))
 
 extern struct super_block *blockdev_superblock;
 static inline bool sb_is_blkdev_sb(struct super_block *sb)