Repository navigation
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The guard prevents invalid indexing, preserves valid-manifest behavior, and uses existing error propagation.
0 open findings
What changed in this PR
Prevents out-of-bounds indexing in the loadable-library manager when a manifest contains no distinct module segments.
Changes:
- Rejects zero-module manifests with
-EINVALbefore accessing module storage. - Logs the rejected manifest’s entry count and documents the guard.
| File | Description |
|---|---|
src/library_manager/llext_manager.c |
Adds a guard against zero-module manifests. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
The existing overflow check only rejects a module entry array that runs past the image, so a manifest describing zero distinct text segments passes it and leaves n_mod == 0. The subsequent ctx->mod[n_mod - 1].n_mod store then writes immediately before the allocation. Bail out with -EINVAL once the modules have been counted and none were found. Found by clang-analyzer-security.ArrayBound. Assisted-by: Copilot:claude-opus-5 clang-tidy Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
PR 11286: test resultsRun date: 2026-10-08 12:02 UTC Tested commit: 3415598cc7961daccadc62acb350c5b13c7b6140
|
| * A manifest with no distinct module segments would leave n_mod == 0, | ||
| * making the ctx->mod[n_mod - 1] accesses below index out of bounds | ||
| */ | ||
| if (!n_mod) { |
There was a problem hiding this comment.
I think the change is good given some malloc() implementations do return a pointer, but I think the comment + commitmsg are misleading as SOF allocated do return NULL so if n_mod is zero, this function will return -ENOMEM on line 568. I think explicit check is still better, but please correct the commit and maybe reduce the inline comments. This is self-explanatory.
The existing overflow check only rejects a module entry array that runs past the image, so a manifest describing zero distinct text segments passes it and leaves n_mod == 0. The subsequent ctx->mod[n_mod - 1].n_mod store then writes immediately before the allocation.
Bail out with -EINVAL once the modules have been counted and none were found.
Found by clang-analyzer-security.ArrayBound.
Assisted-by: Copilot:claude-opus-5 clang-tidy