smex: harden ELF parsing against malformed input#10922
Open
lgirdwood wants to merge 5 commits into
Open
Conversation
The section name string table was located using a header index without bounding it against the section count, and was null terminated at size minus one without checking for a zero-size section. Reject an out-of-range index and a zero-size string section before use. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
On a read error the section reader freed the output buffer but left the caller's pointer set, so a caller cleanup path could free it again. Clear the pointer after freeing. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Section names are used as offsets into the string table; an out-of-range name offset from a crafted ELF read past the table. Validate every section name offset against the string-table size after loading the sections. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
The fw-ready section was cast to a struct and field-read without checking its size, reading past a short section. Reject a section smaller than the struct. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
The extended-manifest walk advanced by an element size read from the section without validating it, so a zero size looped forever and a large size read past the section. Stop on a zero size or one that would leave the section. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
lgirdwood
requested review from
dbaluta,
kv2019i,
lbetlej,
mmaka1 and
plbossart
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Harden smex ELF parsing when handling malformed/untrusted objects by adding bounds checks and safer error handling around string tables, section lookups, and manifest parsing.
Changes:
- Validate section-header string table index/size and bound section-name offsets against the string table size.
- Prevent double-free by nulling the caller buffer on
elf_read_section()error. - Add size/bounds checks for
.fw_readyand extended manifest element walking.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| smex/ldc.c | Adds defensive size/bounds checks when parsing .fw_ready and extended manifest elements. |
| smex/elf.c | Validates shstrndx/string-table invariants, bounds section name offsets, and hardens error cleanup. |
Comment on lines
+60
to
67
| while ((uintptr_t)ext_hdr + sizeof(*ext_hdr) <= | ||
| (uintptr_t)buffer + section_size) { | ||
| if (ext_hdr->type == EXT_MAN_ELEM_DBG_ABI) { | ||
| header->version.abi_version = | ||
| ((struct ext_man_dbg_abi *) | ||
| ext_hdr)->dbg_abi.abi_dbg_version; | ||
| break; | ||
| } |
Comment on lines
+68
to
+75
| /* elem_size must advance the cursor and keep the next header | ||
| * within the section; otherwise stop instead of looping | ||
| * forever (elem_size == 0) or reading past the section | ||
| */ | ||
| if (ext_hdr->elem_size == 0 || | ||
| (uintptr_t)ext_hdr + ext_hdr->elem_size > | ||
| (uintptr_t)buffer + section_size) | ||
| break; |
Comment on lines
66
to
67
| module->strings = calloc(1, section[hdr->shstrndx].size); | ||
| if (!module->strings) { |
Comment on lines
+425
to
+429
| if (hdr->shstrndx >= hdr->shnum) { | ||
| fprintf(stderr, "error: invalid shstrndx %u >= shnum %u\n", | ||
| hdr->shstrndx, hdr->shnum); | ||
| return -EINVAL; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Harden smex against malformed/untrusted ELF input (e.g. a build pipeline
running smex on a contributed object):
string section before indexing
double-free in the caller
.fw_readysection before casting it to a structruns past the section)