On Thu, May 15, 2025 at 02:45:22PM +0200, Petr Pavlu wrote: > On 5/15/25 12:04, Joel Granados wrote: > > 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. ... > > Like this?: > > [...] > > Let's also move the KMOD_PATH_LEN definition and the modprobe_path > declaration from include/linux/kmod.h to kernel/module/internal.h, as > they are now fully internal to the module loader, and use "module" > instead of "kmod" in the sysctl registration to avoid confusion with the > modprobe logic. > > The adjusted patch is below. > > Reviewed-by: Petr Pavlu <petr.pa...@suse.com> Thx for review and patch. Have applied it to my current V2 branch [1]
Best [1] https://git.kernel.org/pub/scm/linux/kernel/git/joel.granados/linux.git/log/?h=jag/mv_ctltables_iter2 > > -- > Thanks, > Petr > > > diff --git a/include/linux/kmod.h b/include/linux/kmod.h > index 68f69362d427..9a07c3215389 100644 > --- a/include/linux/kmod.h > +++ b/include/linux/kmod.h > @@ -14,10 +14,7 @@ > #include <linux/workqueue.h> > #include <linux/sysctl.h> > > -#define KMOD_PATH_LEN 256 > - > #ifdef CONFIG_MODULES > -extern char modprobe_path[]; /* for sysctl */ > /* modprobe exit status on success, -ve on error. Return value > * usually useless though. */ > extern __printf(2, 3) > diff --git a/include/linux/module.h b/include/linux/module.h > index 8050f77c3b64..f4ab8d90c475 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -304,7 +304,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/internal.h b/kernel/module/internal.h > index 626cf8668a7e..0954c8de00c2 100644 > --- a/kernel/module/internal.h > +++ b/kernel/module/internal.h > @@ -58,6 +58,9 @@ extern const struct kernel_symbol __stop___ksymtab_gpl[]; > extern const u32 __start___kcrctab[]; > extern const u32 __start___kcrctab_gpl[]; > > +#define KMOD_PATH_LEN 256 > +extern char modprobe_path[]; > + > struct load_info { > const char *name; > /* pointer to module in temporary copy, freed at end of load_module() */ > diff --git a/kernel/module/main.c b/kernel/module/main.c > index a2859dc3eea6..a336b7b3fb23 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 module_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_module_sysctl(void) > +{ > + register_sysctl_init("kernel", module_sysctl_table); > + return 0; > +} > + > +subsys_initcall(init_module_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", > -- Joel Granados
signature.asc
Description: PGP signature
