linux-kernel - [RFT 5/5] module: add a sanity check prior to allowing kernel module auto-loading

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]

Date:   Sun, 19 Mar 2023 14:49:26 -0700
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     linux-modules@...r.kernel.org, linux-kernel@...r.kernel.org,
        pmladek@...e.com, david@...hat.com, petr.pavlu@...e.com,
        prarit@...hat.com
Cc:     christophe.leroy@...roup.eu, song@...nel.org, mcgrof@...nel.org
Subject: [RFT 5/5] module: add a sanity check prior to allowing kernel module auto-loading

request_module() is what we use for kernel-module autoloading. But
this today is pretty stupid, it just requests for the module without
even checking if its needed. And so we bounce to userspace and hope
for the best.

We can instead do a simple check to see if the module is already
present. This will however contend the module_mutex, but it should
never be heavily contended. However, module auto-loading is a special
use case in the kernel and kernel/kmod already limits the amount of
concurrent requests from the kernel to 50 so we know the kernel can
only contend on the module_mutex for querying if a module exists 50
times at any single point in time.

This work is only valuable if it proves useful for booting up large
systems where a lot of current kernel heuristics use many kernel
module auto-loading features and they could benefit from this. There
are no metrics yet to show, but at least this doesn't penalize much
existing systems.

Signed-off-by: Luis Chamberlain <mcgrof@...nel.org>
---
 kernel/module/internal.h |  1 +
 kernel/module/kmod.c     |  7 +++++++
 kernel/module/main.c     | 18 ++++++++++++++++++
 3 files changed, 26 insertions(+)

diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 6ae29bb8836f..cb00555780bd 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -86,6 +86,7 @@ struct find_symbol_arg {
 	enum mod_license license;
 };
 
+bool module_autoload_proceed(const char *name);
 int mod_verify_sig(const void *mod, struct load_info *info);
 int try_to_force_load(struct module *mod, const char *reason);
 bool find_symbol(struct find_symbol_arg *fsa);
diff --git a/kernel/module/kmod.c b/kernel/module/kmod.c
index b717134ebe17..67efc6de902f 100644
--- a/kernel/module/kmod.c
+++ b/kernel/module/kmod.c
@@ -28,6 +28,8 @@
 
 #include <trace/events/module.h>
 
+#include "internal.h"
+
 /*
  * Assuming:
  *
@@ -167,8 +169,13 @@ int __request_module(bool wait, const char *fmt, ...)
 
 	trace_module_request(module_name, wait, _RET_IP_);
 
+	if (!module_autoload_proceed(module_name)) {
+		ret = 0;
+		goto out;
+	}
 	ret = call_modprobe(module_name, wait ? UMH_WAIT_PROC : UMH_WAIT_EXEC);
 
+out:
 	atomic_inc(&kmod_concurrent_max);
 	wake_up(&kmod_wq);
 
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 32955b7819b3..2a84d747865a 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2665,6 +2665,24 @@ static int module_patient_check_exists(const char *name)
 		return -EBUSY;
 }
 
+/*
+ * We allow contention of the module_mutex here because kernel module
+ * auto-loading a special feature *and* because we are only allowing
+ * MAX_KMOD_CONCURRENT possible checks at a time for a module.
+ */
+bool module_autoload_proceed(const char *name)
+{
+	int err;
+
+	mutex_lock(&module_mutex);
+	err = module_patient_check_exists(name);
+	mutex_unlock(&module_mutex);
+
+	if (err == -EEXIST)
+		return false;
+	return true;
+}
+
 /*
  * We try to place it in the list now to make sure it's unique before
  * we dedicate too many resources.  In particular, temporary percpu
-- 
2.39.1