Skip to content

Support Extended Modversions #270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

maurer
Copy link

@maurer maurer commented Dec 20, 2024

A companion to the EXTENDED_MODVERSIONS kernel patch series, this PR adds support for libkmod to access the new modversions format when present, falling back to the original format otherwise.

Tested with KDIR= override selecting a kernel with EXTENDED_MODVERSIONS=y and BASIC_MODVERSIONS=n enabled with the series above in addition to a normal existing kernel.

@stoeckmann
Copy link
Contributor

Noticed these issues while reviewing changes: #272

Since they are already in master, I have created a new PR. No need to slow down functional review if there are bugs in the underlying base. ;)

These PRs should be kept in sync, depending which one is merged first.

Copy link
Contributor

@lucasdemarchi lucasdemarchi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good to me. I will try to play with them soon to make sure I didn't miss anything. I left some comments on each patch.

0x........ debugfs_create_dir
0x........ debugfs_create_file
0x........ __x86_return_thunk
0x........ module_layout
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stoeckmann / @evelikov do you see a way to also test the extended format? Maybe we could:

  1. build the module with that config set
  2. extract the 2 sections and save it, base64-encoded

when building the kmods, we just add the sections to one of the modules with objcopy --add-section. Thoughts?

@maurer maurer force-pushed the extended-modversions branch 2 times, most recently from aba1927 to c7c2ab5 Compare January 3, 2025 18:38
maurer added 3 commits March 11, 2025 00:00
Makes `kmod_elf_get_dependency_symbols` dispatch to
`kmod_elf_get_modversions` rather than open-coding the same version
extraction. This helps avoid duplicating logic, especially as we add
support for the extended format.

Link: kmod-project#270
Signed-off-by: Lucas De Marchi <[email protected]>
In order to allow longer names, the kernel is taking a new MODVERSIONS
format option which stores the names in a strtab-like section and the
crcs in an array-like one. This patch makes `kmod_elf_get_modversions`
use either format, preferring the new one where present.

Signed-off-by: Matthew Maurer <[email protected]>
Link: kmod-project#270
Signed-off-by: Lucas De Marchi <[email protected]>
We can't test actual CRC values, because they may vary depending on
kernel configuration. We can ensure the correct symbol names show up
though.

Signed-off-by: Matthew Maurer <[email protected]>
Link: kmod-project#270
Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
Copy link
Contributor

@maurer the last commit with the additional testsuite patch breaks it because we don't have a way to test with that. I tried to replicate your env, but couldn't:

$ b4 shazam   [email protected]
Grabbing search results from lore.kernel.org
  Added from v11: 6 patches
  Added from v12: 6 patches
  Added from v13: 6 patches
...
Applying: module: get symbol crc back to unsigned
Patch failed at 0001 module: get symbol crc back to unsigned

@maurer
Copy link
Author

maurer commented Mar 16, 2025

It looks like debian:unstable worked, but nothing else did. It's re-using an existing test module and just checking other properties on it, so this most likely means that the test is failing because kernel doesn't even have CONFIG_MODVERSIONS enabled, nothing to do with CONFIG_EXTENDED_MODVERSIONS vs CONFIG_BASIC_MODVERSIONS. Do you have a preferred way to expect a test to go differently based on kernel configurations? The two ways I could see doing this are to either skip this test if CONFIG_MODVERSIONS is not enabled, or to run the test anyways, but if CONFIG_MODVERSIONS is not enabled, expect to fail to find version information.

As far as replicating my setup, the patches have landed upstream, so just grab a recent release and you'll see it, no need to b4 shazam any longer.

@lucasdemarchi
Copy link
Contributor

CONFIG_MODVERSIONS

how would you check for it? Running kernel may not be the tested kernel so it would require to check $KDIR/.config.

I think what we can do is to skip if the error is due to ENODATA instead of checking the config. We can do it easily with libkmod, but not easily with modprobe --dump-modversions.

As far as replicating my setup, the patches have landed upstream, so just grab a recent release and you'll see it, no need to b4 shazam any longer.

ok, thanks.

lucasdemarchi pushed a commit that referenced this pull request Apr 7, 2025
Makes `kmod_elf_get_dependency_symbols` dispatch to
`kmod_elf_get_modversions` rather than open-coding the same version
extraction. This helps avoid duplicating logic, especially as we add
support for the extended format.

Link: #270
Signed-off-by: Lucas De Marchi <[email protected]>
lucasdemarchi pushed a commit that referenced this pull request Apr 7, 2025
In order to allow longer names, the kernel is taking a new MODVERSIONS
format option which stores the names in a strtab-like section and the
crcs in an array-like one. This patch makes `kmod_elf_get_modversions`
use either format, preferring the new one where present.

Signed-off-by: Matthew Maurer <[email protected]>
Link: #270
Signed-off-by: Lucas De Marchi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants