Repository navigation
logger: validate uids dictionary offset and uid entry bounds - #11313
Shrusti-pk wants to merge 1 commit into
Conversation
|
Can one of the admins verify this patch?
|
|
test this please |
There was a problem hiding this comment.
🟡 Changes recommended
The new bounds check can overflow on 32-bit hosts and allow an out-of-bounds read.
1 open finding
What changed in this PR
This PR hardens sof-logger’s UUID dictionary handling against malformed dictionary and trace inputs.
Changes:
- Reject unexpected UUID dictionary data offsets.
- Require a complete UUID entry when resolving component names.
| File | Description |
|---|---|
| tools/logger/convert.c | Adds dictionary offset validation and full-entry bounds checking. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| uid_ptr + sizeof(struct sof_uuid_entry) > | ||
| uids_dict->base_address + uids_dict->data_length) |
There was a problem hiding this comment.
Right, uid_ptr + sizeof(struct sof_uuid_entry) can wrap with a 32-bit size_t and let a uid near 0xffffffff through. It now checks data_length against the entry size first and compares uid_ptr - base_address against data_length - sizeof(struct sof_uuid_entry), so nothing in the bound can wrap. A uid at base still resolves, while base + data_length - 1 and 0xfffffff0 both print <uid?>.
convert() copies the uuid dictionary header from the .ldc file into uids_dict as is and always loads the uuid entries right after it, but get_uuid_entry(), get_uuid_key(), dump_ldc_info() and get_uuid_by_name() all locate the entries through the header's data_offset field, which is never checked. smex always writes sizeof(struct snd_sof_uids_header) there; any other value makes every dictionary lookup read outside of the allocation. Reject it at load time. get_component_name() also bounds the uid taken from the trace stream with a bare ">=" check, so a uid pointing into the last bytes of the section passes and the entry name is read past the end of the buffer. Require a full sof_uuid_entry to fit in the section, comparing the offset from base_address against the remaining length so the check can't wrap on hosts with a 32-bit size_t. Signed-off-by: Shrushti P K <shrusthi@labs.digiscrypt.com>
274e8fd to
45a4d68
Compare
Two out-of-bounds reads in sof-logger's uuid dictionary handling, both reachable from a malformed .ldc or trace input:
sof-logger -don an .ldc with data_offset 0x10000 gives an ASan heap-buffer-overflow in format_uid_raw.>=, so a uid in the last bytes of the section passes and the name is read past the dictionary buffer (ASan heap-buffer-overflow in print_entry_params). It now uses the same full-entry bound format_uid() already has.Output for a well-formed .ldc and trace is unchanged.