From 05ccc16894333b47927756269ec4987b485d52ba Mon Sep 17 00:00:00 2001 From: Tomasz Leman Date: Wed, 6 May 2026 20:33:50 +0200 Subject: [PATCH] ipc4: handler-user: fix TLV walker pointer wraparound The TLV walker loop in ipc4_set_vendor_config_module_instance() advances the tlv pointer by sizeof(struct sof_tlv) + ALIGN_UP(tlv->length, 4) without validating that the result stays within the IPC payload buffer. Issue was found using static analysis security scanning tools and confirmed by testing that a malformed or incorrectly crafted TLV with an oversized length field causes the 32-bit pointer arithmetic to wrap around, triggering a null pointer dereference and DSP panic. Fix by: 1. Adding an upper-bound check on data_off_size against MAILBOX_HOSTBOX_SIZE at function entry. 2. Validating on each loop iteration that the TLV header + value fits within the remaining buffer bytes before calling set_large_config or advancing the pointer. The check uses integer subtraction (not pointer addition) to avoid undefined behavior from pointer overflow hat the compiler could optimize away, and splits the comparison to prevent size_t overflow when tlv->length is near UINT32_MAX. Signed-off-by: Tomasz Leman --- src/ipc/ipc4/handler-user.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/ipc/ipc4/handler-user.c b/src/ipc/ipc4/handler-user.c index c3ba63b3d267..2bfa31babce0 100644 --- a/src/ipc/ipc4/handler-user.c +++ b/src/ipc/ipc4/handler-user.c @@ -1085,7 +1085,7 @@ __cold static int ipc4_set_vendor_config_module_instance(struct comp_dev *dev, * (4 bytes type | 4 bytes length=0 | no value) * we do not handle such case */ - if (data_off_size < sizeof(struct sof_tlv)) + if (data_off_size < sizeof(struct sof_tlv) || data_off_size > MAILBOX_HOSTBOX_SIZE) return IPC4_INVALID_CONFIG_DATA_STRUCT; /* ===Iterate over payload=== @@ -1097,10 +1097,21 @@ __cold static int ipc4_set_vendor_config_module_instance(struct comp_dev *dev, const uint8_t *end_offset = (const uint8_t *)data + data_off_size; while ((const uint8_t *)tlv < end_offset) { + size_t remaining = (size_t)(end_offset - (const uint8_t *)tlv); + /* check for invalid length */ if (!tlv->length) return IPC4_INVALID_CONFIG_DATA_LEN; + /* Validate TLV header + value fits within remaining + * payload to prevent OOB access and pointer wraparound + * on 32-bit arithmetic (CWE-190). Split into two checks + * to avoid overflow in the size_t addition itself. + */ + if (remaining < sizeof(struct sof_tlv) || + tlv->length > remaining - sizeof(struct sof_tlv)) + return IPC4_INVALID_REQUEST; + ret = drv->ops.set_large_config(dev, tlv->type, init_block, final_block, tlv->length, tlv->value); if (ret < 0) { @@ -1108,7 +1119,7 @@ __cold static int ipc4_set_vendor_config_module_instance(struct comp_dev *dev, (uint32_t)module_id, (uint32_t)instance_id); return IPC4_INVALID_RESOURCE_ID; } - /* Move pointer to the end of this tlv */ + /* Move pointer to the end of this tlv (aligned) */ tlv = (struct sof_tlv *)((const uint8_t *)tlv + sizeof(struct sof_tlv) + ALIGN_UP(tlv->length, 4)); }