Skip to content

Commit 331b378

Browse files
tmlemanlgirdwood
authored andcommitted
ipc4: helper: bound TLV length safely when parsing DMA configs
The IPC4 fuzzer hit an out-of-bounds read in ipc4_find_all_dma_configs_tlvs_only() while parsing a copier gateway config blob as a TLV sequence. AddressSanitizer reports a SEGV on a wild address ~16 MB below the config buffer. Root cause: the per-TLV bounds check and the iterator both do pointer arithmetic on the host-controlled tlvs->length without guarding against overflow. On 32-bit targets (native_sim and the Xtensa DSPs) a large length wraps the pointer: if ((uintptr_t)tlvs->value + tlvs->length > end_addr) /* wraps */ return IPC4_INVALID_REQUEST; ... tlvs = tlv_next(tlvs); /* tlvs + 8 + length, also wraps */ With length = 0xff000000 and a 32-byte buffer, tlvs->value + length wraps to a value below end_addr, so the check passes; tlv_next() wraps to the same address, which is still below end_addr, so the loop continues and dereferences that wild pointer. config_length is already bounded to the init payload (so the buffer itself is small and valid) - this is a separate overflow inside the TLV walk, on the length field embedded in the blob. Compare tlvs->length against the remaining space using subtraction, which cannot overflow because the (tightened) loop condition guarantees the TLV header - and therefore tlvs->value - is within the buffer. tlv_next() is thus bounded by end_addr as well. The loop condition now also requires the full 8-byte header to fit before it is dereferenced. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
1 parent 9f9ea30 commit 331b378

1 file changed

Lines changed: 12 additions & 3 deletions

File tree

src/ipc/ipc4/helper.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1482,10 +1482,19 @@ int ipc4_find_all_dma_configs_tlvs_only(struct ipc_config_dai *dai,
14821482
struct ipc_dma_config *dma_cfg;
14831483
int count = 0;
14841484

1485-
for (tlvs = (struct sof_tlv *)data_buffer; tlvs && (uintptr_t)tlvs < end_addr;
1485+
for (tlvs = (struct sof_tlv *)data_buffer;
1486+
tlvs && (uintptr_t)tlvs + sizeof(*tlvs) <= end_addr;
14861487
tlvs = tlv_next(tlvs)) {
1487-
if ((uintptr_t)tlvs->value + tlvs->length > end_addr) {
1488-
tr_err(&ipc_tr, "Unexpected TLV length %d: exceeds buffer size", tlvs->length);
1488+
/*
1489+
* tlvs->length is host-controlled: computing
1490+
* "tlvs->value + tlvs->length" can overflow the pointer and
1491+
* wrap below end_addr, bypassing this check and letting
1492+
* tlv_next() advance to a wild address. Compare against the
1493+
* remaining space instead. The loop condition guarantees the
1494+
* TLV header fits, so end_addr - tlvs->value does not underflow.
1495+
*/
1496+
if (tlvs->length > end_addr - (uintptr_t)tlvs->value) {
1497+
tr_err(&ipc_tr, "Unexpected TLV length %u: exceeds buffer size", tlvs->length);
14891498
return IPC4_INVALID_REQUEST;
14901499
}
14911500

0 commit comments

Comments
 (0)