* Re: Only rust/bindings.o build fail on rust-1.91.0
From: Miguel Ojeda @ 2025-11-08 5:10 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Haiyue Wang, rust-for-linux, Miguel Ojeda, linux-modules,
Linux Kbuild mailing list
In-Reply-To: <CABCJKue1xeY7CGHScG04PdHT6TOPgsOpZVKWwgzO65qwAcbEJg@mail.gmail.com>
On Sat, Nov 8, 2025 at 3:40 AM Sami Tolvanen <samitolvanen@google.com> wrote:
>
> I'm not sure it makes sense to silently export unusable symbols. I
> feel like we should either ensure there's debugging information for
> these symbols, or just not export them.
(Now that I saw your other reply and things are clearer, replying here
as well for completeness)
No, I didn't mean to silently export, but rather to avoid failing due
to missing DWARF if there is nothing to do for that object file
because there are no exports. That is, what we manually do now with
some of the `skip_gendwarfksyms`, but dynamically, which unties us
from the inlining decisions of `rustc` which could hit us in e.g.
small crates in the future or edge cases like the bindings one.
Cheers,
Miguel
^ permalink raw reply
* RE: [PATCH v7 8/8] modsign: Enable ML-DSA module signing
From: Elliott, Robert (Servers) @ 2025-11-09 19:42 UTC (permalink / raw)
To: David Howells
Cc: Herbert Xu, Eric Biggers, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Jason A . Donenfeld, Ard Biesheuvel,
Stephan Mueller, Lukas Wunner, Ignat Korchagin,
linux-crypto@vger.kernel.org, keyrings@vger.kernel.org,
linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <61637.1762509938@warthog.procyon.org.uk>
> -----Original Message-----
> From: David Howells <dhowells@redhat.com>
> Sent: Friday, November 7, 2025 4:06 AM
> Cc: dhowells@redhat.com; Herbert Xu <herbert@gondor.apana.org.au>; Eric
> Biggers <ebiggers@kernel.org>; Luis Chamberlain <mcgrof@kernel.org>;
> Petr Pavlu <petr.pavlu@suse.com>; Daniel Gomez <da.gomez@kernel.org>;
> Sami Tolvanen <samitolvanen@google.com>; Jason A . Donenfeld
> <Jason@zx2c4.com>; Ard Biesheuvel <ardb@kernel.org>; Stephan Mueller
> <smueller@chronox.de>; Lukas Wunner <lukas@wunner.de>; Ignat Korchagin
> <ignat@cloudflare.com>; linux-crypto@vger.kernel.org;
> keyrings@vger.kernel.org; linux-modules@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: [PATCH v7 8/8] modsign: Enable ML-DSA module signing
>
>
> Allow ML-DSA module signing to be enabled.
>
> Note that openssl's CMS_*() function suite does not, as of openssl-
> 3.5.1,
> support the use of CMS_NOATTR with ML-DSA, so the prohibition against
> using
> authenticatedAttributes with module signing has to be removed. The
> selected
> digest then applies only to the algorithm used to calculate the digest
> stored in the messageDigest attribute.
>
> The ML-DSA algorithm uses its own internal choice of digest (SHAKE256)
> without regard to what's specified in the CMS message. This is, in
> theory,
> configurable, but there's currently no hook in the crypto_sig API to do
> that, though possibly it could be done by parameterising the name of
> the
> algorithm, e.g. ("ml-dsa87(sha512)").
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Lukas Wunner <lukas@wunner.de>
> cc: Ignat Korchagin <ignat@cloudflare.com>
> cc: Stephan Mueller <smueller@chronox.de>
> cc: Eric Biggers <ebiggers@kernel.org>
> cc: Herbert Xu <herbert@gondor.apana.org.au>
> cc: keyrings@vger.kernel.org
> cc: linux-crypto@vger.kernel.org
> ---
> Documentation/admin-guide/module-signing.rst | 15 ++++++++-------
> certs/Kconfig | 24
> ++++++++++++++++++++++++
> certs/Makefile | 3 +++
> crypto/asymmetric_keys/pkcs7_verify.c | 4 ----
> kernel/module/Kconfig | 5 +++++
> scripts/sign-file.c | 26
> ++++++++++++++++++--------
> 6 files changed, 58 insertions(+), 19 deletions(-)
>
> diff --git a/Documentation/admin-guide/module-signing.rst
> b/Documentation/admin-guide/module-signing.rst
> index a8667a777490..6daff80c277b 100644
> --- a/Documentation/admin-guide/module-signing.rst
> +++ b/Documentation/admin-guide/module-signing.rst
> @@ -28,10 +28,11 @@ trusted userspace bits.
>
> This facility uses X.509 ITU-T standard certificates to encode the
> public keys
> involved. The signatures are not themselves encoded in any industrial
> standard
> -type. The built-in facility currently only supports the RSA & NIST P-
> 384 ECDSA
> -public key signing standard (though it is pluggable and permits others
> to be
> -used). The possible hash algorithms that can be used are SHA-2 and
> SHA-3 of
> -sizes 256, 384, and 512 (the algorithm is selected by data in the
> signature).
> +type. The built-in facility currently only supports the RSA, NIST P-
> 384 ECDSA
> +and NIST FIPS-204 ML-DSA (Dilithium) public key signing standards
> (though it is
> +pluggable and permits others to be used). For RSA and ECDSA, the
> possible hash
> +algorithms that can be used are SHA-2 and SHA-3 of sizes 256, 384, and
> 512 (the
> +algorithm is selected by data in the signature); ML-DSA uses SHAKE256.
>
>
> ==========================
> @@ -146,9 +147,9 @@ into vmlinux) using parameters in the::
>
> file (which is also generated if it does not already exist).
>
> -One can select between RSA (``MODULE_SIG_KEY_TYPE_RSA``) and ECDSA
> -(``MODULE_SIG_KEY_TYPE_ECDSA``) to generate either RSA 4k or NIST
> -P-384 keypair.
> +One can select between RSA (``MODULE_SIG_KEY_TYPE_RSA``), ECDSA
> +(``MODULE_SIG_KEY_TYPE_ECDSA``) and ML-DSA
> (``MODULE_SIG_KEY_TYPE_ML_DSA``) to
> +generate an RSA 4k, a NIST P-384 keypair or an ML-DSA keypair.
>
> It is strongly recommended that you provide your own x509.genkey file.
>
> diff --git a/certs/Kconfig b/certs/Kconfig
> index 78307dc25559..f647b944f5da 100644
> --- a/certs/Kconfig
> +++ b/certs/Kconfig
> @@ -39,6 +39,30 @@ config MODULE_SIG_KEY_TYPE_ECDSA
> Note: Remove all ECDSA signing keys, e.g. certs/signing_key.pem,
> when falling back to building Linux 5.14 and older kernels.
>
> +config MODULE_SIG_KEY_TYPE_ML_DSA_44
> + bool "ML-DSA (Dilithium) 44"
> + select CRYPTO_ML_DSA
> + select LIB_SHA3
> + help
> + Use an ML-DSA (Dilithium) 44 key (NIST FIPS 204) for module signing
> + with a SHAKE256 'hash' of the message.
> +
> +config MODULE_SIG_KEY_TYPE_ML_DSA_65
> + bool "ML-DSA (Dilithium) 65"
> + select CRYPTO_ML_DSA
> + select LIB_SHA3
> + help
> + Use an ML-DSA (Dilithium) 65 key (NIST FIPS 204) for module signing
> + with a SHAKE256 'hash' of the message.
> +
> +config MODULE_SIG_KEY_TYPE_ML_DSA_87
> + bool "ML-DSA (Dilithium) 87"
> + select CRYPTO_ML_DSA
> + select LIB_SHA3
> + help
> + Use an ML-DSA (Dilithium) 87 key (NIST FIPS 204) for module signing
> + with a SHAKE256 'hash' of the message.
> +
It'd be helpful to keep the names like "ML-DSA-44" together.
I would drop the Dilithium references altogether; that was used by some
pre-standard implementations that don't necessarily interoperate with the
final standard.
...
> diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
> index 2a1beebf1d37..4b5d1601d537 100644
> --- a/kernel/module/Kconfig
> +++ b/kernel/module/Kconfig
> @@ -327,6 +327,10 @@ config MODULE_SIG_SHA3_512
> bool "SHA3-512"
> select CRYPTO_SHA3
>
> +config MODULE_SIG_SHAKE256
> + bool "SHAKE256"
> + select CRYPTO_SHA3
> +
> endchoice
>
> config MODULE_SIG_HASH
> @@ -339,6 +343,7 @@ config MODULE_SIG_HASH
> default "sha3-256" if MODULE_SIG_SHA3_256
> default "sha3-384" if MODULE_SIG_SHA3_384
> default "sha3-512" if MODULE_SIG_SHA3_512
> + default "shake256" if MODULE_SIG_SHAKE256
ML-DSA can sign any message, including a message that happens
to be a hash value already. It runs its own hash algorithm
(SHAKE256) on that message unless
* using the HashML variant that nobody likes
* using the "external mu" option, requiring confidence
that it really was done, with the correct prefix
(the context string, if any, is part of the prefix)
OpenSSL supports "external mu" with the OSSL_PARAM array
parameter OSSL_SIGNATURE_PARAM_MU, but that needs to be set
with EVP APIs.
(per the OpenSSL EVP_SIGNATURE-ML-DSA documentation page)
If the goal is to just avoid SHA-2 (e.g., running on
a CPU that only has SHA-3 acceleration), then the existing
SHA3 options accomplish that; I'm not sure offering
SHAKE256 in addition to SHA3-512 is useful.
If using a composite based on the CMS proposal, then
the message is hashed with an algorithm defined in that
proposal (for id-MLDSA87-Ed448-SHAKE256, it's
SHAKE256 truncated to 64 bytes) and then fed to each
of the signing algorithms (e.g., pure ML-DSA-87 and
pure Ed448).
That hash wouldn't qualify as an "external mu" offload for
ML-DSA since the hash output size doesn't match (ML-DSA
expects 114 bytes) and the right prefix wouldn't have been
included (Ed448 doesn't expect the message to hold the
ML-DSA prefix).
> config MODULE_COMPRESS
> bool "Module compression"
> diff --git a/scripts/sign-file.c b/scripts/sign-file.c
> index 7070245edfc1..b726581075f9 100644
> --- a/scripts/sign-file.c
> +++ b/scripts/sign-file.c
> @@ -315,18 +315,28 @@ int main(int argc, char **argv)
> ERR(!digest_algo, "EVP_get_digestbyname");
>
> #ifndef USE_PKCS7
> +
> + unsigned int flags =
> + CMS_NOCERTS |
> + CMS_PARTIAL |
> + CMS_BINARY |
> + CMS_DETACHED |
> + CMS_STREAM |
> + CMS_NOSMIMECAP |
> + CMS_NO_SIGNING_TIME |
> + use_keyid;
> + if (!EVP_PKEY_is_a(private_key, "ML-DSA-44") &&
> + !EVP_PKEY_is_a(private_key, "ML-DSA-65") &&
> + !EVP_PKEY_is_a(private_key, "ML-DSA-87"))
> + flags |= use_signed_attrs;
>
> /* Load the signature message from the digest buffer. */
> - cms = CMS_sign(NULL, NULL, NULL, NULL,
> - CMS_NOCERTS | CMS_PARTIAL | CMS_BINARY |
> - CMS_DETACHED | CMS_STREAM);
> + cms = CMS_sign(NULL, NULL, NULL, NULL, flags);
For code signing, a desirable property is reproducible builds.
For RSASSA-PKCSv1_5 (and EdDSA), that is always true.
For ECDSA, OpenSSL defaults to deterministic signing, so
sign-file inherits that behavior.
For ML-DSA, the OpenSSL library defaults to hedged mode rather
than deterministic mode, so some additional steps will be
necessary to keep it deterministic.
OpenSSL supports deterministic mode with the OSSL_PARAM array
parameter OSSL_SIGNATURE_PARAM_DETERMINISTIC, but that needs to
be set with EVP APIs.
^ permalink raw reply
* [PATCH v2] gendwarfksyms: Skip files with no exports
From: Miguel Ojeda @ 2025-11-10 13:19 UTC (permalink / raw)
To: Sami Tolvanen, Miguel Ojeda, Alex Gaynor
Cc: linux-modules, linux-kbuild, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel,
patches, stable, Haiyue Wang
From: Sami Tolvanen <samitolvanen@google.com>
Starting with Rust 1.91.0 (released 2025-10-30), in upstream commit
ab91a63d403b ("Ignore intrinsic calls in cross-crate-inlining cost model")
[1][2], `bindings.o` stops containing DWARF debug information because the
`Default` implementations contained `write_bytes()` calls which are now
ignored in that cost model (note that `CLIPPY=1` does not reproduce it).
This means `gendwarfksyms` complains:
RUSTC L rust/bindings.o
error: gendwarfksyms: process_module: dwarf_get_units failed: no debugging information?
There are several alternatives that would work here: conditionally
skipping in the cases needed (but that is subtle and brittle), forcing
DWARF generation with e.g. a dummy `static` (ugly and we may need to
do it in several crates), skipping the call to the tool in the Kbuild
command when there are no exports (fine) or teaching the tool to do so
itself (simple and clean).
Thus do the last one: don't attempt to process files if we have no symbol
versions to calculate.
[ I used the commit log of my patch linked below since it explained the
root issue and expanded it a bit more to summarize the alternatives.
- Miguel ]
Cc: stable@vger.kernel.org # Needed in 6.12.y and later (Rust is pinned in older LTSs).
Reported-by: Haiyue Wang <haiyuewa@163.com>
Closes: https://lore.kernel.org/rust-for-linux/b8c1c73d-bf8b-4bf2-beb1-84ffdcd60547@163.com/
Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://lore.kernel.org/rust-for-linux/CANiq72nKC5r24VHAp9oUPR1HVPqT+=0ab9N0w6GqTF-kJOeiSw@mail.gmail.com/
Link: https://github.com/rust-lang/rust/commit/ab91a63d403b0105cacd72809cd292a72984ed99 [1]
Link: https://github.com/rust-lang/rust/pull/145910 [2]
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
scripts/gendwarfksyms/gendwarfksyms.c | 3 ++-
scripts/gendwarfksyms/gendwarfksyms.h | 2 +-
scripts/gendwarfksyms/symbols.c | 4 +++-
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/scripts/gendwarfksyms/gendwarfksyms.c b/scripts/gendwarfksyms/gendwarfksyms.c
index 08ae61eb327e..f5203d1640ee 100644
--- a/scripts/gendwarfksyms/gendwarfksyms.c
+++ b/scripts/gendwarfksyms/gendwarfksyms.c
@@ -138,7 +138,8 @@ int main(int argc, char **argv)
error("no input files?");
}
- symbol_read_exports(stdin);
+ if (!symbol_read_exports(stdin))
+ return 0;
if (symtypes_file) {
symfile = fopen(symtypes_file, "w");
diff --git a/scripts/gendwarfksyms/gendwarfksyms.h b/scripts/gendwarfksyms/gendwarfksyms.h
index d9c06d2cb1df..32cec8f7695a 100644
--- a/scripts/gendwarfksyms/gendwarfksyms.h
+++ b/scripts/gendwarfksyms/gendwarfksyms.h
@@ -123,7 +123,7 @@ struct symbol {
typedef void (*symbol_callback_t)(struct symbol *, void *arg);
bool is_symbol_ptr(const char *name);
-void symbol_read_exports(FILE *file);
+int symbol_read_exports(FILE *file);
void symbol_read_symtab(int fd);
struct symbol *symbol_get(const char *name);
void symbol_set_ptr(struct symbol *sym, Dwarf_Die *ptr);
diff --git a/scripts/gendwarfksyms/symbols.c b/scripts/gendwarfksyms/symbols.c
index 35ed594f0749..ecddcb5ffcdf 100644
--- a/scripts/gendwarfksyms/symbols.c
+++ b/scripts/gendwarfksyms/symbols.c
@@ -128,7 +128,7 @@ static bool is_exported(const char *name)
return for_each(name, NULL, NULL) > 0;
}
-void symbol_read_exports(FILE *file)
+int symbol_read_exports(FILE *file)
{
struct symbol *sym;
char *line = NULL;
@@ -159,6 +159,8 @@ void symbol_read_exports(FILE *file)
free(line);
debug("%d exported symbols", nsym);
+
+ return nsym;
}
static void get_symbol(struct symbol *sym, void *arg)
base-commit: e9a6fb0bcdd7609be6969112f3fbfcce3b1d4a7c
--
2.51.2
^ permalink raw reply related
* Re: [PATCH 6/6] kallsyms: Prevent module removal when printing module name and buildid
From: Petr Mladek @ 2025-11-10 13:26 UTC (permalink / raw)
To: Aaron Tomlin
Cc: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel
In-Reply-To: <kubk2a4ydmja45dfnwxkkhpdbov27m6errnenc6eljbgdmidzl@is24eqefukit>
On Fri 2025-11-07 19:36:35, Aaron Tomlin wrote:
> On Wed, Nov 05, 2025 at 03:23:18PM +0100, Petr Mladek wrote:
> > kallsyms_lookup_buildid() copies the symbol name into the given buffer
> > so that it can be safely read anytime later. But it just copies pointers
> > to mod->name and mod->build_id which might get reused after the related
> > struct module gets removed.
> >
> > The lifetime of struct module is synchronized using RCU. Take the rcu
> > read lock for the entire __sprint_symbol().
> >
> > Signed-off-by: Petr Mladek <pmladek@suse.com>
> > ---
> > kernel/kallsyms.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> > index ff7017337535..1fda06b6638c 100644
> > --- a/kernel/kallsyms.c
> > +++ b/kernel/kallsyms.c
> > @@ -468,6 +468,9 @@ static int __sprint_symbol(char *buffer, unsigned long address,
> > unsigned long offset, size;
> > int len;
> >
> > + /* Prevent module removal until modname and modbuildid are printed */
> > + guard(rcu)();
> > +
> > address += symbol_offset;
> > len = kallsyms_lookup_buildid(address, &size, &offset, &modname, &buildid,
> > buffer);
> > --
> > 2.51.1
> >
> >
>
> Hi Petr,
>
> If I am not mistaken, this is handled safely within the context of
> module_address_lookup() since f01369239293e ("module: Use RCU in
> find_kallsyms_symbol()."), no?
The above mention commit fixed an API which is looking only for
the symbol name. It seems to be used, for example, in kprobe
or ftrace code.
This patch is fixing another API which is used in vsprintf() for
printing backtraces. It looks for more information: symbol name,
module name, and buildid. It needs its own RCU read protection.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v2] gendwarfksyms: Skip files with no exports
From: Haiyue Wang @ 2025-11-10 14:23 UTC (permalink / raw)
To: Miguel Ojeda, Sami Tolvanen, Alex Gaynor
Cc: linux-modules, linux-kbuild, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel,
patches, stable
In-Reply-To: <20251110131913.1789896-1-ojeda@kernel.org>
On 11/10/2025 9:19 PM, Miguel Ojeda wrote:
> From: Sami Tolvanen <samitolvanen@google.com>
>
> Starting with Rust 1.91.0 (released 2025-10-30), in upstream commit
> ab91a63d403b ("Ignore intrinsic calls in cross-crate-inlining cost model")
> [1][2], `bindings.o` stops containing DWARF debug information because the
> `Default` implementations contained `write_bytes()` calls which are now
> ignored in that cost model (note that `CLIPPY=1` does not reproduce it).
>
> This means `gendwarfksyms` complains:
>
> RUSTC L rust/bindings.o
> error: gendwarfksyms: process_module: dwarf_get_units failed: no debugging information?
>
> There are several alternatives that would work here: conditionally
> skipping in the cases needed (but that is subtle and brittle), forcing
> DWARF generation with e.g. a dummy `static` (ugly and we may need to
> do it in several crates), skipping the call to the tool in the Kbuild
> command when there are no exports (fine) or teaching the tool to do so
> itself (simple and clean).
>
> Thus do the last one: don't attempt to process files if we have no symbol
> versions to calculate.
>
> [ I used the commit log of my patch linked below since it explained the
> root issue and expanded it a bit more to summarize the alternatives.
>
> - Miguel ]
>
> Cc: stable@vger.kernel.org # Needed in 6.12.y and later (Rust is pinned in older LTSs).
> Reported-by: Haiyue Wang <haiyuewa@163.com>
> Closes: https://lore.kernel.org/rust-for-linux/b8c1c73d-bf8b-4bf2-beb1-84ffdcd60547@163.com/
> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Link: https://lore.kernel.org/rust-for-linux/CANiq72nKC5r24VHAp9oUPR1HVPqT+=0ab9N0w6GqTF-kJOeiSw@mail.gmail.com/
> Link: https://github.com/rust-lang/rust/commit/ab91a63d403b0105cacd72809cd292a72984ed99 [1]
> Link: https://github.com/rust-lang/rust/pull/145910 [2]
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> scripts/gendwarfksyms/gendwarfksyms.c | 3 ++-
> scripts/gendwarfksyms/gendwarfksyms.h | 2 +-
> scripts/gendwarfksyms/symbols.c | 4 +++-
> 3 files changed, 6 insertions(+), 3 deletions(-)
>
Tested-by: Haiyue Wang <haiyuewa@163.com>
^ permalink raw reply
* Re: [PATCH v2] gendwarfksyms: Skip files with no exports
From: Alice Ryhl @ 2025-11-10 14:57 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Sami Tolvanen, Alex Gaynor, linux-modules, linux-kbuild,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Danilo Krummrich, rust-for-linux,
linux-kernel, patches, stable, Haiyue Wang
In-Reply-To: <20251110131913.1789896-1-ojeda@kernel.org>
On Mon, Nov 10, 2025 at 02:19:13PM +0100, Miguel Ojeda wrote:
> Cc: stable@vger.kernel.org # Needed in 6.12.y and later (Rust is pinned in older LTSs).
Is gendwarfksyms actually present in 6.12 upstream? I know we have it in
Android's 6.12 branch, but I thought we backported for Android only.
Alice
^ permalink raw reply
* Re: [PATCH 1/2] module: Override -EEXISTS module return
From: Daniel Gomez @ 2025-11-10 15:17 UTC (permalink / raw)
To: Lucas De Marchi, linux-modules, linux-kernel; +Cc: Petr Pavlu
In-Reply-To: <20251013-module-warn-ret-v1-1-ab65b41af01f@intel.com>
On 13/10/2025 18.26, Lucas De Marchi wrote:
> The -EEXIST errno is reserved by the module loading functionality. When
> userspace calls [f]init_module(), it expects a -EEXIST to mean that the
> module is already loaded in the kernel. If module_init() returns it,
> that is not true anymore.
>
> Add a warning and override the return code to workaround modules
> currently returning the wrong code. It's expected that they eventually
> migrate to a better suited error.
I've been following the thread (and apologies for the delay) and reviewing the
patches, and I do not believe we should push this workaround. While this "fixes"
the bug reported, it also hides the real problem and drivers will continue
misusing EEXIST at module initialization.
From the bug report thread, I agree with Christophe's suggestion that
nf_conntrack_helpers_register() should return EBUSY instead of EEXIST. This
would fix the root cause for this particular module and will allow others to
change their module behavior, if we also follow up with proper documentation
about EEXIST.
>
> Closes: https://lore.kernel.org/all/aKLzsAX14ybEjHfJ@orbyte.nwl.cc/
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> kernel/module/main.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c66b261849362..74ff87b13c517 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3038,6 +3038,11 @@ static noinline int do_init_module(struct module *mod)
> if (mod->init != NULL)
> ret = do_one_initcall(mod->init);
> if (ret < 0) {
> + if (ret == -EEXIST) {
> + pr_warn("%s: init suspiciously returned -EEXIST: Overriding with -EBUSY\n",
> + mod->name);
> + ret = -EBUSY;
> + }
> goto fail_free_freeinit;
> }
> if (ret > 0) {
>
^ permalink raw reply
* Re: [PATCH 1/2] module: Override -EEXISTS module return
From: Lucas De Marchi @ 2025-11-10 15:31 UTC (permalink / raw)
To: Daniel Gomez; +Cc: linux-modules, linux-kernel, Petr Pavlu
In-Reply-To: <ea36d12b-15b9-4c1c-b81f-75834bc3269a@kernel.org>
On Mon, Nov 10, 2025 at 04:17:47PM +0100, Daniel Gomez wrote:
>On 13/10/2025 18.26, Lucas De Marchi wrote:
>> The -EEXIST errno is reserved by the module loading functionality. When
>> userspace calls [f]init_module(), it expects a -EEXIST to mean that the
>> module is already loaded in the kernel. If module_init() returns it,
>> that is not true anymore.
>>
>> Add a warning and override the return code to workaround modules
>> currently returning the wrong code. It's expected that they eventually
>> migrate to a better suited error.
>
>I've been following the thread (and apologies for the delay) and reviewing the
>patches, and I do not believe we should push this workaround. While this "fixes"
>the bug reported, it also hides the real problem and drivers will continue
>misusing EEXIST at module initialization.
>
>From the bug report thread, I agree with Christophe's suggestion that
>nf_conntrack_helpers_register() should return EBUSY instead of EEXIST. This
>would fix the root cause for this particular module and will allow others to
>change their module behavior, if we also follow up with proper documentation
>about EEXIST.
the fix will always be for the modules to stop returning EEXIST, no
discussion on that. This is the messenger, not the fix. Someone starts
seeing this warning and reports the bug. Then the developers can fix
it. It's much easier than dealing with the outcome of a module returning
EEXIST. Also note that we already have a similar reasoning about adding
a warning for module return codes in this same function.
Even if we had the means to check possible return codes from all
module inits, we'd still have external modules.
See patch 2: after some time we can simplify the warning and maybe even
remove it.
Lucas De Marchi
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: Add myself as reviewer for module support
From: Daniel Gomez @ 2025-11-10 16:01 UTC (permalink / raw)
To: mcgrof, petr.pavlu, da.gomez, samitolvanen, Aaron Tomlin
Cc: Daniel Gomez, linux-modules, linux-kernel
In-Reply-To: <20251018180210.347619-1-atomlin@atomlin.com>
From: Daniel Gomez <da.gomez@samsung.com>
On Sat, 18 Oct 2025 14:02:10 -0400, Aaron Tomlin wrote:
> Voluntering as a reviewer for Module support.
>
>
Applied, thanks!
[1/1] MAINTAINERS: Add myself as reviewer for module support
commit: 1ddac5cd7f278345b2e8298c930e4bffe0911a45
Best regards,
--
Daniel Gomez <da.gomez@samsung.com>
^ permalink raw reply
* Re: [PATCH v2] gendwarfksyms: Skip files with no exports
From: Miguel Ojeda @ 2025-11-10 16:48 UTC (permalink / raw)
To: Alice Ryhl
Cc: Miguel Ojeda, Sami Tolvanen, Alex Gaynor, linux-modules,
linux-kbuild, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
rust-for-linux, linux-kernel, patches, stable, Haiyue Wang
In-Reply-To: <aRH9Tjf0tszyQhKX@google.com>
On Mon, Nov 10, 2025 at 3:57 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Is gendwarfksyms actually present in 6.12 upstream? I know we have it in
> Android's 6.12 branch, but I thought we backported for Android only.
No, you are right, this is just me being overeager. It should be:
Cc: stable@vger.kernel.org # Needed in 6.17.y.
It would only be needed in 6.12.y if someone actually backports the
feature (which does rarely happen, so I guess someone could have found
it useful since there is no Fixes tag, but hopefully people would grep
the log in that case...).
Thanks!
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH 6/6] kallsyms: Prevent module removal when printing module name and buildid
From: Aaron Tomlin @ 2025-11-11 2:04 UTC (permalink / raw)
To: Petr Mladek
Cc: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel
In-Reply-To: <aRHoHMJYAhSoEh1e@pathway.suse.cz>
On Mon, Nov 10, 2025 at 02:26:52PM +0100, Petr Mladek wrote:
> > Hi Petr,
> >
> > If I am not mistaken, this is handled safely within the context of
> > module_address_lookup() since f01369239293e ("module: Use RCU in
> > find_kallsyms_symbol()."), no?
>
> The above mention commit fixed an API which is looking only for
> the symbol name. It seems to be used, for example, in kprobe
> or ftrace code.
>
> This patch is fixing another API which is used in vsprintf() for
> printing backtraces. It looks for more information: symbol name,
> module name, and buildid. It needs its own RCU read protection.
Hi Petr,
I see and agree.
--
Aaron Tomlin
^ permalink raw reply
* Re: [PATCH 6/6] kallsyms: Prevent module removal when printing module name and buildid
From: Aaron Tomlin @ 2025-11-11 2:18 UTC (permalink / raw)
To: Petr Mladek
Cc: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel
In-Reply-To: <20251105142319.1139183-7-pmladek@suse.com>
On Wed, Nov 05, 2025 at 03:23:18PM +0100, Petr Mladek wrote:
> kallsyms_lookup_buildid() copies the symbol name into the given buffer
> so that it can be safely read anytime later. But it just copies pointers
> to mod->name and mod->build_id which might get reused after the related
> struct module gets removed.
>
> The lifetime of struct module is synchronized using RCU. Take the rcu
> read lock for the entire __sprint_symbol().
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
> kernel/kallsyms.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index ff7017337535..1fda06b6638c 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -468,6 +468,9 @@ static int __sprint_symbol(char *buffer, unsigned long address,
> unsigned long offset, size;
> int len;
>
> + /* Prevent module removal until modname and modbuildid are printed */
> + guard(rcu)();
> +
> address += symbol_offset;
> len = kallsyms_lookup_buildid(address, &size, &offset, &modname, &buildid,
> buffer);
> --
> 2.51.1
Looks good to me.
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
--
Aaron Tomlin
^ permalink raw reply
* [PATCH v3] module: replace use of system_wq with system_dfl_wq
From: Marco Crivellari @ 2025-11-11 9:50 UTC (permalink / raw)
To: linux-kernel, linux-modules
Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
Sebastian Andrzej Siewior, Marco Crivellari, Michal Hocko,
Luis Chamberlain, Petr Pavlu
Currently if a user enqueues a work item using schedule_delayed_work() the
used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use
WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to
schedule_work() that is using system_wq and queue_work(), that makes use
again of WORK_CPU_UNBOUND.
This lack of consistency cannot be addressed without refactoring the API.
This continues the effort to refactor workqueue APIs, which began with
the introduction of new workqueues and a new alloc_workqueue flag in:
commit 128ea9f6ccfb ("workqueue: Add system_percpu_wq and system_dfl_wq")
commit 930c2ea566af ("workqueue: Add new WQ_PERCPU flag")
Switch to using system_dfl_wq, the new unbound workqueue, because the
users do not benefit from a per-cpu workqueue.
Suggested-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
---
Changes in v3:
- rebased on 6.18-rc5
- commit log improved
Changes in v2:
- a per-cpu wq is not needed: replace system_wq with system_dfl_wq
---
kernel/module/dups.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/module/dups.c b/kernel/module/dups.c
index bd2149fbe117..0b633f2edda6 100644
--- a/kernel/module/dups.c
+++ b/kernel/module/dups.c
@@ -113,7 +113,7 @@ static void kmod_dup_request_complete(struct work_struct *work)
* let this linger forever as this is just a boot optimization for
* possible abuses of vmalloc() incurred by finit_module() thrashing.
*/
- queue_delayed_work(system_wq, &kmod_req->delete_work, 60 * HZ);
+ queue_delayed_work(system_dfl_wq, &kmod_req->delete_work, 60 * HZ);
}
bool kmod_dup_request_exists_wait(char *module_name, bool wait, int *dup_ret)
@@ -240,7 +240,7 @@ void kmod_dup_request_announce(char *module_name, int ret)
* There is no rush. But we also don't want to hold the
* caller up forever or introduce any boot delays.
*/
- queue_work(system_wq, &kmod_req->complete_work);
+ queue_work(system_dfl_wq, &kmod_req->complete_work);
out:
mutex_unlock(&kmod_dup_mutex);
--
2.51.1
^ permalink raw reply related
* Re: [PATCH v2 0/3] module: Add compile-time check for embedded NUL characters
From: Andy Shevchenko @ 2025-11-11 11:42 UTC (permalink / raw)
To: Daniel Gomez
Cc: Kees Cook, Luis Chamberlain, Hans Verkuil, Malcolm Priestley,
Mauro Carvalho Chehab, Hans Verkuil, Uwe Kleine-König,
Rusty Russell, Petr Pavlu, Sami Tolvanen, linux-kernel,
linux-media, linux-modules, linux-hardening
In-Reply-To: <3dd1a00d-08f7-4801-a9f7-d6db61c0e0f3@kernel.org>
On Wed, Nov 05, 2025 at 02:03:59PM +0100, Daniel Gomez wrote:
> On 10/10/2025 05.06, Kees Cook wrote:
> > v2:
> > - use static_assert instead of _Static_assert
> > - add Hans's Reviewed-by's
> > v1: https://lore.kernel.org/lkml/20251008033844.work.801-kees@kernel.org/
> >
> > Hi!
> >
> > A long time ago we had an issue with embedded NUL bytes in MODULE_INFO
> > strings[1]. While this stands out pretty strongly when you look at the
> > code, and we can't do anything about a binary module that just plain lies,
> > we never actually implemented the trivial compile-time check needed to
> > detect it.
> >
> > Add this check (and fix 2 instances of needless trailing semicolons that
> > this change exposed).
> >
> > Note that these patches were produced as part of another LLM exercise.
> > This time I wanted to try "what happens if I ask an LLM to go read
> > a specific LWN article and write a patch based on a discussion?" It
> > pretty effortlessly chose and implemented a suggested solution, tested
> > the change, and fixed new build warnings in the process.
> >
> > Since this was a relatively short session, here's an overview of the
> > prompts involved as I guided it through a clean change and tried to see
> > how it would reason about static_assert vs _Static_assert. (It wanted
> > to use what was most common, not what was the current style -- we may
> > want to update the comment above the static_assert macro to suggest
> > using _Static_assert directly these days...)
> >
> > I want to fix a weakness in the module info strings. Read about it
> > here: https://lwn.net/Articles/82305/
> >
> > Since it's only "info" that we need to check, can you reduce the checks
> > to just that instead of all the other stuff?
> >
> > I think the change to the comment is redundent, and that should be
> > in a commit log instead. Let's just keep the change to the static assert.
> >
> > Is "static_assert" the idiomatic way to use a static assert in this
> > code base? I've seen _Static_assert used sometimes.
> >
> > What's the difference between the two?
> >
> > Does Linux use C11 by default now?
> >
> > Then let's not use the wrapper any more.
> >
> > Do an "allmodconfig all -s" build to verify this works for all modules
> > in the kernel.
> >
> >
> > Thanks!
> >
> > -Kees
> >
> > [1] https://lwn.net/Articles/82305/
> >
> > Kees Cook (3):
> > media: dvb-usb-v2: lmedm04: Fix firmware macro definitions
> > media: radio: si470x: Fix DRIVER_AUTHOR macro definition
> > module: Add compile-time check for embedded NUL characters
> >
> > include/linux/moduleparam.h | 3 +++
> > drivers/media/radio/si470x/radio-si470x-i2c.c | 2 +-
> > drivers/media/usb/dvb-usb-v2/lmedm04.c | 12 ++++++------
> > 3 files changed, 10 insertions(+), 7 deletions(-)
> >
>
> Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
>
> I have also tested a build of v6.18-rc3 + patches using allmodconfig:
>
> Tested-by: Daniel Gomez <da.gomez@samsung.com>
Folks, are you aware that this change blown up the sparse?
Now there is a "bad constant expression" to each MODULE_*() macro line.
Nice that Uwe is in the Cc list, so IIRC he is Debian maintainer for sparse
and perhaps has an influence to it to some extent.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 0/3] module: Add compile-time check for embedded NUL characters
From: Daniel Gomez @ 2025-11-11 12:34 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Kees Cook, Luis Chamberlain, Hans Verkuil, Malcolm Priestley,
Mauro Carvalho Chehab, Hans Verkuil, Uwe Kleine-König,
Rusty Russell, Petr Pavlu, Sami Tolvanen, linux-kernel,
linux-media, linux-modules, linux-hardening
In-Reply-To: <aRMhLEs9NpGexL7B@black.igk.intel.com>
On 11/11/2025 12.42, Andy Shevchenko wrote:
> On Wed, Nov 05, 2025 at 02:03:59PM +0100, Daniel Gomez wrote:
>> On 10/10/2025 05.06, Kees Cook wrote:
>>> v2:
>>> - use static_assert instead of _Static_assert
>>> - add Hans's Reviewed-by's
>>> v1: https://lore.kernel.org/lkml/20251008033844.work.801-kees@kernel.org/
>>>
>>> Hi!
>>>
>>> A long time ago we had an issue with embedded NUL bytes in MODULE_INFO
>>> strings[1]. While this stands out pretty strongly when you look at the
>>> code, and we can't do anything about a binary module that just plain lies,
>>> we never actually implemented the trivial compile-time check needed to
>>> detect it.
>>>
>>> Add this check (and fix 2 instances of needless trailing semicolons that
>>> this change exposed).
>>>
>>> Note that these patches were produced as part of another LLM exercise.
>>> This time I wanted to try "what happens if I ask an LLM to go read
>>> a specific LWN article and write a patch based on a discussion?" It
>>> pretty effortlessly chose and implemented a suggested solution, tested
>>> the change, and fixed new build warnings in the process.
>>>
>>> Since this was a relatively short session, here's an overview of the
>>> prompts involved as I guided it through a clean change and tried to see
>>> how it would reason about static_assert vs _Static_assert. (It wanted
>>> to use what was most common, not what was the current style -- we may
>>> want to update the comment above the static_assert macro to suggest
>>> using _Static_assert directly these days...)
>>>
>>> I want to fix a weakness in the module info strings. Read about it
>>> here: https://lwn.net/Articles/82305/
>>>
>>> Since it's only "info" that we need to check, can you reduce the checks
>>> to just that instead of all the other stuff?
>>>
>>> I think the change to the comment is redundent, and that should be
>>> in a commit log instead. Let's just keep the change to the static assert.
>>>
>>> Is "static_assert" the idiomatic way to use a static assert in this
>>> code base? I've seen _Static_assert used sometimes.
>>>
>>> What's the difference between the two?
>>>
>>> Does Linux use C11 by default now?
>>>
>>> Then let's not use the wrapper any more.
>>>
>>> Do an "allmodconfig all -s" build to verify this works for all modules
>>> in the kernel.
>>>
>>>
>>> Thanks!
>>>
>>> -Kees
>>>
>>> [1] https://lwn.net/Articles/82305/
>>>
>>> Kees Cook (3):
>>> media: dvb-usb-v2: lmedm04: Fix firmware macro definitions
>>> media: radio: si470x: Fix DRIVER_AUTHOR macro definition
>>> module: Add compile-time check for embedded NUL characters
>>>
>>> include/linux/moduleparam.h | 3 +++
>>> drivers/media/radio/si470x/radio-si470x-i2c.c | 2 +-
>>> drivers/media/usb/dvb-usb-v2/lmedm04.c | 12 ++++++------
>>> 3 files changed, 10 insertions(+), 7 deletions(-)
>>>
>>
>> Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
>>
>> I have also tested a build of v6.18-rc3 + patches using allmodconfig:
>>
>> Tested-by: Daniel Gomez <da.gomez@samsung.com>
>
> Folks, are you aware that this change blown up the sparse?
> Now there is a "bad constant expression" to each MODULE_*() macro line.
Thanks for the heads up.
I can see this thread:
https://lore.kernel.org/all/D1CBCBE2-3A54-410A-B15C-F1C621F9F56B@kernel.org/#t
And this:
https://lore.kernel.org/linux-sparse/CACePvbVG2KrGQq4cNKV=wbO5h=jp3M0RO1SdfX8kV4OukjPG8A@mail.gmail.com/T/#t
>
> Nice that Uwe is in the Cc list, so IIRC he is Debian maintainer for sparse
> and perhaps has an influence to it to some extent.
>
Would it be better approach to postpone patch 3 from Kent until sparse is fixed?
^ permalink raw reply
* Re: [v2,0/3] module: Add compile-time check for embedded NUL characters
From: Daniel Gomez @ 2025-11-11 13:14 UTC (permalink / raw)
To: Kees Cook, Ricardo Ribalda
Cc: Dan Carpenter, linux-media, Patchwork Integration, linux-modules,
linux-sparse
In-Reply-To: <202510201146.F12EA92@keescook>
On Mon, Oct 20, 2025 at 11:51:05AM -0700, Kees Cook wrote:
> On Mon, Oct 20, 2025 at 08:35:53PM +0200, Ricardo Ribalda wrote:
> > Hi Kees
> >
> > On Mon, 20 Oct 2025 at 20:29, Kees Cook <kees@kernel.org> wrote:
> > >
> > > On Wed, Oct 15, 2025 at 09:33:40AM +0200, Ricardo Ribalda wrote:
> > > > Hi Dan
> > > >
> > > > On Tue, 14 Oct 2025 at 22:45, Kees Cook <kees@kernel.org> wrote:
> > > > >
> > > > > On Tue, Oct 14, 2025 at 08:24:00AM +0200, Ricardo Ribalda wrote:
> > > > > > Hi Kees
> > > > > >
> > > > > > Thanks for the report.
> > > > > >
> > > > > >
> > > > > > On Tue, 14 Oct 2025 at 07:41, Kees Cook <kees@kernel.org> wrote:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On October 13, 2025 1:34:20 PM PDT, Patchwork Integration <patchwork@media-ci.org> wrote:
> > > > > > > >Dear Kees Cook:
> > > > > > > >
> > > > > > > >Thanks for your patches! Unfortunately the Media CI robot detected some
> > > > > > > >issues:
> > > > > > > >
> > > > > > > ># Test static:test-smatch
> > > > > > > >
> > > > > > > >drivers/media/usb/usbtv/usbtv-core.c:157:1: error: bad constant expression
> > > > > > >
> > > > > > > Where can I find what this test actually does?
> > > > > > >
> > > > > > > >For more details, check the full report at:
> > > > > > > >https://linux-media.pages.freedesktop.org/-/users/patchwork/-/jobs/85913402/artifacts/report.htm .
> > > > > > >
> > > > > > > This webserver appears to be misconfigured to send compressed output without the right headers? I can't actually view this URL.
> > > > > >
> > > > > > I will follow-up with fdo maintainers to figure out what happened.
> > > > > > there. On the meantime you can use these url that seems to work:
> > > > > > https://linux-media.pages.freedesktop.org/-/users/patchwork/-/jobs/85913402/artifacts/report.txt
> > > > > > https://gitlab.freedesktop.org/linux-media/users/patchwork/-/jobs/85913398
> > > > > >
> > > > > > Basically sparse/smatch do not seem to understand the constant.
> > > > >
> > > > > Yeah, I managed to find the actual scripts that are run for the
> > > > > static-sparse/smatch tests. It looks like those tools aren't correctly
> > > > > handling string literals for __builtin_strlen(), which is a constant for
> > > > > constant arguments.
> > > > >
> > > > > So, that's a C parsing bug in those tools (GCC and Clang are fine).
> > > >
> > > > Could you take a look at this patch:
> > > > https://patchwork.linuxtv.org/project/linux-media/patch/20251010030610.3032147-3-kees@kernel.org/
> > > >
> > > > Seems that sparse/smatch are not very happy about __builtin_strlen()
> > > >
> > > > Could you fix support for __builtin_strlen() in your tool?
> > > >
> > > > Once Kees lands his patch it will break all the CIs using
> > > > sparse/smatch, including media-ci.
> > > >
> > > > Eg:
> > > >
> > > > drivers/media/pci/zoran/zr36060.c:33:1: error: bad constant expression
> > > > drivers/media/usb/pvrusb2/pvrusb2-dvb.c:19:1: error: bad constant expression
> > > > drivers/media/usb/pvrusb2/pvrusb2-dvb.c:19:1: error: bad constant expression
> > >
> > > We've waited a decade to get the embedded-NUL check into the modinfo
> > > macros, so I'm happy to wait until we can get the CI tooling updated.
> >
> > For media-ci. It will probably be after 6.19rc1
> >
> > Basically, when
> > https://lore.kernel.org/linux-sparse/CACePvbVG2KrGQq4cNKV=wbO5h=jp3M0RO1SdfX8kV4OukjPG8A@mail.gmail.com/T/#t
> > lands in media-committers.
>
> That's external to Linux, though; it's a patch for sparse and smatch. How
> often does the CI rebuild sparse and smatch?
>
> > How did you plan to land this series? via which tree?
>
> I assume it would go either via the modules tree or the hardening tree.
> (Again, no rush.)
FYI, the patch is applied to modules-next, so I was planning to send it
for v6.19-rc1.
^ permalink raw reply
* Re: [PATCH v2] gendwarfksyms: Skip files with no exports
From: Miguel Ojeda @ 2025-11-11 13:54 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Sami Tolvanen, Alex Gaynor, linux-modules, linux-kbuild,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
rust-for-linux, linux-kernel, patches, stable, Haiyue Wang
In-Reply-To: <20251110131913.1789896-1-ojeda@kernel.org>
On Mon, Nov 10, 2025 at 2:19 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> From: Sami Tolvanen <samitolvanen@google.com>
>
> Starting with Rust 1.91.0 (released 2025-10-30), in upstream commit
> ab91a63d403b ("Ignore intrinsic calls in cross-crate-inlining cost model")
> [1][2], `bindings.o` stops containing DWARF debug information because the
> `Default` implementations contained `write_bytes()` calls which are now
> ignored in that cost model (note that `CLIPPY=1` does not reproduce it).
>
> This means `gendwarfksyms` complains:
>
> RUSTC L rust/bindings.o
> error: gendwarfksyms: process_module: dwarf_get_units failed: no debugging information?
>
> There are several alternatives that would work here: conditionally
> skipping in the cases needed (but that is subtle and brittle), forcing
> DWARF generation with e.g. a dummy `static` (ugly and we may need to
> do it in several crates), skipping the call to the tool in the Kbuild
> command when there are no exports (fine) or teaching the tool to do so
> itself (simple and clean).
>
> Thus do the last one: don't attempt to process files if we have no symbol
> versions to calculate.
>
> [ I used the commit log of my patch linked below since it explained the
> root issue and expanded it a bit more to summarize the alternatives.
>
> - Miguel ]
>
> Cc: stable@vger.kernel.org # Needed in 6.12.y and later (Rust is pinned in older LTSs).
> Reported-by: Haiyue Wang <haiyuewa@163.com>
> Closes: https://lore.kernel.org/rust-for-linux/b8c1c73d-bf8b-4bf2-beb1-84ffdcd60547@163.com/
> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Link: https://lore.kernel.org/rust-for-linux/CANiq72nKC5r24VHAp9oUPR1HVPqT+=0ab9N0w6GqTF-kJOeiSw@mail.gmail.com/
> Link: https://github.com/rust-lang/rust/commit/ab91a63d403b0105cacd72809cd292a72984ed99 [1]
> Link: https://github.com/rust-lang/rust/pull/145910 [2]
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
I will send a couple other fixes to Linus this week, so if nobody
shouts, I will be picking this one.
Thanks!
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH v2] gendwarfksyms: Skip files with no exports
From: Miguel Ojeda @ 2025-11-11 13:58 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Sami Tolvanen, Alex Gaynor, linux-modules, linux-kbuild,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
rust-for-linux, linux-kernel, patches, stable, Haiyue Wang
In-Reply-To: <CANiq72mjFobjfQEtNvk9aA+757RkLpcfmCCEJAH69ZYsr67GdA@mail.gmail.com>
On Tue, Nov 11, 2025 at 2:54 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> I will send a couple other fixes to Linus this week, so if nobody
> shouts, I will be picking this one.
Of course, if someone else (thanks Haiyue!) wants to give tag or two,
I can still pick those up, and that would be very welcome.
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH v2] gendwarfksyms: Skip files with no exports
From: Alice Ryhl @ 2025-11-11 15:05 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Miguel Ojeda, Sami Tolvanen, Alex Gaynor, linux-modules,
linux-kbuild, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
rust-for-linux, linux-kernel, patches, stable, Haiyue Wang
In-Reply-To: <CANiq72m4K+UZxodnKqdx3cowbYB+Mj_Z0gB63j=3jE+E-x+3UA@mail.gmail.com>
On Mon, Nov 10, 2025 at 05:48:51PM +0100, Miguel Ojeda wrote:
> On Mon, Nov 10, 2025 at 3:57 PM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > Is gendwarfksyms actually present in 6.12 upstream? I know we have it in
> > Android's 6.12 branch, but I thought we backported for Android only.
>
> No, you are right, this is just me being overeager. It should be:
>
> Cc: stable@vger.kernel.org # Needed in 6.17.y.
>
> It would only be needed in 6.12.y if someone actually backports the
> feature (which does rarely happen, so I guess someone could have found
> it useful since there is no Fixes tag, but hopefully people would grep
> the log in that case...).
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
^ permalink raw reply
* Re: [PATCH v2] gendwarfksyms: Skip files with no exports
From: Sedat Dilek @ 2025-11-11 15:24 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Miguel Ojeda, Sami Tolvanen, Alex Gaynor, linux-modules,
linux-kbuild, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, rust-for-linux, linux-kernel, patches, stable,
Haiyue Wang
In-Reply-To: <CANiq72mjFobjfQEtNvk9aA+757RkLpcfmCCEJAH69ZYsr67GdA@mail.gmail.com>
On Tue, Nov 11, 2025 at 2:58 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
...
>
> I will send a couple other fixes to Linus this week, so if nobody
> shouts, I will be picking this one.
>
Hi Miguel, Hi Sami,
I switched over to gendwarfksyms in the very early testing days.
Faster builds. DWARFv5 fan.
And was using v5 of Sami's patchset against Linux v6.12 as it cleanly applied.
Last week, I jumped over to Linux v6.17.6 and the next testing will be
Linux v6.18-rc5+ (upcoming next LTS kernel-version).
I will try this patch - might be you will get a Tested-by.
Best regards,
-Sedat-
^ permalink raw reply
* [PATCH 1/2] module: Remove SHA-1 support for module signing
From: Petr Pavlu @ 2025-11-11 15:48 UTC (permalink / raw)
To: David Howells, David Woodhouse, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Aaron Tomlin
Cc: keyrings, linux-modules, linux-kernel
In-Reply-To: <20251111154923.978181-1-petr.pavlu@suse.com>
SHA-1 is considered deprecated and insecure due to vulnerabilities that can
lead to hash collisions. Most distributions have already been using SHA-2
for module signing because of this. The default was also changed last year
from SHA-1 to SHA-512 in commit f3b93547b91a ("module: sign with sha512
instead of sha1 by default"). This was not reported to cause any issues.
Therefore, it now seems to be a good time to remove SHA-1 support for
module signing.
Commit 16ab7cb5825f ("crypto: pkcs7 - remove sha1 support") previously
removed support for reading PKCS#7/CMS signed with SHA-1, along with the
ability to use SHA-1 for module signing. This change broke iwd and was
subsequently completely reverted in commit 203a6763ab69 ("Revert "crypto:
pkcs7 - remove sha1 support""). However, dropping only the support for
using SHA-1 for module signing is unrelated and can still be done
separately.
Note that this change only removes support for new modules to be SHA-1
signed, but already signed modules can still be loaded.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/module/Kconfig | 5 -----
1 file changed, 5 deletions(-)
diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
index 2a1beebf1d37..be74917802ad 100644
--- a/kernel/module/Kconfig
+++ b/kernel/module/Kconfig
@@ -299,10 +299,6 @@ choice
possible to load a signed module containing the algorithm to check
the signature on that module.
-config MODULE_SIG_SHA1
- bool "SHA-1"
- select CRYPTO_SHA1
-
config MODULE_SIG_SHA256
bool "SHA-256"
select CRYPTO_SHA256
@@ -332,7 +328,6 @@ endchoice
config MODULE_SIG_HASH
string
depends on MODULE_SIG || IMA_APPRAISE_MODSIG
- default "sha1" if MODULE_SIG_SHA1
default "sha256" if MODULE_SIG_SHA256
default "sha384" if MODULE_SIG_SHA384
default "sha512" if MODULE_SIG_SHA512
--
2.51.1
^ permalink raw reply related
* [PATCH 0/2] module: Remove SHA-1 support for module signing
From: Petr Pavlu @ 2025-11-11 15:48 UTC (permalink / raw)
To: David Howells, David Woodhouse, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Aaron Tomlin
Cc: keyrings, linux-modules, linux-kernel
SHA-1 is considered deprecated and insecure due to vulnerabilities that can
lead to hash collisions. Most distributions have already been using SHA-2
for module signing because of this. The default was also changed last year
from SHA-1 to SHA-512 in f3b93547b91a ("module: sign with sha512 instead of
sha1 by default"). This was not reported to cause any issues. Therefore, it
now seems to be a good time to remove SHA-1 support for module signing.
Looking at the configs of several distributions [1], it seems only Android
still uses SHA-1 for module signing.
@Sami, it this correct and is there a specific reason for using SHA-1?
Note: The second patch has a minor conflict with the sign-file update in the
series "lib/crypto: Add ML-DSA signing" [2].
[1] https://oracle.github.io/kconfigs/?config=UTS_RELEASE&config=MODULE_SIG_SHA1&version=be8f5f6abf0b0979be20ee8d9afa2a49a13500b8
[2] https://lore.kernel.org/linux-crypto/61637.1762509938@warthog.procyon.org.uk/
Petr Pavlu (2):
module: Remove SHA-1 support for module signing
sign-file: Remove support for signing with PKCS#7
kernel/module/Kconfig | 5 ----
scripts/sign-file.c | 66 ++-----------------------------------------
2 files changed, 3 insertions(+), 68 deletions(-)
base-commit: 4427259cc7f7571a157fbc9b5011e1ef6fe0a4a8
--
2.51.1
^ permalink raw reply
* [PATCH 2/2] sign-file: Remove support for signing with PKCS#7
From: Petr Pavlu @ 2025-11-11 15:48 UTC (permalink / raw)
To: David Howells, David Woodhouse, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Aaron Tomlin
Cc: keyrings, linux-modules, linux-kernel
In-Reply-To: <20251111154923.978181-1-petr.pavlu@suse.com>
The PKCS#7 code in sign-file allows for signing only with SHA-1. Since
SHA-1 support for module signing has been removed, drop PKCS#7 support in
favor of using only CMS.
The use of the PKCS#7 code is selected by the following:
#if defined(LIBRESSL_VERSION_NUMBER) || \
OPENSSL_VERSION_NUMBER < 0x10000000L || \
defined(OPENSSL_NO_CMS)
#define USE_PKCS7
#endif
Looking at the individual ifdefs:
* LIBRESSL_VERSION_NUMBER: LibreSSL added the CMS implementation from
OpenSSL in 3.1.0, making the ifdef no longer relevant. This version was
released on April 8, 2020.
* OPENSSL_VERSION_NUMBER < 0x10000000L: OpenSSL 1.0.0 was released on March
29, 2010. Supporting earlier versions should no longer be necessary. The
file Documentation/process/changes.rst already states that at least
version 1.0.0 is required to build the kernel.
* OPENSSL_NO_CMS: OpenSSL can be configured with "no-cms" to disable the
CMS support. In this case, sign-file will no longer be usable. The CMS
support is now required.
In practice, since distributions now typically sign modules with SHA-2, for
which sign-file already required CMS support, removing PKCS#7 shouldn't
cause any issues.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
scripts/sign-file.c | 66 +++------------------------------------------
1 file changed, 3 insertions(+), 63 deletions(-)
diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index 7070245edfc1..16f2bf2e1e3c 100644
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -24,6 +24,7 @@
#include <arpa/inet.h>
#include <openssl/opensslv.h>
#include <openssl/bio.h>
+#include <openssl/cms.h>
#include <openssl/evp.h>
#include <openssl/pem.h>
#include <openssl/err.h>
@@ -39,29 +40,6 @@
#endif
#include "ssl-common.h"
-/*
- * Use CMS if we have openssl-1.0.0 or newer available - otherwise we have to
- * assume that it's not available and its header file is missing and that we
- * should use PKCS#7 instead. Switching to the older PKCS#7 format restricts
- * the options we have on specifying the X.509 certificate we want.
- *
- * Further, older versions of OpenSSL don't support manually adding signers to
- * the PKCS#7 message so have to accept that we get a certificate included in
- * the signature message. Nor do such older versions of OpenSSL support
- * signing with anything other than SHA1 - so we're stuck with that if such is
- * the case.
- */
-#if defined(LIBRESSL_VERSION_NUMBER) || \
- OPENSSL_VERSION_NUMBER < 0x10000000L || \
- defined(OPENSSL_NO_CMS)
-#define USE_PKCS7
-#endif
-#ifndef USE_PKCS7
-#include <openssl/cms.h>
-#else
-#include <openssl/pkcs7.h>
-#endif
-
struct module_signature {
uint8_t algo; /* Public-key crypto algorithm [0] */
uint8_t hash; /* Digest algorithm [0] */
@@ -228,15 +206,10 @@ int main(int argc, char **argv)
bool raw_sig = false;
unsigned char buf[4096];
unsigned long module_size, sig_size;
- unsigned int use_signed_attrs;
const EVP_MD *digest_algo;
EVP_PKEY *private_key;
-#ifndef USE_PKCS7
CMS_ContentInfo *cms = NULL;
unsigned int use_keyid = 0;
-#else
- PKCS7 *pkcs7 = NULL;
-#endif
X509 *x509;
BIO *bd, *bm;
int opt, n;
@@ -246,21 +219,13 @@ int main(int argc, char **argv)
key_pass = getenv("KBUILD_SIGN_PIN");
-#ifndef USE_PKCS7
- use_signed_attrs = CMS_NOATTR;
-#else
- use_signed_attrs = PKCS7_NOATTR;
-#endif
-
do {
opt = getopt(argc, argv, "sdpk");
switch (opt) {
case 's': raw_sig = true; break;
case 'p': save_sig = true; break;
case 'd': sign_only = true; save_sig = true; break;
-#ifndef USE_PKCS7
case 'k': use_keyid = CMS_USE_KEYID; break;
-#endif
case -1: break;
default: format();
}
@@ -289,14 +254,6 @@ int main(int argc, char **argv)
replace_orig = true;
}
-#ifdef USE_PKCS7
- if (strcmp(hash_algo, "sha1") != 0) {
- fprintf(stderr, "sign-file: %s only supports SHA1 signing\n",
- OPENSSL_VERSION_TEXT);
- exit(3);
- }
-#endif
-
/* Open the module file */
bm = BIO_new_file(module_name, "rb");
ERR(!bm, "%s", module_name);
@@ -314,7 +271,6 @@ int main(int argc, char **argv)
digest_algo = EVP_get_digestbyname(hash_algo);
ERR(!digest_algo, "EVP_get_digestbyname");
-#ifndef USE_PKCS7
/* Load the signature message from the digest buffer. */
cms = CMS_sign(NULL, NULL, NULL, NULL,
CMS_NOCERTS | CMS_PARTIAL | CMS_BINARY |
@@ -323,19 +279,12 @@ int main(int argc, char **argv)
ERR(!CMS_add1_signer(cms, x509, private_key, digest_algo,
CMS_NOCERTS | CMS_BINARY |
- CMS_NOSMIMECAP | use_keyid |
- use_signed_attrs),
+ CMS_NOSMIMECAP | CMS_NOATTR |
+ use_keyid),
"CMS_add1_signer");
ERR(CMS_final(cms, bm, NULL, CMS_NOCERTS | CMS_BINARY) != 1,
"CMS_final");
-#else
- pkcs7 = PKCS7_sign(x509, private_key, NULL, bm,
- PKCS7_NOCERTS | PKCS7_BINARY |
- PKCS7_DETACHED | use_signed_attrs);
- ERR(!pkcs7, "PKCS7_sign");
-#endif
-
if (save_sig) {
char *sig_file_name;
BIO *b;
@@ -344,13 +293,8 @@ int main(int argc, char **argv)
"asprintf");
b = BIO_new_file(sig_file_name, "wb");
ERR(!b, "%s", sig_file_name);
-#ifndef USE_PKCS7
ERR(i2d_CMS_bio_stream(b, cms, NULL, 0) != 1,
"%s", sig_file_name);
-#else
- ERR(i2d_PKCS7_bio(b, pkcs7) != 1,
- "%s", sig_file_name);
-#endif
BIO_free(b);
}
@@ -377,11 +321,7 @@ int main(int argc, char **argv)
module_size = BIO_number_written(bd);
if (!raw_sig) {
-#ifndef USE_PKCS7
ERR(i2d_CMS_bio_stream(bd, cms, NULL, 0) != 1, "%s", dest_name);
-#else
- ERR(i2d_PKCS7_bio(bd, pkcs7) != 1, "%s", dest_name);
-#endif
} else {
BIO *b;
--
2.51.1
^ permalink raw reply related
* Re: [PATCH v3] module: replace use of system_wq with system_dfl_wq
From: Petr Pavlu @ 2025-11-11 16:19 UTC (permalink / raw)
To: Marco Crivellari
Cc: Daniel Gomez, Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
Sebastian Andrzej Siewior, Michal Hocko, Luis Chamberlain,
linux-kernel, linux-modules
In-Reply-To: <20251111095049.67658-1-marco.crivellari@suse.com>
On 11/11/25 10:50 AM, Marco Crivellari wrote:
> Currently if a user enqueues a work item using schedule_delayed_work() the
> used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use
> WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to
> schedule_work() that is using system_wq and queue_work(), that makes use
> again of WORK_CPU_UNBOUND.
>
> This lack of consistency cannot be addressed without refactoring the API.
>
> This continues the effort to refactor workqueue APIs, which began with
> the introduction of new workqueues and a new alloc_workqueue flag in:
>
> commit 128ea9f6ccfb ("workqueue: Add system_percpu_wq and system_dfl_wq")
> commit 930c2ea566af ("workqueue: Add new WQ_PERCPU flag")
>
> Switch to using system_dfl_wq, the new unbound workqueue, because the
> users do not benefit from a per-cpu workqueue.
>
> Suggested-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH 0/2] module: Remove SHA-1 support for module signing
From: Sami Tolvanen @ 2025-11-11 16:22 UTC (permalink / raw)
To: Petr Pavlu
Cc: David Howells, David Woodhouse, Luis Chamberlain, Daniel Gomez,
Aaron Tomlin, keyrings, linux-modules, linux-kernel
In-Reply-To: <20251111154923.978181-1-petr.pavlu@suse.com>
Hi Petr,
On Tue, Nov 11, 2025 at 7:49 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> SHA-1 is considered deprecated and insecure due to vulnerabilities that can
> lead to hash collisions. Most distributions have already been using SHA-2
> for module signing because of this. The default was also changed last year
> from SHA-1 to SHA-512 in f3b93547b91a ("module: sign with sha512 instead of
> sha1 by default"). This was not reported to cause any issues. Therefore, it
> now seems to be a good time to remove SHA-1 support for module signing.
>
> Looking at the configs of several distributions [1], it seems only Android
> still uses SHA-1 for module signing.
>
> @Sami, it this correct and is there a specific reason for using SHA-1?
It looks like GKI just uses the defaults here. Overall, Android
doesn't rely on module signing for security, it's only used to
differentiate between module types. Dropping SHA-1 support sounds like
a good idea to me.
> Note: The second patch has a minor conflict with the sign-file update in the
> series "lib/crypto: Add ML-DSA signing" [2].
>
> [1] https://oracle.github.io/kconfigs/?config=UTS_RELEASE&config=MODULE_SIG_SHA1&version=be8f5f6abf0b0979be20ee8d9afa2a49a13500b8
> [2] https://lore.kernel.org/linux-crypto/61637.1762509938@warthog.procyon.org.uk/
>
> Petr Pavlu (2):
> module: Remove SHA-1 support for module signing
> sign-file: Remove support for signing with PKCS#7
>
> kernel/module/Kconfig | 5 ----
> scripts/sign-file.c | 66 ++-----------------------------------------
> 2 files changed, 3 insertions(+), 68 deletions(-)
For the series:
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Sami
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox