lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <g3e3ygz4jb73b3zhxexpwacwui3imlwauujzeq2nlopp2i2fjp@lzj33hcwztc2>
Date: Thu, 15 May 2025 12:04:04 +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 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.granados@...nel.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

Download attachment "signature.asc" of type "application/pgp-signature" (660 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ