| 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
| ||
|
Message-ID: <CAJfuBxx5q_jyDgH+dvJZSxwus5wxxo-4q6r0Mt1N49VLz96T4A@mail.gmail.com>
Date: Fri, 23 Jan 2026 16:10:34 -0700
From: jim.cromie@...il.com
To: "Christophe Leroy (CS GROUP)" <chleroy@...nel.org>
Cc: linux-kernel@...r.kernel.org, Luis Chamberlain <mcgrof@...nel.org>,
Petr Pavlu <petr.pavlu@...e.com>, Daniel Gomez <da.gomez@...nel.org>,
Sami Tolvanen <samitolvanen@...gle.com>, Aaron Tomlin <atomlin@...mlin.com>,
linux-modules@...r.kernel.org
Subject: Re: [RFC PATCH 1/1] module: speed modprobe by adding name_crc to
struct module
On Fri, Jan 23, 2026 at 4:39 AM Christophe Leroy (CS GROUP)
<chleroy@...nel.org> wrote:
>
>
>
> Le 23/01/2026 à 00:46, Jim Cromie a écrit :
> > [Vous ne recevez pas souvent de courriers de jim.cromie@...il.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> >
> > "modprobe foo" currently does strcmp on the name, this can be improved.
> >
> > So this commit:
> >
> > 1. adds name_crc to struct module
> > 2. modpost.c computes the value and
> > 3. outputs it for "modinfo foo" to see/use.
> >
> > 4. adds hotpath to find_module_all()
> > this uses name_crc to do quick "name-check"
> > falls back to strcmp only to guard against collisions.
> >
> > This should significantly reduce modprobe workload, and shorten module
> > load-time.
>
> Any numbers of how significant is the reduction ?
Not at this time.
In absolute terms, my box is running fedora, has 165 modules,
so the significance is minor in most circumstances.
In relative terms, a numerical equality test is much faster than strcmp.
I recall seeing Luis doing quite a bit of work tuning module loading,
I was hoping he could shed light / opine / provide numbers
on the utility of this.
>
> >
> > Since it alters struct module, its binary incompatible. This means:
> >
> > 1. RFC for its wide "blast radius".
> > 2. suitable for major version bump *only*
> >
> > 3. it opens door for further struct module reorg, to:
> > a. segregate fields by "temperature"
> > b. pack out paholes.
> > c. improve cache locality (by reordering coldest on bottom)
> > name should be cold now.
> > bikeshedding is appropriate here.
> >
> > NB: this isn't a substitute for CONFIG_MODULE_SIG.
> > It reimplements crc_le(), doesn't reuse kernel's version.
>
> Why not use the kernel's version ?
I briefly fiddled with doing so, but sorting the includes looked like a hassle,
so I punted and asked the AI for an implementation.
>
> >
> > CC: Luis Chamberlain <mcgrof@...nel.org>
> > CC: Petr Pavlu <petr.pavlu@...e.com>
> > CC: Daniel Gomez <da.gomez@...nel.org>
> > CC: Sami Tolvanen <samitolvanen@...gle.com>
> > CC: Aaron Tomlin <atomlin@...mlin.com>
> > CC: linux-modules@...r.kernel.org
> >
> > Signed-off-by: Jim Cromie <jim.cromie@...il.com>
> >
> > '#' will be ignored, and an empty message aborts the commit.
> > ---
> > include/linux/module.h | 15 ++++++++-------
> > kernel/module/main.c | 8 ++++++--
> > scripts/mod/modpost.c | 18 ++++++++++++++++++
> > scripts/mod/modpost.h | 6 +++++-
> > 4 files changed, 37 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index d80c3ea57472..4ea6c5ae3374 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -402,10 +402,18 @@ struct klp_modinfo {
> >
> > struct module {
> > enum module_state state;
> > + u32 name_hash;
>
> In the subject you say "name_crc"
>
> >
> > /* Member of list of modules */
> > struct list_head list;
> >
> > + /* Sysfs stuff. */
> > + struct module_kobject mkobj;
> > + struct module_attribute *modinfo_attrs;
> > + const char *version;
> > + const char *srcversion;
> > + struct kobject *holders_dir;
> > +
>
> Shouldn't this move be another patch ?
>
> > /* Unique handle for this module */
> > char name[MODULE_NAME_LEN];
> >
> > @@ -414,13 +422,6 @@ struct module {
> > unsigned char build_id[BUILD_ID_SIZE_MAX];
> > #endif
> >
> > - /* Sysfs stuff. */
> > - struct module_kobject mkobj;
> > - struct module_attribute *modinfo_attrs;
> > - const char *version;
> > - const char *srcversion;
> > - struct kobject *holders_dir;
> > -
> > /* Exported symbols */
> > const struct kernel_symbol *syms;
> > const u32 *crcs;
> > diff --git a/kernel/module/main.c b/kernel/module/main.c
> > index d855f43a2be3..685218b2c5ef 100644
> > --- a/kernel/module/main.c
> > +++ b/kernel/module/main.c
> > @@ -39,6 +39,7 @@
> > #include <linux/mutex.h>
> > #include <linux/rculist.h>
> > #include <linux/uaccess.h>
> > +#include <linux/crc32.h>
> > #include <asm/cacheflush.h>
> > #include <linux/set_memory.h>
> > #include <asm/mmu_context.h>
> > @@ -431,13 +432,16 @@ struct module *find_module_all(const char *name, size_t len,
> > bool even_unformed)
> > {
> > struct module *mod;
> > + u32 incoming_name_hash = crc32_le(0, name, len);
> >
> > list_for_each_entry_rcu(mod, &modules, list,
> > lockdep_is_held(&module_mutex)) {
> > if (!even_unformed && mod->state == MODULE_STATE_UNFORMED)
> > continue;
> > - if (strlen(mod->name) == len && !memcmp(mod->name, name, len))
> > - return mod;
> > + if (mod->name_hash == incoming_name_hash) {
> > + if (strlen(mod->name) == len && !memcmp(mod->name, name, len))
> > + return mod;
> > + }
>
> Why not just adding the following instead of modifing existing test:
>
> if (mod->name_hash != incoming_name_hash)
> continue;
>
> > }
> > return NULL;
> > }
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index 755b842f1f9b..ae90e0bf9330 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -21,6 +21,22 @@
> > #include <stdbool.h>
> > #include <errno.h>
> >
> > +/* Local CRC32 implementation for modpost.c */
> > +#define CRCPOLY_LE 0xEDB88320
> > +
> > +typedef uint32_t u32;
> > +
> > +static u32 crc32_le(u32 crc, char *p, size_t len)
> > +{
> > + int i;
> > + while (len--) {
> > + crc ^= *p++;
> > + for (i = 0; i < 8; i++)
> > + crc = (crc >> 1) ^ ((crc & 1) ? CRCPOLY_LE : 0);
> > + }
> > + return crc;
> > +}
> > +
>
> Why do you re-implement crc32_le() ?
>
> > #include <hash.h>
> > #include <hashtable.h>
> > #include <list.h>
> > @@ -1581,6 +1597,7 @@ static void read_symbols(const char *modname)
> >
> > /* strip trailing .o */
> > mod = new_module(modname, strlen(modname) - strlen(".o"));
> > + mod->name_hash = crc32_le(0, mod->name, strlen(mod->name));
> >
> > /* save .no_trim_symbol section for later use */
> > if (info.no_trim_symbol_len) {
> > @@ -1834,6 +1851,7 @@ static void add_header(struct buffer *b, struct module *mod)
> > buf_printf(b, "#include <linux/compiler.h>\n");
> > buf_printf(b, "\n");
> > buf_printf(b, "MODULE_INFO(name, KBUILD_MODNAME);\n");
> > + buf_printf(b, "MODULE_INFO(name_crc, \"0x%08x\");\n", mod->name_hash);
> > buf_printf(b, "\n");
> > buf_printf(b, "__visible struct module __this_module\n");
> > buf_printf(b, "__section(\".gnu.linkonce.this_module\") = {\n");
> > diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> > index 2aecb8f25c87..3fc3cfd0a039 100644
> > --- a/scripts/mod/modpost.h
> > +++ b/scripts/mod/modpost.h
> > @@ -11,11 +11,14 @@
> > #include <fcntl.h>
> > #include <unistd.h>
> > #include <elf.h>
> > +#include <stdint.h>
> > #include "../../include/linux/module_symbol.h"
> >
> > #include <list_types.h>
> > #include "elfconfig.h"
> >
> > +typedef uint32_t u32;
> > +
> > /* On BSD-alike OSes elf.h defines these according to host's word size */
> > #undef ELF_ST_BIND
> > #undef ELF_ST_TYPE
> > @@ -126,7 +129,8 @@ struct module {
> > bool seen;
> > bool has_init;
> > bool has_cleanup;
> > - char srcversion[25];
> > + char srcversion[25];
> > + u32 name_hash;
> > // Missing namespace dependencies
> > struct list_head missing_namespaces;
> > // Actual imported namespaces
> > --
> > 2.52.0
> >
> >
>
Powered by blists - more mailing lists