From 4e79a4674b7da7a23a143326eaf2c04719207cb9 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Fri, 3 Jan 2025 18:07:08 +0000 Subject: [PATCH 1/3] libkmod: Centralize modversions parsing 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: https://github.com/kmod-project/kmod/pull/270 Signed-off-by: Lucas De Marchi --- libkmod/libkmod-elf.c | 105 ++++++++++++++++-------------------------- 1 file changed, 40 insertions(+), 65 deletions(-) diff --git a/libkmod/libkmod-elf.c b/libkmod/libkmod-elf.c index f215f1ee..a1101ac9 100644 --- a/libkmod/libkmod-elf.c +++ b/libkmod/libkmod-elf.c @@ -945,25 +945,18 @@ int kmod_elf_get_symbols(const struct kmod_elf *elf, struct kmod_modversion **ar return kmod_elf_get_symbols_symtab(elf, array); } -static int kmod_elf_crc_find(const struct kmod_elf *elf, uint64_t off, - uint64_t versionslen, const char *name, uint64_t *crc) +static int kmod_modversion_crc_find(const struct kmod_modversion *vers, size_t count, + const char *name, uint64_t *crc) { - size_t namlen, verlen, crclen; - uint64_t i; - - elf_get_modversion_lengths(elf, &verlen, &crclen, &namlen); + size_t i; - for (i = 0; i < versionslen; i += verlen) { - const char *symbol = elf_get_mem(elf, off + i + crclen); - if (strnlen(symbol, namlen) == namlen || !streq(name, symbol)) { - ELFDBG(elf, "symbol name at index %" PRIu64 " too long\n", i); - continue; + for (i = 0; i < count; i++) { + if (!strcmp(vers[i].symbol, name)) { + *crc = vers[i].crc; + return i; } - *crc = elf_get_uint(elf, off + i, crclen); - return i / verlen; } - ELFDBG(elf, "could not find crc for symbol '%s'\n", name); *crc = 0; return -1; } @@ -977,33 +970,15 @@ static int kmod_elf_crc_find(const struct kmod_elf *elf, uint64_t off, int kmod_elf_get_dependency_symbols(const struct kmod_elf *elf, struct kmod_modversion **array) { - uint64_t versionslen, strtablen, symtablen, str_off, sym_off, ver_off; + uint64_t strtablen, symtablen, str_off, sym_off; uint64_t str_sec_off, sym_sec_off; - struct kmod_modversion *a; - size_t i, count, namlen, vercount, verlen, symcount, symlen, crclen; + struct kmod_modversion *a, *version_array; + size_t i, count, symcount, symlen, vercount; + int verres; bool handle_register_symbols; uint8_t *visited_versions; uint64_t *symcrcs; - ver_off = elf->sections[KMOD_ELF_SECTION_VERSIONS].offset; - versionslen = elf->sections[KMOD_ELF_SECTION_VERSIONS].size; - if (ver_off == 0) { - versionslen = 0; - verlen = 0; - crclen = 0; - namlen = 0; - } else { - elf_get_modversion_lengths(elf, &verlen, &crclen, &namlen); - if (versionslen % verlen != 0) { - ELFDBG(elf, - "unexpected __versions of length %" PRIu64 - ", not multiple of %zu as expected.\n", - versionslen, verlen); - ver_off = 0; - versionslen = 0; - } - } - str_sec_off = elf->sections[KMOD_ELF_SECTION_STRTAB].offset; strtablen = elf->sections[KMOD_ELF_SECTION_STRTAB].size; if (str_sec_off == 0) { @@ -1031,14 +1006,27 @@ int kmod_elf_get_dependency_symbols(const struct kmod_elf *elf, return -EINVAL; } - if (versionslen == 0) { - vercount = 0; + version_array = NULL; + verres = kmod_elf_get_modversions(elf, &version_array); + if (verres < 0) { + if (verres == -ENODATA) + /* we allow no MODVERSIONS */ + vercount = 0; + else + /* but error out on bugged MODVERSIONS */ + return verres; + } else { + vercount = verres; + } + + if (vercount == 0) { visited_versions = NULL; } else { - vercount = versionslen / verlen; visited_versions = calloc(vercount, sizeof(uint8_t)); - if (visited_versions == NULL) + if (visited_versions == NULL) { + free(version_array); return -ENOMEM; + } } handle_register_symbols = @@ -1052,6 +1040,7 @@ int kmod_elf_get_dependency_symbols(const struct kmod_elf *elf, symcrcs = calloc(symcount, sizeof(uint64_t)); if (symcrcs == NULL) { free(visited_versions); + free(version_array); return -ENOMEM; } @@ -1105,6 +1094,7 @@ int kmod_elf_get_dependency_symbols(const struct kmod_elf *elf, ".\n", strtablen, i, name_off); free(visited_versions); + free(version_array); free(symcrcs); return -EINVAL; } @@ -1117,32 +1107,20 @@ int kmod_elf_get_dependency_symbols(const struct kmod_elf *elf, count++; - idx = kmod_elf_crc_find(elf, ver_off, versionslen, name, &crc); + idx = kmod_modversion_crc_find(version_array, vercount, name, &crc); if (idx >= 0 && visited_versions != NULL) visited_versions[idx] = 1; + else + ELFDBG(elf, "could not find crc for symbol '%s'\n", name); symcrcs[i] = crc; } if (visited_versions != NULL) { /* module_layout/struct_module are not visited, but needed */ for (i = 0; i < vercount; i++) { - if (visited_versions[i] == 0) { - const char *name; - size_t nlen; - - name = elf_get_mem(elf, ver_off + i * verlen + crclen); - nlen = strnlen(name, namlen); - - if (nlen == namlen) { - ELFDBG(elf, "symbol name at index %zu too long\n", - i); - free(visited_versions); - free(symcrcs); - return -EINVAL; - } - + if (visited_versions[i] == 0) + /* no verification; version_array handled it */ count++; - } } } @@ -1156,6 +1134,7 @@ int kmod_elf_get_dependency_symbols(const struct kmod_elf *elf, if (count == 0) { free(visited_versions); + free(version_array); free(symcrcs); *array = NULL; return 0; @@ -1164,6 +1143,7 @@ int kmod_elf_get_dependency_symbols(const struct kmod_elf *elf, *array = a = malloc(sizeof(struct kmod_modversion) * count); if (*array == NULL) { free(visited_versions); + free(version_array); free(symcrcs); return -errno; } @@ -1244,21 +1224,16 @@ int kmod_elf_get_dependency_symbols(const struct kmod_elf *elf, /* add unvisited (module_layout/struct_module) */ for (i = 0; i < vercount; i++) { - const char *name; - uint64_t crc; - if (visited_versions[i] != 0) continue; - name = elf_get_mem(elf, ver_off + i * verlen + crclen); - crc = elf_get_uint(elf, ver_off + i * verlen, crclen); - - a[count].crc = crc; + a[count].crc = version_array[i].crc; a[count].bind = KMOD_SYMBOL_UNDEF; - a[count].symbol = name; + a[count].symbol = version_array[i].symbol; count++; } free(visited_versions); + free(version_array); return count; } From ffe6c678b45e456bfb01d5e58ee2902dadfd71b3 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Thu, 19 Dec 2024 22:55:30 +0000 Subject: [PATCH 2/3] libkmod: Support EXTENDED_MODVERSIONS 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 Link: https://github.com/kmod-project/kmod/pull/270 Signed-off-by: Lucas De Marchi --- libkmod/libkmod-elf.c | 90 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 86 insertions(+), 4 deletions(-) diff --git a/libkmod/libkmod-elf.c b/libkmod/libkmod-elf.c index a1101ac9..2c9c9b0e 100644 --- a/libkmod/libkmod-elf.c +++ b/libkmod/libkmod-elf.c @@ -33,6 +33,8 @@ enum kmod_elf_section { KMOD_ELF_SECTION_STRTAB, KMOD_ELF_SECTION_SYMTAB, KMOD_ELF_SECTION_VERSIONS, + KMOD_ELF_SECTION_VERSION_EXT_NAMES, + KMOD_ELF_SECTION_VERSION_EXT_CRCS, KMOD_ELF_SECTION_MAX, }; @@ -42,6 +44,8 @@ static const char *const section_name_map[] = { [KMOD_ELF_SECTION_STRTAB] = ".strtab", [KMOD_ELF_SECTION_SYMTAB] = ".symtab", [KMOD_ELF_SECTION_VERSIONS] = "__versions", + [KMOD_ELF_SECTION_VERSION_EXT_NAMES] = "__version_ext_names", + [KMOD_ELF_SECTION_VERSION_EXT_CRCS] = "__version_ext_crcs", }; struct kmod_elf { @@ -538,8 +542,8 @@ static inline void elf_get_modversion_lengths(const struct kmod_elf *elf, size_t } } -/* array will be allocated with strings in a single malloc, just free *array */ -int kmod_elf_get_modversions(const struct kmod_elf *elf, struct kmod_modversion **array) +static int kmod_elf_get_basic_modversions(const struct kmod_elf *elf, + struct kmod_modversion **array) { size_t i, count, crclen, namlen, verlen; uint64_t off, sec_off, size; @@ -547,8 +551,6 @@ int kmod_elf_get_modversions(const struct kmod_elf *elf, struct kmod_modversion elf_get_modversion_lengths(elf, &verlen, &crclen, &namlen); - *array = NULL; - sec_off = elf->sections[KMOD_ELF_SECTION_VERSIONS].offset; size = elf->sections[KMOD_ELF_SECTION_VERSIONS].size; if (sec_off == 0) @@ -591,6 +593,86 @@ int kmod_elf_get_modversions(const struct kmod_elf *elf, struct kmod_modversion return count; } +static int kmod_elf_get_extended_modversions(const struct kmod_elf *elf, + struct kmod_modversion **array) +{ + uint64_t name_off, name_size, crc_off, crc_size; + const char *name_cursor; + int count, i, ret; + + name_off = elf->sections[KMOD_ELF_SECTION_VERSION_EXT_NAMES].offset; + name_size = elf->sections[KMOD_ELF_SECTION_VERSION_EXT_NAMES].size; + crc_off = elf->sections[KMOD_ELF_SECTION_VERSION_EXT_CRCS].offset; + crc_size = elf->sections[KMOD_ELF_SECTION_VERSION_EXT_CRCS].size; + + if ((name_off == 0) || (crc_off == 0)) + return -ENODATA; + + if (crc_size == 0) + return 0; + + if (crc_size % sizeof(uint32_t) != 0) + return -EINVAL; + + count = crc_size / sizeof(uint32_t); + *array = malloc(sizeof(struct kmod_modversion) * count); + if (*array == NULL) + return -errno; + + for (name_cursor = elf_get_mem(elf, name_off), i = 0; i < count; i++) { + size_t len; + uint32_t crc; + + if (name_size == 0) { + ELFDBG(elf, "fewer symbol names than crcs\n"); + ret = -EINVAL; + goto release_array; + } + len = strnlen(name_cursor, name_size); + if (len == name_size) { + ELFDBG(elf, "last symbol name not null-terminated\n"); + ret = -EINVAL; + goto release_array; + } + crc = elf_get_uint(elf, crc_off, sizeof(uint32_t)); + + /* PPC sometimes prefixes names with `.` */ + if (name_cursor[0] == '.') { + name_cursor++; + len--; + } + + (*array)[i].crc = crc; + (*array)[i].bind = KMOD_SYMBOL_UNDEF; + (*array)[i].symbol = name_cursor; + + /* len doesn't include the NUL, but our pointers and size do */ + name_cursor += len + 1; + name_size -= len + 1; + crc_off += sizeof(uint32_t); + } + + return count; + +release_array: + free(*array); + return ret; +} + +/* array will be allocated with strings in a single malloc, just free *array */ +int kmod_elf_get_modversions(const struct kmod_elf *elf, struct kmod_modversion **array) +{ + int ret; + + *array = NULL; + + ret = kmod_elf_get_extended_modversions(elf, array); + /* if no extended modversions, fall back, otherwise propagate error */ + if (ret != -ENODATA) + return ret; + return kmod_elf_get_basic_modversions(elf, array); +} + static int elf_strip_versions_section(const struct kmod_elf *elf, uint8_t *changed) { uint64_t off, size; From 3eaf725d7af0c82bd8f6856441464ab469018163 Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Fri, 20 Dec 2024 22:20:05 +0000 Subject: [PATCH 3/3] testsuite/test-modprobe: Test --show-modversions 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 Link: https://github.com/kmod-project/kmod/pull/270 Signed-off-by: Lucas De Marchi --- scripts/setup-rootfs.sh | 1 + .../show-modversions/versions-mod-simple.txt | 11 +++++++++++ testsuite/test-modprobe.c | 16 ++++++++++++++++ 3 files changed, 28 insertions(+) create mode 100644 testsuite/rootfs-pristine/test-modprobe/show-modversions/versions-mod-simple.txt diff --git a/scripts/setup-rootfs.sh b/scripts/setup-rootfs.sh index cbf94bc9..6aee25c8 100755 --- a/scripts/setup-rootfs.sh +++ b/scripts/setup-rootfs.sh @@ -97,6 +97,7 @@ map=( ["test-modprobe/external/lib/modules/external/"]="mod-simple.ko" ["test-modprobe/module-from-abspath/home/foo/"]="mod-simple.ko" ["test-modprobe/module-from-relpath/home/foo/"]="mod-simple.ko" + ["test-modprobe/show-modversions/mod-simple.ko"]="mod-simple.ko" ["test-depmod/modules-order-compressed$MODULE_DIRECTORY/4.4.4/kernel/drivers/block/cciss.ko"]="mod-fake-cciss.ko" ["test-depmod/modules-order-compressed$MODULE_DIRECTORY/4.4.4/kernel/drivers/scsi/hpsa.ko"]="mod-fake-hpsa.ko" ["test-depmod/modules-order-compressed$MODULE_DIRECTORY/4.4.4/kernel/drivers/scsi/scsi_mod.ko"]="mod-fake-scsi-mod.ko" diff --git a/testsuite/rootfs-pristine/test-modprobe/show-modversions/versions-mod-simple.txt b/testsuite/rootfs-pristine/test-modprobe/show-modversions/versions-mod-simple.txt new file mode 100644 index 00000000..b4e1cb64 --- /dev/null +++ b/testsuite/rootfs-pristine/test-modprobe/show-modversions/versions-mod-simple.txt @@ -0,0 +1,11 @@ +0x........ single_open +0x........ seq_write +0x........ debugfs_remove +0x........ seq_lseek +0x........ seq_read +0x........ single_release +0x........ __fentry__ +0x........ debugfs_create_dir +0x........ debugfs_create_file +0x........ __x86_return_thunk +0x........ module_layout diff --git a/testsuite/test-modprobe.c b/testsuite/test-modprobe.c index 7a9aaedd..fb7d0bab 100644 --- a/testsuite/test-modprobe.c +++ b/testsuite/test-modprobe.c @@ -361,4 +361,20 @@ DEFINE_TEST(modprobe_module_from_relpath, .modules_loaded = "mod-simple", ); +static noreturn int modprobe_show_modversions(const struct test *t) +{ + EXEC_MODPROBE("--show-modversions", "/mod-simple.ko"); + exit(EXIT_FAILURE); +} +DEFINE_TEST(modprobe_show_modversions, + .description = "check if output for modprobe --show-modversions is correct", + .config = { + [TC_UNAME_R] = "4.4.4", + [TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/show-modversions", + }, + .output = { + .out = TESTSUITE_ROOTFS "test-modprobe/show-modversions/versions-mod-simple.txt", + .regex = true, + }); + TESTSUITE_MAIN();