[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <l7dgle5lhsvfrikd4rqvzuwrqwrseucf5ijgnbi6fxook6dnhj@sc7givb3qimb>
Date: Mon, 19 May 2025 17:20:30 +0200
From: Joel Granados <joel.granados@...nel.org>
To: Petr Pavlu <petr.pavlu@...e.com>
Cc: Luis Chamberlain <mcgrof@...nel.org>,
Sami Tolvanen <samitolvanen@...gle.com>, Daniel Gomez <da.gomez@...sung.com>, Kees Cook <kees@...nel.org>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
Boqun Feng <boqun.feng@...il.com>, Waiman Long <longman@...hat.com>,
"Paul E. McKenney" <paulmck@...nel.org>, Frederic Weisbecker <frederic@...nel.org>,
Neeraj Upadhyay <neeraj.upadhyay@...nel.org>, Joel Fernandes <joel@...lfernandes.org>,
Josh Triplett <josh@...htriplett.org>, Uladzislau Rezki <urezki@...il.com>,
Steven Rostedt <rostedt@...dmis.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Lai Jiangshan <jiangshanlai@...il.com>, Zqiang <qiang.zhang1211@...il.com>,
Andrew Morton <akpm@...ux-foundation.org>, "James E.J. Bottomley" <James.Bottomley@...senpartnership.com>,
Helge Deller <deller@....de>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jirislaby@...nel.org>, linux-modules@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-fsdevel@...r.kernel.org, rcu@...r.kernel.org, linux-mm@...ck.org,
linux-parisc@...r.kernel.org, linux-serial@...r.kernel.org
Subject: Re: [PATCH 01/12] module: Move modprobe_path and modules_disabled
ctl_tables into the module subsys
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.pavlu@...e.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
Download attachment "signature.asc" of type "application/pgp-signature" (660 bytes)
Powered by blists - more mailing lists