| Message ID | 20240925233854.90072-1-mmaurer@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Extended MODVERSIONS Support | expand |
Hi Matt, On Wed, Sep 25, 2024 at 11:39 PM Matthew Maurer <mmaurer@google.com> wrote: > > This patch series is intended for use alongside the Implement > MODVERSIONS for RUST [1] series as a replacement for the symbol name > hashing approach used there 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. > > Currently, this series emits both the extended format and the current > format on all modules, and prefers the extended format when checking if > present. I'm open to various other policies via Kconfig knobs, but this > seemed like a good initial default. > > The refactor to MODVERSIONS is prefixed to this series as result of an > explicit request [5] by Luis in response to the original patchset. > > If you are testing this patch alongside RUST by manually removing the > !MODVERSIONS restriction (this series doesn't remove it, because the > CRCs don't mean what we'd want them to yet, we need the DWARF patch for > that) and have kernel hardening enabled, you may need the CPU > Mitigations [6] series. Without it, the foo.mod.o file produced by the > C compiler will reference __x86_return_thunk, but foo.o will not. > This means that the version table will not contain a version for > __x86_return_thunk, but foo.ko will reference it, which will result > in a version check failure. > > [1] https://lore.kernel.org/all/20240617175818.58219-17-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/lkml/ZVZNh%2FPA5HiVRkeb@bombadil.infradead.org/ > [6] https://lore.kernel.org/all/20240725183325.122827-1-ojeda@kernel.org/ > > Changes in v5: > - Addresses Sami's comments from v3 that I missed in v4 (missing early > return, extra parens) v5 looks good to me, thank you for fixing these issues. I tested this with my latest Rust modversions patches [1] and everything seems to work for me (on x86_64, arm64, and riscv64 w/ clang). For the series: Reviewed-by: Sami Tolvanen <samitolvanen@google.com> Tested-by: Sami Tolvanen <samitolvanen@google.com> [1] https://github.com/samitolvanen/linux/commits/rustmodversions-v3 Sami
On Wed, Sep 25, 2024 at 7:39 PM Matthew Maurer <mmaurer@google.com> wrote: > > This patch series is intended for use alongside the Implement > MODVERSIONS for RUST [1] series as a replacement for the symbol name > hashing approach used there 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. > > Currently, this series emits both the extended format and the current > format on all modules, and prefers the extended format when checking if > present. I'm open to various other policies via Kconfig knobs, but this > seemed like a good initial default. > > The refactor to MODVERSIONS is prefixed to this series as result of an > explicit request [5] by Luis in response to the original patchset. > > If you are testing this patch alongside RUST by manually removing the > !MODVERSIONS restriction (this series doesn't remove it, because the > CRCs don't mean what we'd want them to yet, we need the DWARF patch for > that) and have kernel hardening enabled, you may need the CPU > Mitigations [6] series. Without it, the foo.mod.o file produced by the > C compiler will reference __x86_return_thunk, but foo.o will not. > This means that the version table will not contain a version for > __x86_return_thunk, but foo.ko will reference it, which will result > in a version check failure. > > [1] https://lore.kernel.org/all/20240617175818.58219-17-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/lkml/ZVZNh%2FPA5HiVRkeb@bombadil.infradead.org/ > [6] https://lore.kernel.org/all/20240725183325.122827-1-ojeda@kernel.org/ > > Changes in v5: > - Addresses Sami's comments from v3 that I missed in v4 (missing early > return, extra parens) > > v4: https://lore.kernel.org/asahi/20240924212024.540574-1-mmaurer@google.com/ > - Fix incorrect dot munging in PPC > > v3: https://lore.kernel.org/lkml/87le0w2hop.fsf@mail.lhotse/T/ > - Split up the module verification refactor into smaller patches, per > Greg K-H's suggestion. > > v2: https://lore.kernel.org/all/20231118025748.2778044-1-mmaurer@google.com/ > - Add loading/verification refactor before modifying, per Luis's request > > v1: https://lore.kernel.org/rust-for-linux/20231115185858.2110875-1-mmaurer@google.com/ > > Matthew Maurer (16): > module: Take const arg in validate_section_offset > module: Factor out elf_validity_ehdr > module: Factor out elf_validity_cache_sechdrs > module: Factor out elf_validity_cache_secstrings > module: Factor out elf_validity_cache_index_info > module: Factor out elf_validity_cache_index_mod > module: Factor out elf_validity_cache_index_sym > module: Factor out elf_validity_cache_index_str > module: Group section index calculations together > module: Factor out elf_validity_cache_strtab > module: Additional validation in elf_validity_cache_strtab > module: Reformat struct for code style > export_report: Rehabilitate script > modules: Support extended MODVERSIONS info > modpost: Produce extended modversion information > export_report: Use new version info format > > arch/powerpc/kernel/module_64.c | 23 +- > kernel/module/internal.h | 18 +- > kernel/module/main.c | 647 ++++++++++++++++++++++++-------- > kernel/module/version.c | 45 +++ > scripts/export_report.pl | 17 +- > scripts/mod/modpost.c | 39 +- > 6 files changed, 628 insertions(+), 161 deletions(-) > > -- > 2.46.1.824.gd892dcdcdd-goog > > The series looks straightforward and reasonable to me. Acked-by: Neal Gompa <neal@gompa.dev> -- 真実はいつも一つ！/ Always, there's only one truth!