linux-modules.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Luis Chamberlain <mcgrof@kernel.org>
To: linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org,
	pmladek@suse.com, david@redhat.com, petr.pavlu@suse.com,
	prarit@redhat.com
Cc: christophe.leroy@csgroup.eu, song@kernel.org, mcgrof@kernel.org
Subject: [RFT 5/5] module: add a sanity check prior to allowing kernel module auto-loading
Date: Sun, 19 Mar 2023 14:49:26 -0700	[thread overview]
Message-ID: <20230319214926.1794108-6-mcgrof@kernel.org> (raw)
In-Reply-To: <20230319214926.1794108-1-mcgrof@kernel.org>

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@kernel.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


      parent reply	other threads:[~2023-03-19 21:49 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-19 21:49 [RFT 0/5] module: avoid userspace pressure on unwanted allocations Luis Chamberlain
2023-03-19 21:49 ` [RFT 1/5] module: move finished_loading() Luis Chamberlain
2023-03-19 21:49 ` [RFT 2/5] module: extract patient module check into helper Luis Chamberlain
2023-03-19 21:49 ` [RFT 3/5] module: avoid allocation if module is already present and ready Luis Chamberlain
2023-03-19 21:49 ` [RFT 4/5] module: use list_add_tail_rcu() when adding module Luis Chamberlain
2023-03-19 21:49 ` Luis Chamberlain [this message]

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=20230319214926.1794108-6-mcgrof@kernel.org \
    --to=mcgrof@kernel.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=petr.pavlu@suse.com \
    --cc=pmladek@suse.com \
    --cc=prarit@redhat.com \
    --cc=song@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).