Re: [PATCH] kbuild: stop module name mangling

[Lucas De Marchi]

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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> >
> >  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