Draft
Remove function names from logging calls in src/ipc directory#3
Conversation
Co-authored-by: lyakh <1363683+lyakh@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Remove function names from logging calls in src/ipc directory
Remove function names from logging calls in src/ipc directory
Aug 6, 2025
lyakh
reviewed
Aug 6, 2025
| assert_can_be_cold(); | ||
|
|
||
| tr_dbg(&ipc_tr, "ipc_init()"); | ||
| tr_dbg(&ipc_tr, ""); |
Owner
There was a problem hiding this comment.
empty line comment makes no sense, for such cases let's put, say "function entry" there since usually that's where such calls show up
lyakh
pushed a commit
that referenced
this pull request
Jul 20, 2026
Fix a heap out-of-bounds access found by the IPC4 libFuzzer harness
(native_sim/x86, ASAN-enabled build with CONFIG_SYS_HEAP_SANITIZER_ASAN).
A single 173-byte crash input triggers a use-after-poison READ of size 4
in copier_dai_create():
==ERROR: AddressSanitizer: use-after-poison on address 0x088cf1c4
READ of size 4 ... in copier_dai_create copier_dai.c:284
#1 copier_init copier.c:220
#2 module_init generic.c:123
#3 module_adapter_new_ext module_adapter.c:307
#4 comp_new_ipc4 ipc4/helper.c:198
#5 ipc4_init_module_instance handler-user.c:763
copier_dai.c:284 reads cd->config.gtw_cfg.config_length, which lands 8
bytes past the end of the under-allocated cd block.
Fuzzer payload
--------------
The harness frames the raw input as a 2-byte little-endian length prefix
followed by an 8-byte IPC4 compact header (primary + extension word) and
a payload that is mirrored into MAILBOX_HOSTBOX. The decoded message is
an INIT_MODULE_INSTANCE (MODULE_MSG / INIT_INSTANCE) whose module_id
resolves to the copier driver, so comp_new_ipc4() -> module_adapter_new()
-> copier_init() runs on host-controlled config bytes.
The copier's ipc4_copier_module_cfg carried (values from gdb):
gtw_cfg.node_id.dw = 0x050012f5
dma_type = 18 (ipc4_alh_uaol_stream_link_
output_class, a non-host ALH/UAOL link
gateway), v_index = 245
gtw_cfg.dma_buffer_size = 0xffffffff
gtw_cfg.config_length = 0xffffffff <-- the trigger
md->cfg.size (payload) = 888 bytes
node_id != IPC4_INVALID_NODE_ID selects the gateway-copier path, and the
UAOL link class routes into copier_dai_create(). The long run of 0xff
bytes at the tail of the input is what supplies config_length =
0xffffffff. libFuzzer discovered the message by mutating the corpus; the
crucial field is the all-ones config_length.
Root cause
----------
copier_init() sizes the variable-length gateway blob that trails the
config struct:
if (copier->gtw_cfg.config_length > 1) {
gtw_cfg_var_size += (copier->gtw_cfg.config_length - 1) << 2;
cfg_total_size += gtw_cfg_var_size;
}
if (cfg_total_size > md->cfg.size) /* payload guard */
return -EINVAL;
cd = mod_zalloc(mod, sizeof(*cd) + gtw_cfg_var_size);
config_length is a uint32_t from the untrusted payload. The expression
(config_length - 1) << 2 is evaluated in 32-bit arithmetic and wraps:
with 0xffffffff it yields 0xfffffff8, i.e. -8 as a size_t. That poisons
both dependent computations:
* cfg_total_size wraps down to 76 (below even sizeof(*copier) = 84),
so the "cfg_total_size > md->cfg.size" guard added earlier does not
fire (76 < 888).
* mod_zalloc() is called with sizeof(*cd) + (size_t)-8 = 452 - 8 =
444 bytes, while struct copier_data needs 452. gtw_cfg.config_length
sits at offset 444, exactly past the truncated allocation.
copier_dai_create() then reads that field (and the config_data pointer
right after it) out of bounds. size_t is 32-bit on native_sim and on the
real Xtensa DSP targets, so this is a genuine firmware bug, not just a
host-fuzzer artifact.
Fix
---
Validate config_length before any size arithmetic is performed on it.
The blob is expressed in dwords and must fit within the received init
payload, so an upper bound of md->cfg.size / sizeof(uint32_t) both is
correct and keeps (config_length - 1) << 2 from overflowing (the mailbox
payload is bounded by SOF_IPC_MSG_MAX_SIZE):
if (copier->gtw_cfg.config_length >
md->cfg.size / sizeof(uint32_t)) {
comp_err(dev, "copier_init(): invalid gtw_cfg.config_length %u",
copier->gtw_cfg.config_length);
return -EINVAL;
}
The existing precise "cfg_total_size > md->cfg.size" check is retained as
a secondary guard for the non-overflowing mismatch case.
Verification
------------
- The crash input now executes cleanly: config_length is rejected with
-EINVAL, exit code 0, no ASAN report. The new bound branch is
confirmed hit (config_length 0xffffffff vs bound 222).
- Full IPC4 corpus replay: 92,780 inputs, 0 crashes (-runs=0), coverage
unchanged (cov 1331 / ft 5270), confirming the check does not
over-restrict legitimate copier init payloads.
Found-by: IPC4 libFuzzer (ASAN, native_sim)
Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
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.
This patch removes function names from the beginning of all logging calls in the
src/ipcdirectory as requested. The changes affect logging macros includingcomp_info(),comp_dbg(),comp_err(),comp_warn(),tr_info(),tr_dbg(),tr_err(),tr_warn(), and related variants.Changes Made
src/ipcdirectoryExamples of Changes
Function names with messages:
Standalone function name logging:
Files Modified
src/ipc/dma-copy.c- 4 function names removedsrc/ipc/ipc-common.c- 2 function names removedsrc/ipc/ipc-helper.c- 6 function names removedsrc/ipc/ipc4/helper.c- 4 function names removedsrc/ipc/ipc4/dai.c- 8 function names removedsrc/ipc/ipc3/helper.c- 19 function names removedsrc/ipc/ipc3/host-page-table.c- 6 function names removedsrc/ipc/ipc3/dai.c- 10 function names removedsrc/ipc/ipc3/handler.c- 20 function names removedAll changes have been verified to ensure no logging patterns remain that include function names at the beginning of log messages. The patch maintains all existing log levels and preserves the informational content of the messages.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.