mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/3] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n
@ 2026-03-13 13:48 Petr Pavlu
  2026-03-13 13:48 ` [PATCH v2 1/3] " Petr Pavlu
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Petr Pavlu @ 2026-03-13 13:48 UTC (permalink / raw)
  To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
  Cc: Christophe Leroy, Aaron Tomlin, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, linux-modules, linux-kernel

Fix freeing of charp module parameters when CONFIG_SYSFS=n and, related to
this, update moduleparam.h to keep its coding style consistent.

Changes since v1 [1]:
* Remove the extern keyword from the declaration of module_destroy_params()
  and update the type of its num parameter from `unsigned` to
  `unsigned int`.
* Add a cleanup patch for parse_args() to similarly update its num
  parameter to `unsigned int` and to synchronize the parameter names
  between its prototype and definition.
* Add a cleanup patch to drop the unnecessary extern keyword for all
  function declarations in moduleparam.h.

[1] https://lore.kernel.org/linux-modules/20260306125457.1377402-1-petr.pavlu@suse.com/

Petr Pavlu (3):
  module: Fix freeing of charp module parameters when CONFIG_SYSFS=n
  module: Clean up parse_args() arguments
  module: Remove extern keyword from param prototypes

 include/linux/moduleparam.h | 100 +++++++++++++++++-------------------
 kernel/module/main.c        |   4 +-
 kernel/params.c             |  29 +++++++----
 3 files changed, 68 insertions(+), 65 deletions(-)


base-commit: 0257f64bdac7fdca30fa3cae0df8b9ecbec7733a
-- 
2.53.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/3] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n
  2026-03-13 13:48 [PATCH v2 0/3] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n Petr Pavlu
@ 2026-03-13 13:48 ` Petr Pavlu
  2026-03-13 13:48 ` [PATCH v2 2/3] module: Clean up parse_args() arguments Petr Pavlu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Petr Pavlu @ 2026-03-13 13:48 UTC (permalink / raw)
  To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
  Cc: Christophe Leroy, Aaron Tomlin, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, linux-modules, linux-kernel

When setting a charp module parameter, the param_set_charp() function
allocates memory to store a copy of the input value. Later, when the module
is potentially unloaded, the destroy_params() function is called to free
this allocated memory.

However, destroy_params() is available only when CONFIG_SYSFS=y, otherwise
only a dummy variant is present. In the unlikely case that the kernel is
configured with CONFIG_MODULES=y and CONFIG_SYSFS=n, this results in
a memory leak of charp values when a module is unloaded.

Fix this issue by making destroy_params() always available when
CONFIG_MODULES=y. Rename the function to module_destroy_params() to clarify
that it is intended for use by the module loader.

Fixes: e180a6b7759a ("param: fix charp parameters set via sysfs")
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
 include/linux/moduleparam.h | 11 +++--------
 kernel/module/main.c        |  4 ++--
 kernel/params.c             | 27 ++++++++++++++++++---------
 3 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 7d22d4c4ea2e..8667f72503d9 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -426,14 +426,9 @@ extern char *parse_args(const char *name,
 		      void *arg, parse_unknown_fn unknown);
 
 /* Called by module remove. */
-#ifdef CONFIG_SYSFS
-extern void destroy_params(const struct kernel_param *params, unsigned num);
-#else
-static inline void destroy_params(const struct kernel_param *params,
-				  unsigned num)
-{
-}
-#endif /* !CONFIG_SYSFS */
+#ifdef CONFIG_MODULES
+void module_destroy_params(const struct kernel_param *params, unsigned int num);
+#endif
 
 /* All the helper functions */
 /* The macros to do compile-time type checking stolen from Jakub
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c3ce106c70af..ef2e2130972f 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1408,7 +1408,7 @@ static void free_module(struct module *mod)
 	module_unload_free(mod);
 
 	/* Free any allocated parameters. */
-	destroy_params(mod->kp, mod->num_kp);
+	module_destroy_params(mod->kp, mod->num_kp);
 
 	if (is_livepatch_module(mod))
 		free_module_elf(mod);
@@ -3519,7 +3519,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
 	mod_sysfs_teardown(mod);
  coming_cleanup:
 	mod->state = MODULE_STATE_GOING;
-	destroy_params(mod->kp, mod->num_kp);
+	module_destroy_params(mod->kp, mod->num_kp);
 	blocking_notifier_call_chain(&module_notify_list,
 				     MODULE_STATE_GOING, mod);
 	klp_module_going(mod);
diff --git a/kernel/params.c b/kernel/params.c
index 7188a12dbe86..c6a354d54213 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -745,15 +745,6 @@ void module_param_sysfs_remove(struct module *mod)
 }
 #endif
 
-void destroy_params(const struct kernel_param *params, unsigned num)
-{
-	unsigned int i;
-
-	for (i = 0; i < num; i++)
-		if (params[i].ops->free)
-			params[i].ops->free(params[i].arg);
-}
-
 struct module_kobject * __init_or_module
 lookup_or_create_module_kobject(const char *name)
 {
@@ -985,3 +976,21 @@ static int __init param_sysfs_builtin_init(void)
 late_initcall(param_sysfs_builtin_init);
 
 #endif /* CONFIG_SYSFS */
+
+#ifdef CONFIG_MODULES
+
+/*
+ * module_destroy_params - free all parameters for one module
+ * @params: module parameters (array)
+ * @num: number of module parameters
+ */
+void module_destroy_params(const struct kernel_param *params, unsigned int num)
+{
+	unsigned int i;
+
+	for (i = 0; i < num; i++)
+		if (params[i].ops->free)
+			params[i].ops->free(params[i].arg);
+}
+
+#endif /* CONFIG_MODULES */
-- 
2.53.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 2/3] module: Clean up parse_args() arguments
  2026-03-13 13:48 [PATCH v2 0/3] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n Petr Pavlu
  2026-03-13 13:48 ` [PATCH v2 1/3] " Petr Pavlu
@ 2026-03-13 13:48 ` Petr Pavlu
  2026-03-13 13:48 ` [PATCH v2 3/3] module: Remove extern keyword from param prototypes Petr Pavlu
  2026-03-20 17:45 ` [PATCH v2 0/3] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n Sami Tolvanen
  3 siblings, 0 replies; 5+ messages in thread
From: Petr Pavlu @ 2026-03-13 13:48 UTC (permalink / raw)
  To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
  Cc: Christophe Leroy, Aaron Tomlin, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, linux-modules, linux-kernel

* Use the preferred `unsigned int` over plain `unsigned` for the `num`
  parameter.
* Synchronize the parameter names in moduleparam.h with the ones used by
  the implementation in params.c.

Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
 include/linux/moduleparam.h | 8 ++++----
 kernel/params.c             | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 8667f72503d9..604bc6e9f3a1 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -417,12 +417,12 @@ extern bool parameqn(const char *name1, const char *name2, size_t n);
 typedef int (*parse_unknown_fn)(char *param, char *val, const char *doing, void *arg);
 
 /* Called on module insert or kernel boot */
-extern char *parse_args(const char *name,
+extern char *parse_args(const char *doing,
 		      char *args,
 		      const struct kernel_param *params,
-		      unsigned num,
-		      s16 level_min,
-		      s16 level_max,
+		      unsigned int num,
+		      s16 min_level,
+		      s16 max_level,
 		      void *arg, parse_unknown_fn unknown);
 
 /* Called by module remove. */
diff --git a/kernel/params.c b/kernel/params.c
index c6a354d54213..74d620bc2521 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -161,7 +161,7 @@ static int parse_one(char *param,
 char *parse_args(const char *doing,
 		 char *args,
 		 const struct kernel_param *params,
-		 unsigned num,
+		 unsigned int num,
 		 s16 min_level,
 		 s16 max_level,
 		 void *arg, parse_unknown_fn unknown)
-- 
2.53.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 3/3] module: Remove extern keyword from param prototypes
  2026-03-13 13:48 [PATCH v2 0/3] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n Petr Pavlu
  2026-03-13 13:48 ` [PATCH v2 1/3] " Petr Pavlu
  2026-03-13 13:48 ` [PATCH v2 2/3] module: Clean up parse_args() arguments Petr Pavlu
@ 2026-03-13 13:48 ` Petr Pavlu
  2026-03-20 17:45 ` [PATCH v2 0/3] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n Sami Tolvanen
  3 siblings, 0 replies; 5+ messages in thread
From: Petr Pavlu @ 2026-03-13 13:48 UTC (permalink / raw)
  To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
  Cc: Christophe Leroy, Aaron Tomlin, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, linux-modules, linux-kernel

The external function declarations do not need the "extern" keyword. Remove
it to align with the Linux kernel coding style and to silence the
associated checkpatch warnings.

Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
 include/linux/moduleparam.h | 89 ++++++++++++++++++-------------------
 1 file changed, 44 insertions(+), 45 deletions(-)

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 604bc6e9f3a1..075f28585074 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -317,8 +317,8 @@ struct kparam_array
 			    name, &__param_ops_##name, arg, perm, -1, 0)
 
 #ifdef CONFIG_SYSFS
-extern void kernel_param_lock(struct module *mod);
-extern void kernel_param_unlock(struct module *mod);
+void kernel_param_lock(struct module *mod);
+void kernel_param_unlock(struct module *mod);
 #else
 static inline void kernel_param_lock(struct module *mod)
 {
@@ -398,7 +398,7 @@ static inline void kernel_param_unlock(struct module *mod)
  * Returns: true if the two parameter names are equal.
  * Dashes (-) are considered equal to underscores (_).
  */
-extern bool parameq(const char *name1, const char *name2);
+bool parameq(const char *name1, const char *name2);
 
 /**
  * parameqn - checks if two parameter names match
@@ -412,18 +412,18 @@ extern bool parameq(const char *name1, const char *name2);
  * are equal.
  * Dashes (-) are considered equal to underscores (_).
  */
-extern bool parameqn(const char *name1, const char *name2, size_t n);
+bool parameqn(const char *name1, const char *name2, size_t n);
 
 typedef int (*parse_unknown_fn)(char *param, char *val, const char *doing, void *arg);
 
 /* Called on module insert or kernel boot */
-extern char *parse_args(const char *doing,
-		      char *args,
-		      const struct kernel_param *params,
-		      unsigned int num,
-		      s16 min_level,
-		      s16 max_level,
-		      void *arg, parse_unknown_fn unknown);
+char *parse_args(const char *doing,
+		 char *args,
+		 const struct kernel_param *params,
+		 unsigned int num,
+		 s16 min_level,
+		 s16 max_level,
+		 void *arg, parse_unknown_fn unknown);
 
 /* Called by module remove. */
 #ifdef CONFIG_MODULES
@@ -437,78 +437,77 @@ void module_destroy_params(const struct kernel_param *params, unsigned int num);
 	static inline type __always_unused *__check_##name(void) { return(p); }
 
 extern const struct kernel_param_ops param_ops_byte;
-extern int param_set_byte(const char *val, const struct kernel_param *kp);
-extern int param_get_byte(char *buffer, const struct kernel_param *kp);
+int param_set_byte(const char *val, const struct kernel_param *kp);
+int param_get_byte(char *buffer, const struct kernel_param *kp);
 #define param_check_byte(name, p) __param_check(name, p, unsigned char)
 
 extern const struct kernel_param_ops param_ops_short;
-extern int param_set_short(const char *val, const struct kernel_param *kp);
-extern int param_get_short(char *buffer, const struct kernel_param *kp);
+int param_set_short(const char *val, const struct kernel_param *kp);
+int param_get_short(char *buffer, const struct kernel_param *kp);
 #define param_check_short(name, p) __param_check(name, p, short)
 
 extern const struct kernel_param_ops param_ops_ushort;
-extern int param_set_ushort(const char *val, const struct kernel_param *kp);
-extern int param_get_ushort(char *buffer, const struct kernel_param *kp);
+int param_set_ushort(const char *val, const struct kernel_param *kp);
+int param_get_ushort(char *buffer, const struct kernel_param *kp);
 #define param_check_ushort(name, p) __param_check(name, p, unsigned short)
 
 extern const struct kernel_param_ops param_ops_int;
-extern int param_set_int(const char *val, const struct kernel_param *kp);
-extern int param_get_int(char *buffer, const struct kernel_param *kp);
+int param_set_int(const char *val, const struct kernel_param *kp);
+int param_get_int(char *buffer, const struct kernel_param *kp);
 #define param_check_int(name, p) __param_check(name, p, int)
 
 extern const struct kernel_param_ops param_ops_uint;
-extern int param_set_uint(const char *val, const struct kernel_param *kp);
-extern int param_get_uint(char *buffer, const struct kernel_param *kp);
+int param_set_uint(const char *val, const struct kernel_param *kp);
+int param_get_uint(char *buffer, const struct kernel_param *kp);
 int param_set_uint_minmax(const char *val, const struct kernel_param *kp,
 		unsigned int min, unsigned int max);
 #define param_check_uint(name, p) __param_check(name, p, unsigned int)
 
 extern const struct kernel_param_ops param_ops_long;
-extern int param_set_long(const char *val, const struct kernel_param *kp);
-extern int param_get_long(char *buffer, const struct kernel_param *kp);
+int param_set_long(const char *val, const struct kernel_param *kp);
+int param_get_long(char *buffer, const struct kernel_param *kp);
 #define param_check_long(name, p) __param_check(name, p, long)
 
 extern const struct kernel_param_ops param_ops_ulong;
-extern int param_set_ulong(const char *val, const struct kernel_param *kp);
-extern int param_get_ulong(char *buffer, const struct kernel_param *kp);
+int param_set_ulong(const char *val, const struct kernel_param *kp);
+int param_get_ulong(char *buffer, const struct kernel_param *kp);
 #define param_check_ulong(name, p) __param_check(name, p, unsigned long)
 
 extern const struct kernel_param_ops param_ops_ullong;
-extern int param_set_ullong(const char *val, const struct kernel_param *kp);
-extern int param_get_ullong(char *buffer, const struct kernel_param *kp);
+int param_set_ullong(const char *val, const struct kernel_param *kp);
+int param_get_ullong(char *buffer, const struct kernel_param *kp);
 #define param_check_ullong(name, p) __param_check(name, p, unsigned long long)
 
 extern const struct kernel_param_ops param_ops_hexint;
-extern int param_set_hexint(const char *val, const struct kernel_param *kp);
-extern int param_get_hexint(char *buffer, const struct kernel_param *kp);
+int param_set_hexint(const char *val, const struct kernel_param *kp);
+int param_get_hexint(char *buffer, const struct kernel_param *kp);
 #define param_check_hexint(name, p) param_check_uint(name, p)
 
 extern const struct kernel_param_ops param_ops_charp;
-extern int param_set_charp(const char *val, const struct kernel_param *kp);
-extern int param_get_charp(char *buffer, const struct kernel_param *kp);
-extern void param_free_charp(void *arg);
+int param_set_charp(const char *val, const struct kernel_param *kp);
+int param_get_charp(char *buffer, const struct kernel_param *kp);
+void param_free_charp(void *arg);
 #define param_check_charp(name, p) __param_check(name, p, char *)
 
 /* We used to allow int as well as bool.  We're taking that away! */
 extern const struct kernel_param_ops param_ops_bool;
-extern int param_set_bool(const char *val, const struct kernel_param *kp);
-extern int param_get_bool(char *buffer, const struct kernel_param *kp);
+int param_set_bool(const char *val, const struct kernel_param *kp);
+int param_get_bool(char *buffer, const struct kernel_param *kp);
 #define param_check_bool(name, p) __param_check(name, p, bool)
 
 extern const struct kernel_param_ops param_ops_bool_enable_only;
-extern int param_set_bool_enable_only(const char *val,
-				      const struct kernel_param *kp);
+int param_set_bool_enable_only(const char *val, const struct kernel_param *kp);
 /* getter is the same as for the regular bool */
 #define param_check_bool_enable_only param_check_bool
 
 extern const struct kernel_param_ops param_ops_invbool;
-extern int param_set_invbool(const char *val, const struct kernel_param *kp);
-extern int param_get_invbool(char *buffer, const struct kernel_param *kp);
+int param_set_invbool(const char *val, const struct kernel_param *kp);
+int param_get_invbool(char *buffer, const struct kernel_param *kp);
 #define param_check_invbool(name, p) __param_check(name, p, bool)
 
 /* An int, which can only be set like a bool (though it shows as an int). */
 extern const struct kernel_param_ops param_ops_bint;
-extern int param_set_bint(const char *val, const struct kernel_param *kp);
+int param_set_bint(const char *val, const struct kernel_param *kp);
 #define param_get_bint param_get_int
 #define param_check_bint param_check_int
 
@@ -615,19 +614,19 @@ enum hwparam_type {
 extern const struct kernel_param_ops param_array_ops;
 
 extern const struct kernel_param_ops param_ops_string;
-extern int param_set_copystring(const char *val, const struct kernel_param *);
-extern int param_get_string(char *buffer, const struct kernel_param *kp);
+int param_set_copystring(const char *val, const struct kernel_param *kp);
+int param_get_string(char *buffer, const struct kernel_param *kp);
 
 /* for exporting parameters in /sys/module/.../parameters */
 
 struct module;
 
 #if defined(CONFIG_SYSFS) && defined(CONFIG_MODULES)
-extern int module_param_sysfs_setup(struct module *mod,
-				    const struct kernel_param *kparam,
-				    unsigned int num_params);
+int module_param_sysfs_setup(struct module *mod,
+			     const struct kernel_param *kparam,
+			     unsigned int num_params);
 
-extern void module_param_sysfs_remove(struct module *mod);
+void module_param_sysfs_remove(struct module *mod);
 #else
 static inline int module_param_sysfs_setup(struct module *mod,
 			     const struct kernel_param *kparam,
-- 
2.53.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 0/3] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n
  2026-03-13 13:48 [PATCH v2 0/3] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n Petr Pavlu
                   ` (2 preceding siblings ...)
  2026-03-13 13:48 ` [PATCH v2 3/3] module: Remove extern keyword from param prototypes Petr Pavlu
@ 2026-03-20 17:45 ` Sami Tolvanen
  3 siblings, 0 replies; 5+ messages in thread
From: Sami Tolvanen @ 2026-03-20 17:45 UTC (permalink / raw)
  To: Luis Chamberlain, Daniel Gomez, Petr Pavlu
  Cc: Christophe Leroy, Aaron Tomlin, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, linux-modules, linux-kernel

On Fri, 13 Mar 2026 14:48:01 +0100, Petr Pavlu wrote:
> Fix freeing of charp module parameters when CONFIG_SYSFS=n and, related to
> this, update moduleparam.h to keep its coding style consistent.
> 
> Changes since v1 [1]:
> * Remove the extern keyword from the declaration of module_destroy_params()
>   and update the type of its num parameter from `unsigned` to
>   `unsigned int`.
> * Add a cleanup patch for parse_args() to similarly update its num
>   parameter to `unsigned int` and to synchronize the parameter names
>   between its prototype and definition.
> * Add a cleanup patch to drop the unnecessary extern keyword for all
>   function declarations in moduleparam.h.
> 
> [...]

Applied to modules-next, thanks!

[1/3] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n
      commit: deffe1edba626d474fef38007c03646ca5876a0e
[2/3] module: Clean up parse_args() arguments
      commit: 65f535501e2a3378629b8650eca553920de5e5a2
[3/3] module: Remove extern keyword from param prototypes
      commit: 44a063c00fb13cf1f2e8a53a2ab10b232a44954b

Best regards,

	Sami



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-03-20 17:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-13 13:48 [PATCH v2 0/3] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n Petr Pavlu
2026-03-13 13:48 ` [PATCH v2 1/3] " Petr Pavlu
2026-03-13 13:48 ` [PATCH v2 2/3] module: Clean up parse_args() arguments Petr Pavlu
2026-03-13 13:48 ` [PATCH v2 3/3] module: Remove extern keyword from param prototypes Petr Pavlu
2026-03-20 17:45 ` [PATCH v2 0/3] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n Sami Tolvanen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®