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 <joel.grana...@kernel.org>
> > [...]
> > --- 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
