Skip to content

Commit 4ae5905

Browse files
committed
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 <tomasz.m.leman@intel.com>
1 parent 8d75e2c commit 4ae5905

1 file changed

Lines changed: 15 additions & 1 deletion

File tree

src/ipc/ipc4/handler-user.c

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1088,6 +1088,9 @@ __cold static int ipc4_set_vendor_config_module_instance(struct comp_dev *dev,
10881088
if (data_off_size < sizeof(struct sof_tlv))
10891089
return IPC4_INVALID_CONFIG_DATA_STRUCT;
10901090

1091+
if (data_off_size > MAILBOX_HOSTBOX_SIZE)
1092+
return IPC4_INVALID_REQUEST;
1093+
10911094
/* ===Iterate over payload===
10921095
* Payload can have multiple sof_tlv structures inside,
10931096
* You can find how many by checking payload size (data_off_size)
@@ -1097,18 +1100,29 @@ __cold static int ipc4_set_vendor_config_module_instance(struct comp_dev *dev,
10971100
const uint8_t *end_offset = (const uint8_t *)data + data_off_size;
10981101

10991102
while ((const uint8_t *)tlv < end_offset) {
1103+
size_t remaining = (size_t)(end_offset - (const uint8_t *)tlv);
1104+
11001105
/* check for invalid length */
11011106
if (!tlv->length)
11021107
return IPC4_INVALID_CONFIG_DATA_LEN;
11031108

1109+
/* Validate TLV header + value fits within remaining
1110+
* payload to prevent OOB access and pointer wraparound
1111+
* on 32-bit arithmetic (CWE-190). Split into two checks
1112+
* to avoid overflow in the size_t addition itself.
1113+
*/
1114+
if (remaining < sizeof(struct sof_tlv) ||
1115+
tlv->length > remaining - sizeof(struct sof_tlv))
1116+
return IPC4_INVALID_REQUEST;
1117+
11041118
ret = drv->ops.set_large_config(dev, tlv->type, init_block,
11051119
final_block, tlv->length, tlv->value);
11061120
if (ret < 0) {
11071121
ipc_cmd_err(&ipc_tr, "failed to set large_config_module_instance %x : %x",
11081122
(uint32_t)module_id, (uint32_t)instance_id);
11091123
return IPC4_INVALID_RESOURCE_ID;
11101124
}
1111-
/* Move pointer to the end of this tlv */
1125+
/* Move pointer to the end of this tlv (aligned) */
11121126
tlv = (struct sof_tlv *)((const uint8_t *)tlv +
11131127
sizeof(struct sof_tlv) + ALIGN_UP(tlv->length, 4));
11141128
}

0 commit comments

Comments
 (0)