* [PATCH] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n
@ 2026-03-06 11:10 Petr Pavlu
2026-03-09 11:26 ` Christophe Leroy (CS GROUP)
0 siblings, 1 reply; 3+ messages in thread
From: Petr Pavlu @ 2026-03-06 11:10 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: 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 | 12 ++++--------
kernel/module/main.c | 4 ++--
kernel/params.c | 27 ++++++++++++++++++---------
3 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 7d22d4c4ea2e..6283665ec614 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -426,14 +426,10 @@ 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
+extern void module_destroy_params(const struct kernel_param *params,
+ unsigned 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..1a436c9d6140 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 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 */
base-commit: c107785c7e8dbabd1c18301a1c362544b5786282
--
2.53.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n
2026-03-06 11:10 [PATCH] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n Petr Pavlu
@ 2026-03-09 11:26 ` Christophe Leroy (CS GROUP)
2026-03-09 14:20 ` Petr Pavlu
0 siblings, 1 reply; 3+ messages in thread
From: Christophe Leroy (CS GROUP) @ 2026-03-09 11:26 UTC (permalink / raw)
To: Petr Pavlu, Luis Chamberlain, Daniel Gomez, Sami Tolvanen
Cc: Aaron Tomlin, Greg Kroah-Hartman, Rafael J. Wysocki,
Danilo Krummrich, linux-modules, linux-kernel
Le 06/03/2026 à 12:10, Petr Pavlu a écrit :
> 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 | 12 ++++--------
> kernel/module/main.c | 4 ++--
> kernel/params.c | 27 ++++++++++++++++++---------
> 3 files changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index 7d22d4c4ea2e..6283665ec614 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -426,14 +426,10 @@ 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
> +extern void module_destroy_params(const struct kernel_param *params,
> + unsigned num);
'extern' is pointless for function prototypes, don't add new ones.
num has no type.
> +#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..1a436c9d6140 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 num)
num has no type
> +{
> + unsigned int i;
> +
> + for (i = 0; i < num; i++)
> + if (params[i].ops->free)
> + params[i].ops->free(params[i].arg);
> +}
> +
> +#endif /* CONFIG_MODULES */
>
> base-commit: c107785c7e8dbabd1c18301a1c362544b5786282
Note, checkpatch reports the same:
CHECK: extern prototypes should be avoided in .h files
#47: FILE: include/linux/moduleparam.h:430:
+extern void module_destroy_params(const struct kernel_param *params,
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#48: FILE: include/linux/moduleparam.h:431:
+ unsigned num);
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#107: FILE: kernel/params.c:987:
+void module_destroy_params(const struct kernel_param *params, unsigned num)
total: 0 errors, 2 warnings, 1 checks, 70 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or
--fix-inplace.
Commit 4aad08ba007a ("module: Fix freeing of charp module parameters
when CONFIG_SYSFS=n") has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
Christophe
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n
2026-03-09 11:26 ` Christophe Leroy (CS GROUP)
@ 2026-03-09 14:20 ` Petr Pavlu
0 siblings, 0 replies; 3+ messages in thread
From: Petr Pavlu @ 2026-03-09 14:20 UTC (permalink / raw)
To: Christophe Leroy (CS GROUP)
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, Aaron Tomlin,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
linux-modules, linux-kernel
On 3/9/26 12:26 PM, Christophe Leroy (CS GROUP) wrote:
> Le 06/03/2026 à 12:10, Petr Pavlu a écrit :
>> 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 | 12 ++++--------
>> kernel/module/main.c | 4 ++--
>> kernel/params.c | 27 ++++++++++++++++++---------
>> 3 files changed, 24 insertions(+), 19 deletions(-)
>>
>> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
>> index 7d22d4c4ea2e..6283665ec614 100644
>> --- a/include/linux/moduleparam.h
>> +++ b/include/linux/moduleparam.h
>> @@ -426,14 +426,10 @@ 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
>> +extern void module_destroy_params(const struct kernel_param *params,
>> + unsigned num);
>
> 'extern' is pointless for function prototypes, don't add new ones.
>
> num has no type.
>
>> +#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..1a436c9d6140 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 num)
>
> num has no type
>
>> +{
>> + unsigned int i;
>> +
>> + for (i = 0; i < num; i++)
>> + if (params[i].ops->free)
>> + params[i].ops->free(params[i].arg);
>> +}
>> +
>> +#endif /* CONFIG_MODULES */
>>
>> base-commit: c107785c7e8dbabd1c18301a1c362544b5786282
>
> Note, checkpatch reports the same:
>
> CHECK: extern prototypes should be avoided in .h files
> #47: FILE: include/linux/moduleparam.h:430:
> +extern void module_destroy_params(const struct kernel_param *params,
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #48: FILE: include/linux/moduleparam.h:431:
> + unsigned num);
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #107: FILE: kernel/params.c:987:
> +void module_destroy_params(const struct kernel_param *params, unsigned num)
>
> total: 0 errors, 2 warnings, 1 checks, 70 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
> mechanically convert to the typical style using --fix or --fix-inplace.
>
> Commit 4aad08ba007a ("module: Fix freeing of charp module parameters when CONFIG_SYSFS=n") has style problems, please review.
>
> NOTE: If any of the errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
I intentionally kept the `extern` keyword and the `unsigned` parameter
to maintain consistency with the surrounding style in moduleparam.h.
I'll send a v2 to correct this in my patch. Additionally, I can include
cleanup patches for moduleparam.h to remove the unnecessary `extern` and
change `unsigned` to `unsigned int` in other occurrences, so the content
of the file remains consistent in this regard.
--
Thanks,
Petr
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-03-09 14:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-06 11:10 [PATCH] module: Fix freeing of charp module parameters when CONFIG_SYSFS=n Petr Pavlu
2026-03-09 11:26 ` Christophe Leroy (CS GROUP)
2026-03-09 14:20 ` Petr Pavlu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox