On Thu, May 15, 2025 at 10:04:53AM +0200, Petr Pavlu wrote: > On 5/9/25 14:54, Joel Granados wrote: > > Move module sysctl (modprobe_path and modules_disabled) out of sysctl.c > > and into the modules subsystem. Make the modprobe_path variable static > > as it no longer needs to be exported. Remove module.h from the includes > > in sysctl as it no longer uses any module exported variables. > > > > This is part of a greater effort to move ctl tables into their > > respective subsystems which will reduce the merge conflicts in > > kernel/sysctl.c. > > > > Signed-off-by: Joel Granados <[email protected]> > > [...] > > --- a/kernel/module/kmod.c > > +++ b/kernel/module/kmod.c > > @@ -60,7 +60,7 @@ static DEFINE_SEMAPHORE(kmod_concurrent_max, > > MAX_KMOD_CONCURRENT); > > /* > > modprobe_path is set via /proc/sys. > > */ > > -char modprobe_path[KMOD_PATH_LEN] = CONFIG_MODPROBE_PATH; > > +static char modprobe_path[KMOD_PATH_LEN] = CONFIG_MODPROBE_PATH; > > > > static void free_modprobe_argv(struct subprocess_info *info) > > { > > @@ -177,3 +177,33 @@ int __request_module(bool wait, const char *fmt, ...) > > return ret; > > } > > EXPORT_SYMBOL(__request_module); > > + > > +#ifdef CONFIG_MODULES > > +static const struct ctl_table kmod_sysctl_table[] = { > > + { > > + .procname = "modprobe", > > + .data = &modprobe_path, > > + .maxlen = KMOD_PATH_LEN, > > + .mode = 0644, > > + .proc_handler = proc_dostring, > > + }, > > + { > > + .procname = "modules_disabled", > > + .data = &modules_disabled, > > + .maxlen = sizeof(int), > > + .mode = 0644, > > + /* only handle a transition from default "0" to "1" */ > > + .proc_handler = proc_dointvec_minmax, > > + .extra1 = SYSCTL_ONE, > > + .extra2 = SYSCTL_ONE, > > + }, > > This is minor.. but the file kernel/module/kmod.c contains the logic to > request direct modprobe invocation by the kernel. Registering the > modprobe_path sysctl here is appropriate. However, the modules_disabled > setting affects the entire module loader so I don't think it's best to > register it here. > > I suggest keeping a single table for the module sysctl values but moving > it to kernel/module/main.c. This means the variable modprobe_path must > retain external linkage, on the other hand, modules_disabled can be made > static.
Like this?:
---
include/linux/module.h | 1 -
kernel/module/main.c | 30 +++++++++++++++++++++++++++++-
kernel/sysctl.c | 20 --------------------
3 files changed, 29 insertions(+), 22 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index d94b196d5a34..25476168e012 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -302,7 +302,6 @@ struct notifier_block;
#ifdef CONFIG_MODULES
-extern int modules_disabled; /* for sysctl */
/* Get/put a kernel symbol (calls must be symmetric) */
void *__symbol_get(const char *symbol);
void *__symbol_get_gpl(const char *symbol);
diff --git a/kernel/module/main.c b/kernel/module/main.c
index a2859dc3eea6..13055ef65f15 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -126,9 +126,37 @@ static void mod_update_bounds(struct module *mod)
}
/* Block module loading/unloading? */
-int modules_disabled;
+static int modules_disabled;
core_param(nomodule, modules_disabled, bint, 0);
+static const struct ctl_table kmod_sysctl_table[] = {
+ {
+ .procname = "modprobe",
+ .data = &modprobe_path,
+ .maxlen = KMOD_PATH_LEN,
+ .mode = 0644,
+ .proc_handler = proc_dostring,
+ },
+ {
+ .procname = "modules_disabled",
+ .data = &modules_disabled,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ /* only handle a transition from default "0" to "1" */
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ONE,
+ .extra2 = SYSCTL_ONE,
+ },
+};
+
+static int __init init_kmod_sysctl(void)
+{
+ register_sysctl_init("kernel", kmod_sysctl_table);
+ return 0;
+}
+
+subsys_initcall(init_kmod_sysctl);
+
/* Waiting for a module to finish initializing? */
static DECLARE_WAIT_QUEUE_HEAD(module_wq);
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 9b4f0cff76ea..473133d9651e 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -19,7 +19,6 @@
* Removed it and replaced it with older style, 03/23/00, Bill Wendling
*/
-#include <linux/module.h>
#include <linux/sysctl.h>
#include <linux/bitmap.h>
#include <linux/printk.h>
@@ -1616,25 +1615,6 @@ static const struct ctl_table kern_table[] = {
.proc_handler = proc_dointvec,
},
#endif
-#ifdef CONFIG_MODULES
- {
- .procname = "modprobe",
- .data = &modprobe_path,
- .maxlen = KMOD_PATH_LEN,
- .mode = 0644,
- .proc_handler = proc_dostring,
- },
- {
- .procname = "modules_disabled",
- .data = &modules_disabled,
- .maxlen = sizeof(int),
- .mode = 0644,
- /* only handle a transition from default "0" to "1" */
- .proc_handler = proc_dointvec_minmax,
- .extra1 = SYSCTL_ONE,
- .extra2 = SYSCTL_ONE,
- },
-#endif
#ifdef CONFIG_UEVENT_HELPER
{
.procname = "hotplug",
--
2.47.2
--
Joel Granados
signature.asc
Description: PGP signature
