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

Linus Torvalds <[email protected]>
Newsgroups org.kernel.vger.audit,org.kernel.vger.io-uring,org.kernel.vger.linux-fsdevel
Message-ID <CAHk-=wjA=iXRyu1-ABST43vdT60Md9zpQDJ4Kg14V3f_2Bf+BA@mail.gmail.com> 
On Sun, 9 Nov 2025 at 11:18, Linus Torvalds
<[email protected]> 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
patch.diff (text/x-patch, 7 KB) 
 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)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.