lib_manager: bound build info offset and check resume allocation#10931
Open
lgirdwood wants to merge 2 commits into
Open
lib_manager: bound build info offset and check resume allocation#10931lgirdwood wants to merge 2 commits into
lgirdwood wants to merge 2 commits into
Conversation
The build info pointer was derived from a manifest-supplied text segment offset without bounds, so a crafted manifest could read outside the library buffer. Validate the offset against the library image size before dereferencing and fail the module type lookup otherwise. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
The resume path allocated a library context and immediately wrote through it without a NULL check, crashing under memory pressure. Check the allocation and fail the restore gracefully. 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.
Hardens library manager restore and module parsing paths by adding bounds/NULL checks to prevent OOB reads and NULL dereferences under adverse conditions.
Changes:
- Add NULL-check for resume-path context allocation before dereferencing.
- Validate manifest-supplied TEXT segment
file_offsetagainst the library image size before using it to derivebuild_info.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/library_manager/llext_manager_dram.c | Adds allocation failure handling on DRAM/IMR resume path. |
| src/library_manager/lib_manager.c | Adds bounds check for TEXT file_offset before dereferencing derived build-info pointer. |
Comment on lines
+583
to
+589
| const size_t lib_size = (size_t)desc->header.preload_page_count * PAGE_SZ; | ||
| const uint32_t text_off = mod->segment[SOF_MAN_SEGMENT_TEXT].file_offset; | ||
|
|
||
| if (text_off > lib_size || lib_size - text_off < sizeof(*build_info)) { | ||
| tr_err(&lib_manager_tr, "Invalid TEXT file_offset %u", text_off); | ||
| return MOD_TYPE_INVALID; | ||
| } |
Comment on lines
+586
to
+589
| if (text_off > lib_size || lib_size - text_off < sizeof(*build_info)) { | ||
| tr_err(&lib_manager_tr, "Invalid TEXT file_offset %u", text_off); | ||
| return MOD_TYPE_INVALID; | ||
| } |
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.
Two robustness fixes in the library manager:
image size before deriving the build-info pointer, so a crafted manifest
cannot read outside the library buffer
through it (NULL dereference under memory pressure otherwise)