All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexey Dobriyan <adobriyan@gmail.com>
To: Luis Chamberlain <mcgrof@kernel.org>, akpm@linux-foundation.org
Cc: linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, viro@zeniv.linux.org.uk,
	brauner@kernel.org, jack@suse.cz
Subject: [PATCH] module: ban '.', '..' as module names, ban '/' in module names
Date: Sun, 14 Apr 2024 22:05:05 +0300	[thread overview]
Message-ID: <ee371cf7-69fa-4f9c-99b9-59bab86f25e4@p183> (raw)

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'.

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.

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.

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.

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@gmail.com>
---

 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)

             reply	other threads:[~2024-04-14 19:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-14 19:05 Alexey Dobriyan [this message]
2024-04-14 20:58 ` [PATCH] module: ban '.', '..' as module names, ban '/' in module names Luis Chamberlain
2024-04-15 17:23   ` Alexey Dobriyan
2024-04-15 17:40     ` Dr. David Alan Gilbert
2024-04-15  0:35 ` Matthew Wilcox
2024-04-15  8:08 ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ee371cf7-69fa-4f9c-99b9-59bab86f25e4@p183 \
    --to=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.