linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kbuild: stop module name mangling
@ 2025-06-02 13:06 Masahiro Yamada
  2025-06-02 19:00 ` Masahiro Yamada
  0 siblings, 1 reply; 3+ messages in thread
From: Masahiro Yamada @ 2025-06-02 13:06 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Peter Zijlstra, Lucas De Marchi, Masahiro Yamada,
	Daniel Gomez, Luis Chamberlain, Nathan Chancellor, Nicolas Schier,
	Petr Pavlu, Sami Tolvanen, linux-modules

In the old days, KBUILD_MODNAME was passed to C without double quotes,
and then handled by __stringify() on the C side. This was the reason
why KBUILD_MODNAME was mangled: characters such as commas (,) and
hyphens (-) are not allowed in C identifiers, so they were replaced
with underscores (_) in Kbuild.

Since commit f83b5e323f57 ("kbuild: set correct KBUILD_MODNAME when
using well known kernel symbols as module names"), KBUILD_MODNAME has
been passed to C as a string literal, which allows any characters.

Aside from this historical behavior in the build system, there is no
longer a reason for mangling. In fact, it is rather annoying, as we
now need to convert between hyphens and underscores in some places,
but not in others. See commit 0267cbf297bf ("module: Account for the
build time module name mangling").

This commit eliminates that oddity, so the module name will now match
the filename. For example, the module name of "foo-bar.ko" will be
"foo-bar", not "foo_bar".

However, this oddity persisted for so long and also affected the
userspace. To adapt to this behavior, when a user runs "rmmod foo-bar",
kmod converts hyphens to underscores, and passes "foo_bar" to the
delete_module syscall.

Therefore, the mod_strncmp() needs to remain in find_module_all(),
otherwise, we cannot unload modules.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 kernel/module/main.c | 8 ++++++--
 scripts/Makefile.lib | 4 ++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index b8440b0887e3..1fa90a95e0c5 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -410,7 +410,11 @@ struct module *find_module_all(const char *name, size_t len,
 				lockdep_is_held(&module_mutex)) {
 		if (!even_unformed && mod->state == MODULE_STATE_UNFORMED)
 			continue;
-		if (strlen(mod->name) == len && !memcmp(mod->name, name, len))
+		/*
+		 * For historical reasons, kmod passes a module name with
+		 * a hyphen replaced with an underscore.
+		 */
+		if (!mod_strncmp(mod->name, name, len))
 			return mod;
 	}
 	return NULL;
@@ -1135,7 +1139,7 @@ static bool module_match(const char *modname, const char *patterns)
 		if (*sep)
 			sep++;
 
-		if (mod_strncmp(patterns, modname, len) == 0 && (glob || len == modlen))
+		if (strncmp(patterns, modname, len) == 0 && (glob || len == modlen))
 			return true;
 	}
 
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 336fb0d763c7..e37e2db5f528 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -18,8 +18,8 @@ target-stem = $(basename $(patsubst $(obj)/%,%,$@))
 # end up in (or would, if it gets compiled in)
 name-fix-token = $(subst $(comma),_,$(subst -,_,$1))
 name-fix = $(call stringify,$(call name-fix-token,$1))
-basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget))
-modname_flags  = -DKBUILD_MODNAME=$(call name-fix,$(modname)) \
+basename_flags = -DKBUILD_BASENAME=$(call stringify,$(basetarget))
+modname_flags  = -DKBUILD_MODNAME=$(call stringify,$(modname)) \
 		 -D__KBUILD_MODNAME=kmod_$(call name-fix-token,$(modname))
 modfile_flags  = -DKBUILD_MODFILE=$(call stringify,$(modfile))
 
-- 
2.43.0


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

* Re: [PATCH] kbuild: stop module name mangling
  2025-06-02 13:06 [PATCH] kbuild: stop module name mangling Masahiro Yamada
@ 2025-06-02 19:00 ` Masahiro Yamada
  2025-06-02 19:35   ` Lucas De Marchi
  0 siblings, 1 reply; 3+ messages in thread
From: Masahiro Yamada @ 2025-06-02 19:00 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Peter Zijlstra, Lucas De Marchi, Daniel Gomez,
	Luis Chamberlain, Nathan Chancellor, Nicolas Schier, Petr Pavlu,
	Sami Tolvanen, linux-modules

On Mon, Jun 2, 2025 at 10:06 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> In the old days, KBUILD_MODNAME was passed to C without double quotes,
> and then handled by __stringify() on the C side. This was the reason
> why KBUILD_MODNAME was mangled: characters such as commas (,) and
> hyphens (-) are not allowed in C identifiers, so they were replaced
> with underscores (_) in Kbuild.
>
> Since commit f83b5e323f57 ("kbuild: set correct KBUILD_MODNAME when
> using well known kernel symbols as module names"), KBUILD_MODNAME has
> been passed to C as a string literal, which allows any characters.
>
> Aside from this historical behavior in the build system, there is no
> longer a reason for mangling. In fact, it is rather annoying, as we
> now need to convert between hyphens and underscores in some places,
> but not in others. See commit 0267cbf297bf ("module: Account for the
> build time module name mangling").
>
> This commit eliminates that oddity, so the module name will now match
> the filename. For example, the module name of "foo-bar.ko" will be
> "foo-bar", not "foo_bar".
>
> However, this oddity persisted for so long and also affected the
> userspace. To adapt to this behavior, when a user runs "rmmod foo-bar",
> kmod converts hyphens to underscores, and passes "foo_bar" to the
> delete_module syscall.

Hmm. That was modprobe/rmmod from busybox.

kmod tries to open /sys/module/*, and
hyphen/underscore conversion happens everywhere.

libkmod: ERROR ../libkmod/libkmod-module.c:2039
kmod_module_get_holders: could not open '/sys/module/fo_o/holders': No
such file or directory

So, we may need to carry this forever...



>
> Therefore, the mod_strncmp() needs to remain in find_module_all(),
> otherwise, we cannot unload modules.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  kernel/module/main.c | 8 ++++++--
>  scripts/Makefile.lib | 4 ++--
>  2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index b8440b0887e3..1fa90a95e0c5 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -410,7 +410,11 @@ struct module *find_module_all(const char *name, size_t len,
>                                 lockdep_is_held(&module_mutex)) {
>                 if (!even_unformed && mod->state == MODULE_STATE_UNFORMED)
>                         continue;
> -               if (strlen(mod->name) == len && !memcmp(mod->name, name, len))
> +               /*
> +                * For historical reasons, kmod passes a module name with
> +                * a hyphen replaced with an underscore.
> +                */
> +               if (!mod_strncmp(mod->name, name, len))
>                         return mod;
>         }
>         return NULL;
> @@ -1135,7 +1139,7 @@ static bool module_match(const char *modname, const char *patterns)
>                 if (*sep)
>                         sep++;
>
> -               if (mod_strncmp(patterns, modname, len) == 0 && (glob || len == modlen))
> +               if (strncmp(patterns, modname, len) == 0 && (glob || len == modlen))
>                         return true;
>         }
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 336fb0d763c7..e37e2db5f528 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -18,8 +18,8 @@ target-stem = $(basename $(patsubst $(obj)/%,%,$@))
>  # end up in (or would, if it gets compiled in)
>  name-fix-token = $(subst $(comma),_,$(subst -,_,$1))
>  name-fix = $(call stringify,$(call name-fix-token,$1))
> -basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget))
> -modname_flags  = -DKBUILD_MODNAME=$(call name-fix,$(modname)) \
> +basename_flags = -DKBUILD_BASENAME=$(call stringify,$(basetarget))
> +modname_flags  = -DKBUILD_MODNAME=$(call stringify,$(modname)) \
>                  -D__KBUILD_MODNAME=kmod_$(call name-fix-token,$(modname))
>  modfile_flags  = -DKBUILD_MODFILE=$(call stringify,$(modfile))
>
> --
> 2.43.0
>


-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH] kbuild: stop module name mangling
  2025-06-02 19:00 ` Masahiro Yamada
@ 2025-06-02 19:35   ` Lucas De Marchi
  0 siblings, 0 replies; 3+ messages in thread
From: Lucas De Marchi @ 2025-06-02 19:35 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, linux-kernel, Peter Zijlstra, Lucas De Marchi,
	Daniel Gomez, Luis Chamberlain, Nathan Chancellor, Nicolas Schier,
	Petr Pavlu, Sami Tolvanen, linux-modules

On Tue, Jun 03, 2025 at 04:00:30AM +0900, Masahiro Yamada wrote:
>On Mon, Jun 2, 2025 at 10:06 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>>
>> In the old days, KBUILD_MODNAME was passed to C without double quotes,
>> and then handled by __stringify() on the C side. This was the reason
>> why KBUILD_MODNAME was mangled: characters such as commas (,) and
>> hyphens (-) are not allowed in C identifiers, so they were replaced
>> with underscores (_) in Kbuild.
>>
>> Since commit f83b5e323f57 ("kbuild: set correct KBUILD_MODNAME when
>> using well known kernel symbols as module names"), KBUILD_MODNAME has
>> been passed to C as a string literal, which allows any characters.
>>
>> Aside from this historical behavior in the build system, there is no
>> longer a reason for mangling. In fact, it is rather annoying, as we
>> now need to convert between hyphens and underscores in some places,
>> but not in others. See commit 0267cbf297bf ("module: Account for the
>> build time module name mangling").
>>
>> This commit eliminates that oddity, so the module name will now match
>> the filename. For example, the module name of "foo-bar.ko" will be
>> "foo-bar", not "foo_bar".
>>
>> However, this oddity persisted for so long and also affected the
>> userspace. To adapt to this behavior, when a user runs "rmmod foo-bar",
>> kmod converts hyphens to underscores, and passes "foo_bar" to the
>> delete_module syscall.
>
>Hmm. That was modprobe/rmmod from busybox.
>
>kmod tries to open /sys/module/*, and
>hyphen/underscore conversion happens everywhere.
>
>libkmod: ERROR ../libkmod/libkmod-module.c:2039
>kmod_module_get_holders: could not open '/sys/module/fo_o/holders': No
>such file or directory

the dash to underscore is done everywhere in kmod (and previously in
module-init-tools), otherwise we couldn't reliably compare modules by
name, open the right sysfs file, etc.

$ git grep underscore
libkmod/libkmod-config.c:               if (underscores(modname) < 0) {
libkmod/libkmod-config.c:                       if (underscores(alias) < 0 || underscores(modname) < 0)
libkmod/libkmod-config.c:                       if (underscores(modname) < 0)
libkmod/libkmod-config.c:                       if (underscores(modname) < 0 || options == NULL)
libkmod/libkmod-config.c:                       if (underscores(modname) < 0 || installcmd == NULL)
libkmod/libkmod-config.c:                       if (underscores(modname) < 0 || removecmd == NULL)
libkmod/libkmod-config.c:                       if (underscores(modname) < 0 || softdeps == NULL)
libkmod/libkmod-config.c:                       if (underscores(modname) < 0 || weakdeps == NULL)
libkmod/libkmod.h: * it's always normalized (dashes are replaced with underscores).
man/modprobe.8.scd:(automatic underscore conversion is performed). *modprobe* looks in the module
man/modprobe.d.5.scd:them: both are interchangeable throughout all the module commands as underscore
shared/util.c: * Replace dashes with underscores.
shared/util.c:int underscores(char *s)
shared/util.h:_must_check_ int underscores(char *s);
testsuite/test-util.c:static int test_underscores(const struct test *t)
testsuite/test-util.c:          assert_return(!underscores(val), EXIT_FAILURE);
testsuite/test-util.c:DEFINE_TEST(test_underscores, .description = "check implementation of underscores()");

I think it's reasonable to apply
https://lore.kernel.org/linux-modules/vqeq3ioklrgrf227zgdfho4virh74qrt5reoyptmzgktyronbr@c2mw32pqikft/
otherwise there's also an implicit conversion needed for '/', probably
moving string_is_vfs_ready() to kernel/module/

Lucas De Marchi

>
>So, we may need to carry this forever...
>
>
>
>>
>> Therefore, the mod_strncmp() needs to remain in find_module_all(),
>> otherwise, we cannot unload modules.
>>
>> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>> ---
>>
>>  kernel/module/main.c | 8 ++++++--
>>  scripts/Makefile.lib | 4 ++--
>>  2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/module/main.c b/kernel/module/main.c
>> index b8440b0887e3..1fa90a95e0c5 100644
>> --- a/kernel/module/main.c
>> +++ b/kernel/module/main.c
>> @@ -410,7 +410,11 @@ struct module *find_module_all(const char *name, size_t len,
>>                                 lockdep_is_held(&module_mutex)) {
>>                 if (!even_unformed && mod->state == MODULE_STATE_UNFORMED)
>>                         continue;
>> -               if (strlen(mod->name) == len && !memcmp(mod->name, name, len))
>> +               /*
>> +                * For historical reasons, kmod passes a module name with
>> +                * a hyphen replaced with an underscore.
>> +                */
>> +               if (!mod_strncmp(mod->name, name, len))
>>                         return mod;
>>         }
>>         return NULL;
>> @@ -1135,7 +1139,7 @@ static bool module_match(const char *modname, const char *patterns)
>>                 if (*sep)
>>                         sep++;
>>
>> -               if (mod_strncmp(patterns, modname, len) == 0 && (glob || len == modlen))
>> +               if (strncmp(patterns, modname, len) == 0 && (glob || len == modlen))
>>                         return true;
>>         }
>>
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index 336fb0d763c7..e37e2db5f528 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -18,8 +18,8 @@ target-stem = $(basename $(patsubst $(obj)/%,%,$@))
>>  # end up in (or would, if it gets compiled in)
>>  name-fix-token = $(subst $(comma),_,$(subst -,_,$1))
>>  name-fix = $(call stringify,$(call name-fix-token,$1))
>> -basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget))
>> -modname_flags  = -DKBUILD_MODNAME=$(call name-fix,$(modname)) \
>> +basename_flags = -DKBUILD_BASENAME=$(call stringify,$(basetarget))
>> +modname_flags  = -DKBUILD_MODNAME=$(call stringify,$(modname)) \
>>                  -D__KBUILD_MODNAME=kmod_$(call name-fix-token,$(modname))
>>  modfile_flags  = -DKBUILD_MODFILE=$(call stringify,$(modfile))
>>
>> --
>> 2.43.0
>>
>
>
>-- 
>Best Regards
>Masahiro Yamada

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

end of thread, other threads:[~2025-06-02 19:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-02 13:06 [PATCH] kbuild: stop module name mangling Masahiro Yamada
2025-06-02 19:00 ` Masahiro Yamada
2025-06-02 19:35   ` Lucas De Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).