Skip to content

Support modules with a lot of sections #280

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 50 additions & 26 deletions libkmod/libkmod-elf.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@
struct {
struct {
uint64_t offset;
uint16_t count;
uint32_t count;
uint16_t entry_size;
} section;
struct {
uint16_t section; /* index of the strings section */
uint32_t section; /* index of the strings section */
uint64_t size;
uint64_t offset;
} strings;
Expand Down Expand Up @@ -203,28 +203,28 @@
* (offset 0 cannot be a valid section offset because ELF header is located there).
*/
static inline uint64_t elf_get_section_header_offset(const struct kmod_elf *elf,
uint16_t idx)
uint32_t idx)
{
assert(idx != SHN_UNDEF);
assert(idx < elf->header.section.count);
if (idx == SHN_UNDEF || idx >= elf->header.section.count) {
ELFDBG(elf, "invalid section number: %" PRIu16 ", last=%" PRIu16 "\n",
ELFDBG(elf, "invalid section number: %" PRIu32 ", last=%" PRIu32 "\n",
idx, elf->header.section.count);
return 0;
}
return elf->header.section.offset +
(uint64_t)(idx * elf->header.section.entry_size);
(uint64_t)idx * elf->header.section.entry_size;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure how easy is for the uint16_t * uint16_t overflow to trigger, but let's keep this change as separate commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since idx is changed in this PR to a uint32_t and the C standard would therefore use unsigned int for the result, it would be possible.

}

static inline int elf_get_section_info(const struct kmod_elf *elf, uint16_t idx,
static inline int elf_get_section_info(const struct kmod_elf *elf, uint32_t idx,
uint64_t *offset, uint64_t *size,
const char **name)
{
uint64_t nameoff;
uint64_t off = elf_get_section_header_offset(elf, idx);

if (off == 0) {
ELFDBG(elf, "no section at %" PRIu16 "\n", idx);
ELFDBG(elf, "no section at %" PRIu32 "\n", idx);

Check warning on line 227 in libkmod/libkmod-elf.c

View check run for this annotation

Codecov / codecov/patch

libkmod/libkmod-elf.c#L227

Added line #L227 was not covered by tests
goto fail;
}

Expand Down Expand Up @@ -258,7 +258,7 @@
*name = elf_get_mem(elf, elf->header.strings.offset + nameoff);

ELFDBG(elf,
"section=%" PRIu16 " is: offset=%" PRIu64 " size=%" PRIu64 " name=%s\n",
"section=%" PRIu32 " is: offset=%" PRIu64 " size=%" PRIu64 " name=%s\n",
idx, *offset, *size, *name);

return 0;
Expand All @@ -275,7 +275,7 @@
uint16_t found_sec = 0;
enum kmod_elf_section sec;

for (uint16_t i = 1; i < elf->header.section.count && found_sec != all_sec; i++) {
for (uint32_t i = 1; i < elf->header.section.count && found_sec != all_sec; i++) {
uint64_t off, size;
const char *n;
int err = elf_get_section_info(elf, i, &off, &size, &n);
Expand Down Expand Up @@ -338,22 +338,42 @@
elf->size = size;

#define READV(field) elf_get_uint(elf, offsetof(typeof(*hdr), field), sizeof(hdr->field))
#define READXV(field) \
elf_get_uint(elf, elf->header.section.offset + offsetof(typeof(*shdr), field), \
sizeof(shdr->field))

#define LOAD_HEADER \
elf->header.section.offset = READV(e_shoff); \
elf->header.section.count = READV(e_shnum); \
elf->header.section.entry_size = READV(e_shentsize); \
elf->header.strings.section = READV(e_shstrndx); \
elf->header.machine = READV(e_machine); \
if (elf->header.section.count == 0 || \
elf->header.strings.section == SHN_XINDEX) { \
if (!elf_range_valid(elf, elf->header.section.offset, shdr_size)) \
goto invalid; \
if (elf->header.section.count == 0) { \
uint64_t val = READXV(sh_size); \
if (val > UINT32_MAX) \
goto invalid; \
elf->header.section.count = val; \
} \
if (elf->header.strings.section == SHN_XINDEX) { \
elf->header.strings.section = READXV(sh_link); \
} \
}

#define LOAD_HEADER \
elf->header.section.offset = READV(e_shoff); \
elf->header.section.count = READV(e_shnum); \
elf->header.section.entry_size = READV(e_shentsize); \
elf->header.strings.section = READV(e_shstrndx); \
elf->header.machine = READV(e_machine)
if (elf->x32) {
Elf32_Ehdr *hdr;
Elf32_Shdr *shdr;

Check warning on line 368 in libkmod/libkmod-elf.c

View check run for this annotation

Codecov / codecov/patch

libkmod/libkmod-elf.c#L368

Added line #L368 was not covered by tests

shdr_size = sizeof(Elf32_Shdr);
if (!elf_range_valid(elf, 0, shdr_size))
goto invalid;
LOAD_HEADER;
} else {
Elf64_Ehdr *hdr;
Elf64_Shdr *shdr;

shdr_size = sizeof(Elf64_Shdr);
if (!elf_range_valid(elf, 0, shdr_size))
Expand All @@ -364,8 +384,8 @@
#undef READV

ELFDBG(elf,
"section: offset=%" PRIu64 " count=%" PRIu16 " entry_size=%" PRIu16
" strings index=%" PRIu16 "\n",
"section: offset=%" PRIu64 " count=%" PRIu32 " entry_size=%" PRIu16
" strings index=%" PRIu32 "\n",
elf->header.section.offset, elf->header.section.count,
elf->header.section.entry_size, elf->header.strings.section);

Expand All @@ -374,7 +394,11 @@
elf->header.section.entry_size, shdr_size);
goto invalid;
}
shdrs_size = shdr_size * elf->header.section.count;
if (umulsz_overflow(shdr_size, elf->header.section.count, &shdrs_size)) {
ELFDBG(elf, "sections too large: %zu * %" PRIu32 "\n", shdr_size,
elf->header.section.count);
goto invalid;

Check warning on line 400 in libkmod/libkmod-elf.c

View check run for this annotation

Codecov / codecov/patch

libkmod/libkmod-elf.c#L398-L400

Added lines #L398 - L400 were not covered by tests
}
if (!elf_range_valid(elf, elf->header.section.offset, shdrs_size))
goto invalid;

Expand Down Expand Up @@ -412,13 +436,13 @@
}

/*
* Returns section index on success, negative value otherwise.
* Returns section index on success, zero otherwise.
* On success, sec_off and sec_size are range checked and valid.
*/
int kmod_elf_get_section(const struct kmod_elf *elf, const char *section,
uint64_t *sec_off, uint64_t *sec_size)
uint32_t kmod_elf_get_section(const struct kmod_elf *elf, const char *section,
uint64_t *sec_off, uint64_t *sec_size)
{
uint16_t i;
uint32_t i;

*sec_off = 0;
*sec_size = 0;
Expand All @@ -437,7 +461,7 @@
return i;
}

return -ENODATA;
return 0;
}

/* array will be allocated with strings in a single malloc, just free *array */
Expand Down Expand Up @@ -596,11 +620,11 @@
uint64_t off, size;
const void *buf;
/* the off and size values are not used, supply them as dummies */
int idx = kmod_elf_get_section(elf, "__versions", &off, &size);
uint32_t idx = kmod_elf_get_section(elf, "__versions", &off, &size);
uint64_t val;

if (idx < 0)
return idx == -ENODATA ? 0 : idx;
if (idx == 0)
return 0;

off = elf_get_section_header_offset(elf, idx);

Expand Down
2 changes: 1 addition & 1 deletion libkmod/libkmod-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ _must_check_ _nonnull_all_ const void *kmod_elf_strip(const struct kmod_elf *elf
* Debug mock lib need to find section ".gnu.linkonce.this_module" in order to
* get modname
*/
_must_check_ _nonnull_all_ int kmod_elf_get_section(const struct kmod_elf *elf, const char *section, uint64_t *sec_off, uint64_t *sec_size);
_must_check_ _nonnull_all_ uint32_t kmod_elf_get_section(const struct kmod_elf *elf, const char *section, uint64_t *sec_off, uint64_t *sec_size);

/* libkmod-signature.c */
struct kmod_signature_info {
Expand Down
5 changes: 3 additions & 2 deletions testsuite/init_module.c
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ long init_module(void *mem, unsigned long len, const char *args)
struct kmod_elf *elf;
struct mod *mod;
const void *buf;
uint32_t idx;
uint64_t off;
uint64_t bufsize;
int err;
Expand All @@ -238,12 +239,12 @@ long init_module(void *mem, unsigned long len, const char *args)
if (elf == NULL)
return 0;

err = kmod_elf_get_section(elf, ".gnu.linkonce.this_module", &off, &bufsize);
idx = kmod_elf_get_section(elf, ".gnu.linkonce.this_module", &off, &bufsize);
buf = (const char *)kmod_elf_get_memory(elf) + off;
kmod_elf_unref(elf);

/* We couldn't parse the ELF file. Just exit as if it was successful */
if (err < 0)
if (idx == 0)
return 0;

/* We need to open both 32 and 64 bits module - hack! */
Expand Down