| 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: <6277616a-e66d-4ed1-b5d2-270f95d6eacc@kernel.org>
Date: Fri, 23 Jan 2026 12:39:06 +0100
From: "Christophe Leroy (CS GROUP)" <chleroy@...nel.org>
To: Jim Cromie <jim.cromie@...il.com>, linux-kernel@...r.kernel.org
Cc: 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
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 ?
>
> 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 ?
>
> 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