linux-kernel - Re: [PATCH] module: ban '.', '..' as module names, ban '/' in module names

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <vqeq3ioklrgrf227zgdfho4virh74qrt5reoyptmzgktyronbr@c2mw32pqikft>
Date: Wed, 19 Feb 2025 14:35:59 -0600
From: Lucas De Marchi <lucas.demarchi@...el.com>
To: Alexey Dobriyan <adobriyan@...il.com>
CC: Luis Chamberlain <mcgrof@...nel.org>, <akpm@...ux-foundation.org>,
	<linux-modules@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-fsdevel@...r.kernel.org>, <viro@...iv.linux.org.uk>,
	<brauner@...nel.org>, <jack@...e.cz>
Subject: Re: [PATCH] module: ban '.', '..' as module names, ban '/' in module
 names

On Sun, Apr 14, 2024 at 10:05:05PM +0300, Alexey Dobriyan wrote:
>As the title says, ban
>
>	.
>	..
>
>and any name containing '/' as they show in sysfs as directory names:
>
>	/sys/module/${mod.name}
>
>sysfs tries to mangle the name and make '/' into '!' which kind of work
>but not really.
>
>Corrupting simple module to have name '/est' and loading it works:
>
>	# insmod xxx.ko
>
>	$ cat /proc/modules
>	/est 12288 0 - Live 0x0000000000000000 (P)
>
>/proc has no problems with it as it ends in data not pathname.
>
>sysfs mangles it to '/sys/module/!test'.

did you mean !est?

>
>lsmod is confused:
>
>	$ lsmod
>	Module                  Size  Used by
>	libkmod: ERROR ../libkmod/libkmod-module.c:1998 kmod_module_get_holders: could not open '/sys/module//est/holders': No such file or directory
>	/est                      -2  -2
>
>Size and refcount are bogus entirely.
>
>Apparently lsmod doesn't know about sysfs mangling scheme.

correct

>
>Worse, rmmod doesn't work too:
>
>	$ sudo rmmod '/est'
>	rmmod: ERROR: Module /est is not currently loaded
>
>I don't even want to know what it is doing.

because of the missing sysfs entry above... it first checks if the
module is loaded.

>
>Practically there is no nice way for the admin to get rid of the module,
>so we should just ban such names. Writing small program to just delete
>module by name could possibly work maybe.

--force drops the check for "is it loaded?" and should work

>
>Any other subsystem should use nice helper function aptly named
>
>	string_is_vfs_ready()
>
>and apply additional restrictions if necessary.
>
>/proc/modules hints that newlines should be banned too,
>and \x1f, and whitespace, and similar looking characters
>from different languages and emojis (except 🐧obviously).
>
>Signed-off-by: Alexey Dobriyan <adobriyan@...il.com>

But I agree that it would be better to ban these chars from module
names. I don't think we'd ever merge such a module in tree neither.

Lucas De Marchi

>---
>
> include/linux/fs.h   |    8 ++++++++
> kernel/module/main.c |    5 +++++
> 2 files changed, 13 insertions(+)
>
>--- a/include/linux/fs.h
>+++ b/include/linux/fs.h
>@@ -3616,4 +3616,12 @@ extern int vfs_fadvise(struct file *file, loff_t offset, loff_t len,
> extern int generic_fadvise(struct file *file, loff_t offset, loff_t len,
> 			   int advice);
>
>+/*
>+ * Use this if data from userspace end up as directory/filename on
>+ * some virtual filesystem.
>+ */
>+static inline bool string_is_vfs_ready(const char *s)
>+{
>+	return strcmp(s, ".") != 0 && strcmp(s, "..") != 0 && !strchr(s, '/');
>+}
> #endif /* _LINUX_FS_H */
>--- a/kernel/module/main.c
>+++ b/kernel/module/main.c
>@@ -2893,6 +2893,11 @@ static int load_module(struct load_info *info, const char __user *uargs,
>
> 	audit_log_kern_module(mod->name);
>
>+	if (!string_is_vfs_ready(mod->name)) {
>+		err = -EINVAL;
>+		goto free_module;
>+	}
>+
> 	/* Reserve our place in the list. */
> 	err = add_unformed_module(mod);
> 	if (err)