* [PATCH v7 1/3] modules: Support extended MODVERSIONS info
2024-10-23 2:31 [PATCH v7 0/3] Extended MODVERSIONS Support Matthew Maurer
@ 2024-10-23 2:31 ` Matthew Maurer
2024-10-24 23:00 ` Sami Tolvanen
2024-10-25 11:14 ` Michael Ellerman
2024-10-23 2:31 ` [PATCH v7 2/3] modpost: Produce extended MODVERSIONS information Matthew Maurer
` (2 subsequent siblings)
3 siblings, 2 replies; 12+ messages in thread
From: Matthew Maurer @ 2024-10-23 2:31 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Madhavan Srinivasan, Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: linuxppc-dev, linux-kernel, linux-modules, linux-kbuild,
rust-for-linux, Matthew Maurer
Adds a new format for MODVERSIONS which stores each field in a separate
ELF section. This initially adds support for variable length names, but
could later be used to add additional fields to MODVERSIONS in a
backwards compatible way if needed. Any new fields will be ignored by
old user tooling, unlike the current format where user tooling cannot
tolerate adjustments to the format (for example making the name field
longer).
Since PPC munges its version records to strip leading dots, we reproduce
the munging for the new format. Other architectures do not appear to
have architecture-specific usage of this information.
Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
arch/powerpc/kernel/module_64.c | 24 ++++++++++-
kernel/module/internal.h | 11 +++++
kernel/module/main.c | 92 +++++++++++++++++++++++++++++++++++++----
kernel/module/version.c | 45 ++++++++++++++++++++
4 files changed, 162 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index e9bab599d0c2745e4d2b5cae04f2c56395c24654..02ada0b057cef6b2f29fa7519a5d52acac740ee5 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -355,6 +355,24 @@ static void dedotify_versions(struct modversion_info *vers,
}
}
+/* Same as normal versions, remove a leading dot if present. */
+static void dedotify_ext_version_names(char *str_seq, unsigned long size)
+{
+ unsigned long out = 0;
+ unsigned long in;
+ char last = '\0';
+
+ for (in = 0; in < size; in++) {
+ /* Skip one leading dot */
+ if (last == '\0' && str_seq[in] == '.')
+ in++;
+ last = str_seq[in];
+ str_seq[out++] = last;
+ }
+ /* Zero the trailing portion of the names table for robustness */
+ memset(&str_seq[out], 0, size - out);
+}
+
/*
* Undefined symbols which refer to .funcname, hack to funcname. Make .TOC.
* seem to be defined (value set later).
@@ -424,10 +442,12 @@ int module_frob_arch_sections(Elf64_Ehdr *hdr,
me->arch.toc_section = i;
if (sechdrs[i].sh_addralign < 8)
sechdrs[i].sh_addralign = 8;
- }
- else if (strcmp(secstrings+sechdrs[i].sh_name,"__versions")==0)
+ } else if (strcmp(secstrings + sechdrs[i].sh_name, "__versions") == 0)
dedotify_versions((void *)hdr + sechdrs[i].sh_offset,
sechdrs[i].sh_size);
+ else if (strcmp(secstrings + sechdrs[i].sh_name, "__version_ext_names") == 0)
+ dedotify_ext_version_names((void *)hdr + sechdrs[i].sh_offset,
+ sechdrs[i].sh_size);
if (sechdrs[i].sh_type == SHT_SYMTAB)
dedotify((void *)hdr + sechdrs[i].sh_offset,
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index daef2be8390222c22220e2f168baa8d35ad531b9..59959c21b205bf91c0073260885743098c4022cf 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -86,6 +86,8 @@ struct load_info {
unsigned int vers;
unsigned int info;
unsigned int pcpu;
+ unsigned int vers_ext_crc;
+ unsigned int vers_ext_name;
} index;
};
@@ -389,6 +391,15 @@ void module_layout(struct module *mod, struct modversion_info *ver, struct kerne
struct kernel_symbol *ks, struct tracepoint * const *tp);
int check_modstruct_version(const struct load_info *info, struct module *mod);
int same_magic(const char *amagic, const char *bmagic, bool has_crcs);
+struct modversion_info_ext {
+ size_t remaining;
+ const s32 *crc;
+ const char *name;
+};
+void modversion_ext_start(const struct load_info *info, struct modversion_info_ext *ver);
+void modversion_ext_advance(struct modversion_info_ext *ver);
+#define for_each_modversion_info_ext(ver, info) \
+ for (modversion_ext_start(info, &ver); ver.remaining > 0; modversion_ext_advance(&ver))
#else /* !CONFIG_MODVERSIONS */
static inline int check_version(const struct load_info *info,
const char *symname,
diff --git a/kernel/module/main.c b/kernel/module/main.c
index b40b632f00a65e66ed73c4b386ef1f323a5b790c..9a9feca344f8bb06408d350e13f759bb909962cd 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2039,6 +2039,82 @@ static int elf_validity_cache_index_str(struct load_info *info)
return 0;
}
+/**
+ * elf_validity_cache_index_versions() - Validate and cache version indices
+ * @info: Load info to cache version indices in.
+ * Must have &load_info->sechdrs and &load_info->secstrings populated.
+ * @flags: Load flags, relevant to suppress version loading, see
+ * uapi/linux/module.h
+ *
+ * If we're ignoring modversions based on @flags, zero all version indices
+ * and return validity. Othewrise check:
+ *
+ * * If "__version_ext_crcs" is present, "__version_ext_names" is present
+ * * There is a name present for every crc
+ *
+ * Then populate:
+ *
+ * * &load_info->index.vers
+ * * &load_info->index.vers_ext_crc
+ * * &load_info->index.vers_ext_names
+ *
+ * if present.
+ *
+ * Return: %0 if valid, %-ENOEXEC on failure.
+ */
+static int elf_validity_cache_index_versions(struct load_info *info, int flags)
+{
+ unsigned int vers_ext_crc;
+ unsigned int vers_ext_name;
+ size_t crc_count;
+ size_t remaining_len;
+ size_t name_size;
+ char *name;
+
+ /* If modversions were suppressed, pretend we didn't find any */
+ if (flags & MODULE_INIT_IGNORE_MODVERSIONS) {
+ info->index.vers = 0;
+ info->index.vers_ext_crc = 0;
+ info->index.vers_ext_name = 0;
+ return 0;
+ }
+
+ vers_ext_crc = find_sec(info, "__version_ext_crcs");
+ vers_ext_name = find_sec(info, "__version_ext_names");
+
+ /* If we have one field, we must have the other */
+ if (!!vers_ext_crc != !!vers_ext_name) {
+ pr_err("extended version crc+name presence does not match");
+ return -ENOEXEC;
+ }
+
+ /*
+ * If we have extended version information, we should have the same
+ * number of entries in every section.
+ */
+ if (vers_ext_crc) {
+ crc_count = info->sechdrs[vers_ext_crc].sh_size / sizeof(s32);
+ name = (void *)info->hdr +
+ info->sechdrs[vers_ext_name].sh_offset;
+ remaining_len = info->sechdrs[vers_ext_name].sh_size;
+
+ while (crc_count--) {
+ name_size = strnlen(name, remaining_len) + 1;
+ if (name_size > remaining_len) {
+ pr_err("more extended version crcs than names");
+ return -ENOEXEC;
+ }
+ remaining_len -= name_size;
+ name += name_size;
+ }
+ }
+
+ info->index.vers = find_sec(info, "__versions");
+ info->index.vers_ext_crc = vers_ext_crc;
+ info->index.vers_ext_name = vers_ext_name;
+ return 0;
+}
+
/**
* elf_validity_cache_index() - Resolve, validate, cache section indices
* @info: Load info to read from and update.
@@ -2053,9 +2129,7 @@ static int elf_validity_cache_index_str(struct load_info *info)
* * elf_validity_cache_index_mod()
* * elf_validity_cache_index_sym()
* * elf_validity_cache_index_str()
- *
- * If versioning is not suppressed via flags, load the version index from
- * a section called "__versions" with no validation.
+ * * elf_validity_cache_index_versions()
*
* If CONFIG_SMP is enabled, load the percpu section by name with no
* validation.
@@ -2078,11 +2152,9 @@ static int elf_validity_cache_index(struct load_info *info, int flags)
err = elf_validity_cache_index_str(info);
if (err < 0)
return err;
-
- if (flags & MODULE_INIT_IGNORE_MODVERSIONS)
- info->index.vers = 0; /* Pretend no __versions section! */
- else
- info->index.vers = find_sec(info, "__versions");
+ err = elf_validity_cache_index_versions(info, flags);
+ if (err < 0)
+ return err;
info->index.pcpu = find_pcpusec(info);
@@ -2293,6 +2365,10 @@ static int rewrite_section_headers(struct load_info *info, int flags)
/* Track but don't keep modinfo and version sections. */
info->sechdrs[info->index.vers].sh_flags &= ~(unsigned long)SHF_ALLOC;
+ info->sechdrs[info->index.vers_ext_crc].sh_flags &=
+ ~(unsigned long)SHF_ALLOC;
+ info->sechdrs[info->index.vers_ext_name].sh_flags &=
+ ~(unsigned long)SHF_ALLOC;
info->sechdrs[info->index.info].sh_flags &= ~(unsigned long)SHF_ALLOC;
return 0;
diff --git a/kernel/module/version.c b/kernel/module/version.c
index 53f43ac5a73e9d537a9e95ff97728a51fad0e797..c246d40879706d4f413fa7ea9bbe2264ea1b2aa8 100644
--- a/kernel/module/version.c
+++ b/kernel/module/version.c
@@ -19,11 +19,28 @@ int check_version(const struct load_info *info,
unsigned int versindex = info->index.vers;
unsigned int i, num_versions;
struct modversion_info *versions;
+ struct modversion_info_ext version_ext;
/* Exporting module didn't supply crcs? OK, we're already tainted. */
if (!crc)
return 1;
+ /* If we have extended version info, rely on it */
+ if (info->index.vers_ext_crc) {
+ for_each_modversion_info_ext(version_ext, info) {
+ if (strcmp(version_ext.name, symname) != 0)
+ continue;
+ if (*version_ext.crc == *crc)
+ return 1;
+ pr_debug("Found checksum %X vs module %X\n",
+ *crc, *version_ext.crc);
+ goto bad_version;
+ }
+ pr_warn_once("%s: no extended symbol version for %s\n",
+ info->name, symname);
+ return 1;
+ }
+
/* No versions at all? modprobe --force does this. */
if (versindex == 0)
return try_to_force_load(mod, symname) == 0;
@@ -87,6 +104,34 @@ int same_magic(const char *amagic, const char *bmagic,
return strcmp(amagic, bmagic) == 0;
}
+void modversion_ext_start(const struct load_info *info,
+ struct modversion_info_ext *start)
+{
+ unsigned int crc_idx = info->index.vers_ext_crc;
+ unsigned int name_idx = info->index.vers_ext_name;
+ Elf_Shdr *sechdrs = info->sechdrs;
+
+ /*
+ * Both of these fields are needed for this to be useful
+ * Any future fields should be initialized to NULL if absent.
+ */
+ if (crc_idx == 0 || name_idx == 0) {
+ start->remaining = 0;
+ return;
+ }
+
+ start->crc = (const s32 *)sechdrs[crc_idx].sh_addr;
+ start->name = (const char *)sechdrs[name_idx].sh_addr;
+ start->remaining = sechdrs[crc_idx].sh_size / sizeof(*start->crc);
+}
+
+void modversion_ext_advance(struct modversion_info_ext *vers)
+{
+ vers->remaining--;
+ vers->crc++;
+ vers->name += strlen(vers->name) + 1;
+}
+
/*
* Generate the signature for all relevant module structures here.
* If these change, we don't want to try to parse the module.
--
2.47.0.105.g07ac214952-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v7 1/3] modules: Support extended MODVERSIONS info
2024-10-23 2:31 ` [PATCH v7 1/3] modules: Support extended MODVERSIONS info Matthew Maurer
@ 2024-10-24 23:00 ` Sami Tolvanen
2024-10-25 11:14 ` Michael Ellerman
1 sibling, 0 replies; 12+ messages in thread
From: Sami Tolvanen @ 2024-10-24 23:00 UTC (permalink / raw)
To: Matthew Maurer
Cc: Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Madhavan Srinivasan, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
linuxppc-dev, linux-kernel, linux-modules, linux-kbuild,
rust-for-linux
On Wed, Oct 23, 2024 at 02:31:28AM +0000, Matthew Maurer wrote:
> Adds a new format for MODVERSIONS which stores each field in a separate
> ELF section. This initially adds support for variable length names, but
> could later be used to add additional fields to MODVERSIONS in a
> backwards compatible way if needed. Any new fields will be ignored by
> old user tooling, unlike the current format where user tooling cannot
> tolerate adjustments to the format (for example making the name field
> longer).
>
> Since PPC munges its version records to strip leading dots, we reproduce
> the munging for the new format. Other architectures do not appear to
> have architecture-specific usage of this information.
>
> Signed-off-by: Matthew Maurer <mmaurer@google.com>
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Sami
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v7 1/3] modules: Support extended MODVERSIONS info
2024-10-23 2:31 ` [PATCH v7 1/3] modules: Support extended MODVERSIONS info Matthew Maurer
2024-10-24 23:00 ` Sami Tolvanen
@ 2024-10-25 11:14 ` Michael Ellerman
2024-10-25 15:17 ` Matthew Maurer
1 sibling, 1 reply; 12+ messages in thread
From: Michael Ellerman @ 2024-10-25 11:14 UTC (permalink / raw)
To: Matthew Maurer, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Madhavan Srinivasan, Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: linuxppc-dev, linux-kernel, linux-modules, linux-kbuild,
rust-for-linux, Matthew Maurer
Matthew Maurer <mmaurer@google.com> writes:
> Adds a new format for MODVERSIONS which stores each field in a separate
> ELF section. This initially adds support for variable length names, but
> could later be used to add additional fields to MODVERSIONS in a
> backwards compatible way if needed. Any new fields will be ignored by
> old user tooling, unlike the current format where user tooling cannot
> tolerate adjustments to the format (for example making the name field
> longer).
>
> Since PPC munges its version records to strip leading dots, we reproduce
> the munging for the new format. Other architectures do not appear to
> have architecture-specific usage of this information.
>
> Signed-off-by: Matthew Maurer <mmaurer@google.com>
> ---
> arch/powerpc/kernel/module_64.c | 24 ++++++++++-
> kernel/module/internal.h | 11 +++++
> kernel/module/main.c | 92 +++++++++++++++++++++++++++++++++++++----
> kernel/module/version.c | 45 ++++++++++++++++++++
> 4 files changed, 162 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
> index e9bab599d0c2745e4d2b5cae04f2c56395c24654..02ada0b057cef6b2f29fa7519a5d52acac740ee5 100644
> --- a/arch/powerpc/kernel/module_64.c
> +++ b/arch/powerpc/kernel/module_64.c
> @@ -355,6 +355,24 @@ static void dedotify_versions(struct modversion_info *vers,
> }
> }
>
> +/* Same as normal versions, remove a leading dot if present. */
> +static void dedotify_ext_version_names(char *str_seq, unsigned long size)
> +{
> + unsigned long out = 0;
> + unsigned long in;
> + char last = '\0';
> +
> + for (in = 0; in < size; in++) {
> + /* Skip one leading dot */
> + if (last == '\0' && str_seq[in] == '.')
> + in++;
> + last = str_seq[in];
> + str_seq[out++] = last;
> + }
> + /* Zero the trailing portion of the names table for robustness */
> + memset(&str_seq[out], 0, size - out);
> +}
Sorry I realise it's version 7, but although the above looks correct it's
kind of dense.
I think the below would also work and is (I think) easier to follow, and
is more obviously similar to the existing code. I'm sure your version is
faster, but I don't think it's that performance critical.
static void dedotify_ext_version_names(char *str_seq, unsigned long size)
{
char *end = str_seq + size;
char *p = str_seq;
while (p < end) {
if (*p == '.')
memmove(p, p + 1, end - p - 1);
p += strlen(p) + 1;
}
}
The tail of str_seq will be filled with nulls as long as the last string
was null terminated.
cheers
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v7 1/3] modules: Support extended MODVERSIONS info
2024-10-25 11:14 ` Michael Ellerman
@ 2024-10-25 15:17 ` Matthew Maurer
2024-10-30 5:35 ` Michael Ellerman
0 siblings, 1 reply; 12+ messages in thread
From: Matthew Maurer @ 2024-10-25 15:17 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Madhavan Srinivasan, Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, linuxppc-dev, linux-kernel, linux-modules,
linux-kbuild, rust-for-linux
> Sorry I realise it's version 7, but although the above looks correct it's
> kind of dense.
>
> I think the below would also work and is (I think) easier to follow, and
> is more obviously similar to the existing code. I'm sure your version is
> faster, but I don't think it's that performance critical.
>
> static void dedotify_ext_version_names(char *str_seq, unsigned long size)
> {
> char *end = str_seq + size;
> char *p = str_seq;
>
> while (p < end) {
> if (*p == '.')
> memmove(p, p + 1, end - p - 1);
>
> p += strlen(p) + 1;
> }
> }
>
> The tail of str_seq will be filled with nulls as long as the last string
> was null terminated.
>
> cheers
As you alluded to, what you're providing is potentially O(n^2) in the
number of symbols a module depends on - the existing code is O(n).
If leading dots on names are rare, this is probably fine. If they're
common, this will potentially make loading modules with a large number
of imported symbols actually take a measurable amount of additional
time.
That said, I take your point about complexity, and trust you to know
your arch's inputs/requirements, so if I don't hear back again I will
incorporate that into the next revision of the patch (to be produced
after the gendwarfksyms update comes out).
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v7 1/3] modules: Support extended MODVERSIONS info
2024-10-25 15:17 ` Matthew Maurer
@ 2024-10-30 5:35 ` Michael Ellerman
0 siblings, 0 replies; 12+ messages in thread
From: Michael Ellerman @ 2024-10-30 5:35 UTC (permalink / raw)
To: Matthew Maurer
Cc: Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Madhavan Srinivasan, Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, linuxppc-dev, linux-kernel, linux-modules,
linux-kbuild, rust-for-linux
Matthew Maurer <mmaurer@google.com> writes:
>> Sorry I realise it's version 7, but although the above looks correct it's
>> kind of dense.
>>
>> I think the below would also work and is (I think) easier to follow, and
>> is more obviously similar to the existing code. I'm sure your version is
>> faster, but I don't think it's that performance critical.
>>
>> static void dedotify_ext_version_names(char *str_seq, unsigned long size)
>> {
>> char *end = str_seq + size;
>> char *p = str_seq;
>>
>> while (p < end) {
>> if (*p == '.')
>> memmove(p, p + 1, end - p - 1);
>>
>> p += strlen(p) + 1;
>> }
>> }
>>
>> The tail of str_seq will be filled with nulls as long as the last string
>> was null terminated.
>
> As you alluded to, what you're providing is potentially O(n^2) in the
> number of symbols a module depends on - the existing code is O(n).
> If leading dots on names are rare, this is probably fine. If they're
> common, this will potentially make loading modules with a large number
> of imported symbols actually take a measurable amount of additional
> time.
It should only be a single symbol these days, .TOC., for both big and
little endian builds.
But maybe someone out there is still building their kernel ELFv1, in
which case every function will begin with '.'.
I still don't think it will be measurable, but n^2 is asking for
trouble.
So forget it, just use your version, you've already written it anyway.
Sorry for the noise.
cheers
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v7 2/3] modpost: Produce extended MODVERSIONS information
2024-10-23 2:31 [PATCH v7 0/3] Extended MODVERSIONS Support Matthew Maurer
2024-10-23 2:31 ` [PATCH v7 1/3] modules: Support extended MODVERSIONS info Matthew Maurer
@ 2024-10-23 2:31 ` Matthew Maurer
2024-10-24 23:01 ` Sami Tolvanen
2024-10-23 2:31 ` [PATCH v7 3/3] rust: Use gendwarfksyms + extended modversions for CONFIG_MODVERSIONS Matthew Maurer
2024-10-24 23:07 ` [PATCH v7 0/3] Extended MODVERSIONS Support Sami Tolvanen
3 siblings, 1 reply; 12+ messages in thread
From: Matthew Maurer @ 2024-10-23 2:31 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Madhavan Srinivasan, Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: linuxppc-dev, linux-kernel, linux-modules, linux-kbuild,
rust-for-linux, Matthew Maurer
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.
Signed-off-by: Matthew Maurer <mmaurer@google.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 e6b2427e5c190aacf7b9c5c1bb57fca39d311564..a31c617cd67d3d66b24d2fba34cbd5cc9c53ab78 100644
--- a/kernel/module/Kconfig
+++ b/kernel/module/Kconfig
@@ -208,6 +208,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 44936ebad161e914cbcc40ac74a2d651596d7b07..765da63d592be56fe93c0f4a35f1bfbcb924541a 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -43,6 +43,7 @@ MODPOST = 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 107393a8c48a5993dbe456702fec0652a967ee86..bd38f33fd41fbd98bce34f8924b2fb0ac04297ee 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -32,6 +32,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 */
@@ -1817,6 +1819,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");
+ 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;
+ }
+ buf_printf(b, "\t%#8x,\n", s->crc);
+ }
+ 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;
+ }
+ buf_printf(b, "\t\"%s\\0\"\n", s->name);
+ }
+ buf_printf(b, ";\n");
+}
+
/**
* Record CRCs for unresolved symbols
**/
@@ -1840,9 +1888,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;
+ else {
+ error("too long symbol \"%s\" [%s.ko]\n",
+ s->name, mod->name);
+ break;
+ }
}
buf_printf(b, "\t{ %#8x, \"%s\" },\n",
s->crc, s->name);
@@ -1972,6 +2025,7 @@ static void write_mod_c_file(struct module *mod)
add_header(&buf, mod);
add_exported_symbols(&buf, mod);
add_versions(&buf, mod);
+ add_extended_versions(&buf, mod);
add_depends(&buf, mod);
add_moddevtable(&buf, mod);
add_srcversion(&buf, mod);
@@ -2130,7 +2184,7 @@ int main(int argc, char **argv)
LIST_HEAD(dump_lists);
struct dump_list *dl, *dl2;
- while ((opt = getopt(argc, argv, "ei:MmnT:to:au:WwENd:")) != -1) {
+ while ((opt = getopt(argc, argv, "ei:MmnT:to:au:WwENd:x")) != -1) {
switch (opt) {
case 'e':
external_module = true;
@@ -2179,6 +2233,9 @@ int main(int argc, char **argv)
case 'd':
missing_namespace_deps = optarg;
break;
+ case 'x':
+ extended_modversions = true;
+ break;
default:
exit(1);
}
--
2.47.0.105.g07ac214952-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v7 2/3] modpost: Produce extended MODVERSIONS information
2024-10-23 2:31 ` [PATCH v7 2/3] modpost: Produce extended MODVERSIONS information Matthew Maurer
@ 2024-10-24 23:01 ` Sami Tolvanen
0 siblings, 0 replies; 12+ messages in thread
From: Sami Tolvanen @ 2024-10-24 23:01 UTC (permalink / raw)
To: Matthew Maurer
Cc: Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Madhavan Srinivasan, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
linuxppc-dev, linux-kernel, linux-modules, linux-kbuild,
rust-for-linux
On Wed, Oct 23, 2024 at 02:31:29AM +0000, Matthew Maurer 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.
>
> Signed-off-by: Matthew Maurer <mmaurer@google.com>
Thanks for fixing this, LGTM.
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Sami
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v7 3/3] rust: Use gendwarfksyms + extended modversions for CONFIG_MODVERSIONS
2024-10-23 2:31 [PATCH v7 0/3] Extended MODVERSIONS Support Matthew Maurer
2024-10-23 2:31 ` [PATCH v7 1/3] modules: Support extended MODVERSIONS info Matthew Maurer
2024-10-23 2:31 ` [PATCH v7 2/3] modpost: Produce extended MODVERSIONS information Matthew Maurer
@ 2024-10-23 2:31 ` Matthew Maurer
2024-10-24 23:06 ` Sami Tolvanen
2024-10-24 23:07 ` [PATCH v7 0/3] Extended MODVERSIONS Support Sami Tolvanen
3 siblings, 1 reply; 12+ messages in thread
From: Matthew Maurer @ 2024-10-23 2:31 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Madhavan Srinivasan, Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: linuxppc-dev, linux-kernel, linux-modules, linux-kbuild,
rust-for-linux, Matthew Maurer
From: Sami Tolvanen <samitolvanen@google.com>
Previously, two things stopped Rust from using MODVERSIONS:
1. Rust symbols are occasionally too long to be represented in the
original versions table
2. Rust types cannot be properly hashed by the existing genksyms
approach because:
* Looking up type definitions in Rust is more complex than C
* Type layout is potentially dependent on the compiler in Rust,
not just the source type declaration.
CONFIG_EXTENDED_MODVERSIONS addresses the first point, and
CONFIG_GENDWARFKSYMS the second. If Rust wants to use MODVERSIONS, allow
it to do so by selecting both features.
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
Co-developed-by: Matthew Maurer <mmaurer@google.com>
Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
init/Kconfig | 3 ++-
rust/Makefile | 32 ++++++++++++++++++++++++++++++--
2 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/init/Kconfig b/init/Kconfig
index 530a382ee0feb391b4717abdba3672e584a462d0..f5cce579f29b2ed89e97f8075a3bf70e32e71ad0 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1942,7 +1942,8 @@ config RUST
bool "Rust support"
depends on HAVE_RUST
depends on RUST_IS_AVAILABLE
- depends on !MODVERSIONS
+ select EXTENDED_MODVERSIONS if MODVERSIONS
+ depends on (GENDWARFKSYMS || !MODVERSIONS)
depends on !GCC_PLUGIN_RANDSTRUCT
depends on !RANDSTRUCT
depends on !DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE
diff --git a/rust/Makefile b/rust/Makefile
index b5e0a73b78f3e58fc8fb8c9fab8fb5792406c6d8..b80bc4eb98202f774c493da89c2caee322cffc91 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -303,10 +303,11 @@ $(obj)/bindings/bindings_helpers_generated.rs: private bindgen_target_extra = ;
$(obj)/bindings/bindings_helpers_generated.rs: $(src)/helpers/helpers.c FORCE
$(call if_changed_dep,bindgen)
+rust_exports = $(NM) -p --defined-only $(1) | awk '$$2~/(T|R|D|B)/ && $$3!~/__cfi/ { printf $(2),$(3) }'
+
quiet_cmd_exports = EXPORTS $@
cmd_exports = \
- $(NM) -p --defined-only $< \
- | awk '$$2~/(T|R|D|B)/ && $$3!~/__cfi/ {printf "EXPORT_SYMBOL_RUST_GPL(%s);\n",$$3}' > $@
+ $(call rust_exports,$<,"EXPORT_SYMBOL_RUST_GPL(%s);\n",$$3) > $@
$(obj)/exports_core_generated.h: $(obj)/core.o FORCE
$(call if_changed,exports)
@@ -378,11 +379,35 @@ ifneq ($(or $(CONFIG_ARM64),$(and $(CONFIG_RISCV),$(CONFIG_64BIT))),)
__ashlti3 __lshrti3
endif
+ifdef CONFIG_MODVERSIONS
+cmd_gendwarfksyms = $(if $(skip_gendwarfksyms),, \
+ $(call rust_exports,$@,"%s\n",$$3) | \
+ scripts/gendwarfksyms/gendwarfksyms \
+ $(if $(KBUILD_SYMTYPES), --symtypes $(@:.o=.symtypes),) \
+ $@ >> $(dot-target).cmd)
+endif
+
define rule_rustc_library
$(call cmd_and_fixdep,rustc_library)
$(call cmd,gen_objtooldep)
+ $(call cmd,gendwarfksyms)
endef
+define rule_rust_cc_library
+ $(call if_changed_rule,cc_o_c)
+ $(call cmd,force_checksrc)
+ $(call cmd,gendwarfksyms)
+endef
+
+# helpers.o uses the same export mechanism as Rust libraries, so ensure symbol
+# versions are calculated for the helpers too.
+$(obj)/helpers/helpers.o: $(src)/helpers/helpers.c $(recordmcount_source) FORCE
+ +$(call if_changed_rule,rust_cc_library)
+
+# Disable symbol versioning for exports.o to avoid conflicts with the actual
+# symbol versions generated from Rust objects.
+$(obj)/exports.o: private skip_gendwarfksyms = 1
+
$(obj)/core.o: private skip_clippy = 1
$(obj)/core.o: private skip_flags = -Wunreachable_pub
$(obj)/core.o: private rustc_objcopy = $(foreach sym,$(redirect-intrinsics),--redefine-sym $(sym)=__rust$(sym))
@@ -394,6 +419,7 @@ ifneq ($(or $(CONFIG_X86_64),$(CONFIG_X86_32)),)
$(obj)/core.o: scripts/target.json
endif
+$(obj)/compiler_builtins.o: private skip_gendwarfksyms = 1
$(obj)/compiler_builtins.o: private rustc_objcopy = -w -W '__*'
$(obj)/compiler_builtins.o: $(src)/compiler_builtins.rs $(obj)/core.o FORCE
+$(call if_changed_rule,rustc_library)
@@ -404,6 +430,7 @@ $(obj)/alloc.o: private rustc_target_flags = $(alloc-cfgs)
$(obj)/alloc.o: $(RUST_LIB_SRC)/alloc/src/lib.rs $(obj)/compiler_builtins.o FORCE
+$(call if_changed_rule,rustc_library)
+$(obj)/build_error.o: private skip_gendwarfksyms = 1
$(obj)/build_error.o: $(src)/build_error.rs $(obj)/compiler_builtins.o FORCE
+$(call if_changed_rule,rustc_library)
@@ -413,6 +440,7 @@ $(obj)/bindings.o: $(src)/bindings/lib.rs \
$(obj)/bindings/bindings_helpers_generated.rs FORCE
+$(call if_changed_rule,rustc_library)
+$(obj)/uapi.o: private skip_gendwarfksyms = 1
$(obj)/uapi.o: $(src)/uapi/lib.rs \
$(obj)/compiler_builtins.o \
$(obj)/uapi/uapi_generated.rs FORCE
--
2.47.0.105.g07ac214952-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v7 3/3] rust: Use gendwarfksyms + extended modversions for CONFIG_MODVERSIONS
2024-10-23 2:31 ` [PATCH v7 3/3] rust: Use gendwarfksyms + extended modversions for CONFIG_MODVERSIONS Matthew Maurer
@ 2024-10-24 23:06 ` Sami Tolvanen
0 siblings, 0 replies; 12+ messages in thread
From: Sami Tolvanen @ 2024-10-24 23:06 UTC (permalink / raw)
To: Matthew Maurer
Cc: Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Madhavan Srinivasan, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
linuxppc-dev, linux-kernel, linux-modules, linux-kbuild,
rust-for-linux
On Wed, Oct 23, 2024 at 02:31:30AM +0000, Matthew Maurer wrote:
> From: Sami Tolvanen <samitolvanen@google.com>
>
> Previously, two things stopped Rust from using MODVERSIONS:
> 1. Rust symbols are occasionally too long to be represented in the
> original versions table
> 2. Rust types cannot be properly hashed by the existing genksyms
> approach because:
> * Looking up type definitions in Rust is more complex than C
> * Type layout is potentially dependent on the compiler in Rust,
> not just the source type declaration.
>
> CONFIG_EXTENDED_MODVERSIONS addresses the first point, and
> CONFIG_GENDWARFKSYMS the second. If Rust wants to use MODVERSIONS, allow
> it to do so by selecting both features.
>
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> Co-developed-by: Matthew Maurer <mmaurer@google.com>
> Signed-off-by: Matthew Maurer <mmaurer@google.com>
> ---
> init/Kconfig | 3 ++-
> rust/Makefile | 32 ++++++++++++++++++++++++++++++--
> 2 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 530a382ee0feb391b4717abdba3672e584a462d0..f5cce579f29b2ed89e97f8075a3bf70e32e71ad0 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1942,7 +1942,8 @@ config RUST
> bool "Rust support"
> depends on HAVE_RUST
> depends on RUST_IS_AVAILABLE
> - depends on !MODVERSIONS
> + select EXTENDED_MODVERSIONS if MODVERSIONS
> + depends on (GENDWARFKSYMS || !MODVERSIONS)
Nit: `depends on !MODVERSIONS || GENDWARFKSYMS` would match the
convention used below.
> +cmd_gendwarfksyms = $(if $(skip_gendwarfksyms),, \
> + $(call rust_exports,$@,"%s\n",$$3) | \
> + scripts/gendwarfksyms/gendwarfksyms \
> + $(if $(KBUILD_SYMTYPES), --symtypes $(@:.o=.symtypes),) \
> + $@ >> $(dot-target).cmd)
Note that Petr suggested adding a KBUILD_ flag for passing the
--stable parameter to gendwarfksyms, which I think would be useful,
so once I send out v5 of that series, we'll have to update this to
also use the parameter when processing Rust objects:
https://lore.kernel.org/linux-modules/8017c328-f039-46c3-b472-f203cd6e2cfe@suse.com/
Sami
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v7 0/3] Extended MODVERSIONS Support
2024-10-23 2:31 [PATCH v7 0/3] Extended MODVERSIONS Support Matthew Maurer
` (2 preceding siblings ...)
2024-10-23 2:31 ` [PATCH v7 3/3] rust: Use gendwarfksyms + extended modversions for CONFIG_MODVERSIONS Matthew Maurer
@ 2024-10-24 23:07 ` Sami Tolvanen
2024-10-25 4:53 ` Luis Chamberlain
3 siblings, 1 reply; 12+ messages in thread
From: Sami Tolvanen @ 2024-10-24 23:07 UTC (permalink / raw)
To: Matthew Maurer
Cc: Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Madhavan Srinivasan, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
linuxppc-dev, linux-kernel, linux-modules, linux-kbuild,
rust-for-linux
On Wed, Oct 23, 2024 at 02:31:27AM +0000, Matthew Maurer wrote:
> This patch series is intended for use alongside the Implement DWARF
> modversions series [1] to enable RUST and MODVERSIONS at the same
> time.
>
> Elsewhere, we've seen a desire for long symbol name support for LTO
> symbol names [2], and the previous series came up [3] as a possible
> solution rather than hashing, which some have objected [4] to.
>
> This series adds a MODVERSIONS format which uses a section per column.
> This avoids userspace tools breaking if we need to make a similar change
> to the format in the future - we would do so by adding a new section,
> rather than editing the struct definition. In the new format, the name
> section is formatted as a concatenated sequence of NUL-terminated
> strings, which allows for arbitrary length names.
>
> Emitting the extended format is guarded by CONFIG_EXTENDED_MODVERSIONS,
> but the kernel always knows how to validate both the original and
> extended formats.
>
> Selecting RUST and MODVERSIONS is now possible if GENDWARFKSYMS is
> selected, and will implicitly select EXTENDED_MODVERSIONS.
>
> This series depends upon the module verification refactor patches [5]
> that were split off of v5, and DWARF-based versions [1].
>
> linuxppc-dev is requested to look at the ppc-specific munging,
> as Luis would like some eyes on there [6].
>
> [1] https://lore.kernel.org/lkml/20241008183823.36676-21-samitolvanen@google.com/
> [2] https://lore.kernel.org/lkml/20240605032120.3179157-1-song@kernel.org/
> [3] https://lore.kernel.org/lkml/ZoxbEEsK40ASi1cY@bombadil.infradead.org/
> [4] https://lore.kernel.org/lkml/0b2697fd-7ab4-469f-83a6-ec9ebc701ba0@suse.com/
> [5] https://lore.kernel.org/linux-modules/20241015231651.3851138-1-mmaurer@google.com/T/#t
> [6] https://lore.kernel.org/lkml/ZxahDv5ZKdM__0sZ@bombadil.infradead.org/
>
> Changes in v7:
> - Fix modpost to detect EXTENDED_MODVERSIONS based on a flag
> - Drop patches to fix export_report.pl
Unless Luis prefers to deal with this separately, perhaps you can
just include a patch to delete export_report.pl in this series?
Sami
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v7 0/3] Extended MODVERSIONS Support
2024-10-24 23:07 ` [PATCH v7 0/3] Extended MODVERSIONS Support Sami Tolvanen
@ 2024-10-25 4:53 ` Luis Chamberlain
0 siblings, 0 replies; 12+ messages in thread
From: Luis Chamberlain @ 2024-10-25 4:53 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Matthew Maurer, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Madhavan Srinivasan, Petr Pavlu,
Daniel Gomez, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, linuxppc-dev, linux-kernel, linux-modules,
linux-kbuild, rust-for-linux
On Thu, Oct 24, 2024 at 11:07:57PM +0000, Sami Tolvanen wrote:
> > Changes in v7:
> > - Fix modpost to detect EXTENDED_MODVERSIONS based on a flag
> > - Drop patches to fix export_report.pl
>
> Unless Luis prefers to deal with this separately, perhaps you can
> just include a patch to delete export_report.pl in this series?
Yes, please do the homework to check the history of export_report.pl
and just nuke it if we don't need it. Ignoring it just because it may
not be used is not the right thing.
If you don't find the history on the current tree we have the
history.git tree [0] I often use to go down memory lane pre-git.
[0] https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/
Luis
^ permalink raw reply [flat|nested] 12+ messages in thread