Skip to content

llvm patches for SOF xtensa - #1

Merged
lgirdwood merged 2 commits into
lgirdwood:clang-upstreamfrom
hliao8:llvm10
May 20, 2026
Merged

llvm patches for SOF xtensa#1
lgirdwood merged 2 commits into
lgirdwood:clang-upstreamfrom
hliao8:llvm10

Conversation

@hliao8

@hliao8 hliao8 commented May 19, 2026

Copy link
Copy Markdown

Hope this PR is OK I used AI. Had to add some code so I could use my llvm patches on SOF and Zephyr.

@hliao8
hliao8 requested a review from lgirdwood as a code owner May 19, 2026 11:24
hliao8 added 2 commits May 19, 2026 19:45
Zephyr's FindZephyr-sdk.cmake intentionally skips auto-discovery
when ZEPHYR_TOOLCHAIN_VARIANT=llvm, expecting the user to point
ZEPHYR_SDK_INSTALL_DIR at the SDK that provides the GNU Xtensa
binutils / sysroot the LLVM clang driver wraps around.

When neither the user nor the environment supplies that variable
the build fails late with confusing 'string.h: file not found' /
malformed -isystem errors. Auto-detect a zephyr-sdk-* directory
under $HOME, /opt, or /usr/local (highest version wins) and export
ZEPHYR_SDK_INSTALL_DIR before invoking west when the user passes
--llvm-clang.

This keeps the LLVM build flow as one command for the common case
while still respecting any value the user has already exported.

Signed-off-by: Henry Liao <hliao8@proton.me>
The HiFi3 / HiFi4 / HiFi5 sources unconditionally include
<xtensa/config/defs.h>, a header that ships only with the Cadence
xt-clang / xt-gcc toolchains. The LLVM clang toolchain variant uses
the Zephyr SDK GNU Xtensa binutils for assembly/linking but does
not expose that Cadence-internal header, so every HiFi
translation unit fails to find it even though the body of the file
is guarded by SOF_USE_HIFI(...) and is unreachable for the LLVM
build.

Wrap the include in __has_include so the compiler silently skips
it on toolchains that lack the header. The HiFi intrinsics are
already supplied by <xtensa/tie/xt_hifiN.h>, which the LLVM
toolchain does provide.

No functional change for the Cadence toolchain path.

Signed-off-by: Henry Liao <hliao8@proton.me>
@lgirdwood
lgirdwood merged commit c159128 into lgirdwood:clang-upstream May 20, 2026
lgirdwood pushed a commit that referenced this pull request Jul 19, 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
        thesofproject#2 module_init                 generic.c:123
        thesofproject#3 module_adapter_new_ext      module_adapter.c:307
        thesofproject#4 comp_new_ipc4               ipc4/helper.c:198
        thesofproject#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants