| 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: <CAK7LNASDYVfPPvR7RB1t2iB653nkFRmitKp-mV59Jx4DFm=jGQ@mail.gmail.com>
Date: Sun, 29 Dec 2024 01:04:20 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Matthew Maurer <mmaurer@...gle.com>
Cc: Michael Ellerman <mpe@...erman.id.au>, Nicholas Piggin <npiggin@...il.com>,
Christophe Leroy <christophe.leroy@...roup.eu>, Naveen N Rao <naveen@...nel.org>,
Madhavan Srinivasan <maddy@...ux.ibm.com>, Luis Chamberlain <mcgrof@...nel.org>,
Petr Pavlu <petr.pavlu@...e.com>, Sami Tolvanen <samitolvanen@...gle.com>,
Daniel Gomez <da.gomez@...sung.com>, Nathan Chancellor <nathan@...nel.org>,
Nicolas Schier <nicolas@...sle.eu>, Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>,
Gary Guo <gary@...yguo.net>, Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
Jonathan Corbet <corbet@....net>, linuxppc-dev@...ts.ozlabs.org,
linux-kernel@...r.kernel.org, linux-modules@...r.kernel.org,
linux-kbuild@...r.kernel.org, rust-for-linux@...r.kernel.org,
linux-doc@...r.kernel.org
Subject: Re: [PATCH v11 2/5] modpost: Produce extended MODVERSIONS information
On Tue, Dec 24, 2024 at 5:13 AM Matthew Maurer <mmaurer@...gle.com> wrote:
>
> Generate both the existing modversions format and the new extended one
> when running modpost. Presence of this metadata in the final .ko is
> guarded by CONFIG_EXTENDED_MODVERSIONS.
>
> We no longer generate an error on long symbols in modpost if
> CONFIG_EXTENDED_MODVERSIONS is set, as they can now be appropriately
> encoded in the extended section. These symbols will be skipped in the
> previous encoding. An error will still be generated if
> CONFIG_EXTENDED_MODVERSIONS is not set.
>
> Reviewed-by: Sami Tolvanen <samitolvanen@...gle.com>
> Signed-off-by: Matthew Maurer <mmaurer@...gle.com>
> ---
> kernel/module/Kconfig | 10 ++++++++
> scripts/Makefile.modpost | 1 +
> scripts/mod/modpost.c | 65 +++++++++++++++++++++++++++++++++++++++++++++---
> 3 files changed, 72 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
> index d443fc504ffca0d1001f880ec496ab1f21fe979e..9568b629a03ce8289d3f3597eefc66fc96445720 100644
> --- a/kernel/module/Kconfig
> +++ b/kernel/module/Kconfig
> @@ -207,6 +207,16 @@ config ASM_MODVERSIONS
> assembly. This can be enabled only when the target architecture
> supports it.
>
> +config EXTENDED_MODVERSIONS
> + bool "Extended Module Versioning Support"
> + depends on MODVERSIONS
> + help
> + This enables extended MODVERSIONs support, allowing long symbol
> + names to be versioned.
> +
> + The most likely reason you would enable this is to enable Rust
> + support. If unsure, say N.
> +
> config MODULE_SRCVERSION_ALL
> bool "Source checksum for all modules"
> help
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index ab0e94ea62496e11dbaa3ffc289ce546862795ca..40426fc6350985780c0092beb49c6cc29b9eff62 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -43,6 +43,7 @@ MODPOST = $(objtree)/scripts/mod/modpost
> modpost-args = \
> $(if $(CONFIG_MODULES),-M) \
> $(if $(CONFIG_MODVERSIONS),-m) \
> + $(if $(CONFIG_EXTENDED_MODVERSIONS),-x) \
> $(if $(CONFIG_MODULE_SRCVERSION_ALL),-a) \
> $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E) \
> $(if $(KBUILD_MODPOST_WARN),-w) \
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index fb787a5715f5e8b37713162979ca45355770dc73..c58f08681fc1e37929333ba3a3e0989c4db5a79d 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -33,6 +33,8 @@ static bool module_enabled;
> static bool modversions;
> /* Is CONFIG_MODULE_SRCVERSION_ALL set? */
> static bool all_versions;
> +/* Is CONFIG_EXTENDED_MODVERSIONS set? */
> +static bool extended_modversions;
> /* If we are modposting external module set to 1 */
> static bool external_module;
> /* Only warn about unresolved symbols */
> @@ -1804,6 +1806,52 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)
> }
> }
>
> +/**
> + * Record CRCs for unresolved symbols, supporting long names
> + */
> +static void add_extended_versions(struct buffer *b, struct module *mod)
> +{
> + struct symbol *s;
> +
> + if (!extended_modversions)
> + return;
> +
> + buf_printf(b, "\n");
> + buf_printf(b, "static const s32 ____version_ext_crcs[]\n");
In general, 's32' (i.e. signed) and the '%x' printf-format (i.e. unsigned)
are an odd combination. I know why you are mixing up these.
Previously, we had a reason to use 's32' because the 'crc' field may
contain a negative address offset.
After commit 7b4537199a4a, there is no more reason
for keeping this weirdness.
I will apply the following first.
https://lore.kernel.org/linux-kbuild/20241228154603.2234284-2-masahiroy@kernel.org/T/#u
Then, you can change 's32' to 'u32' in this series.
> + buf_printf(b, "__used __section(\"__version_ext_crcs\") = {\n");
> + list_for_each_entry(s, &mod->unresolved_symbols, list) {
> + if (!s->module)
> + continue;
> + if (!s->crc_valid) {
> + /*
> + * We already warned on this when producing the legacy
> + * modversions table.
> + */
> + continue;
Personally, I tend to avoid a noise change because you will change this comment
to warn() in the next commit.
It is up to you.
> + }
> + buf_printf(b, "\t%#8x,\n", s->crc);
This is also a copy-paste of the existing code.
I like to have zero-padding for better alignment.
Please follow this format:
https://lore.kernel.org/linux-kbuild/20241228154603.2234284-2-masahiroy@kernel.org/T/#mc46376f41c0313c660990e92ed37689432f325c7
> + }
> + buf_printf(b, "};\n");
> +
> + buf_printf(b, "static const char ____version_ext_names[]\n");
> + buf_printf(b, "__used __section(\"__version_ext_names\") =\n");
> + list_for_each_entry(s, &mod->unresolved_symbols, list) {
> + if (!s->module)
> + continue;
> + if (!s->crc_valid) {
> + /*
> + * We already warned on this when producing the legacy
> + * modversions table.
> + * We need to skip its name too, as the indexes in
> + * both tables need to align.
> + */
> + continue;
> + }
Unnecessary braces.
Reason:
https://github.com/torvalds/linux/blob/v6.12/Documentation/process/coding-style.rst?plain=1#L197
> + buf_printf(b, "\t\"%s\\0\"\n", s->name);
> + }
> + buf_printf(b, ";\n");
> +}
> +
> /**
> * Record CRCs for unresolved symbols
> **/
> @@ -1827,9 +1875,14 @@ static void add_versions(struct buffer *b, struct module *mod)
> continue;
> }
> if (strlen(s->name) >= MODULE_NAME_LEN) {
> - error("too long symbol \"%s\" [%s.ko]\n",
> - s->name, mod->name);
> - break;
> + if (extended_modversions)
> + /* this symbol will only be in the extended info */
> + continue;
You need braces here.
Reason:
https://github.com/torvalds/linux/blob/v6.12/Documentation/process/coding-style.rst?plain=1#L213
--
Best Regards
Masahiro Yamada
Powered by blists - more mailing lists