Skip to content

Commit fe7281b

Browse files
tmlemankv2019i
authored andcommitted
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 62c713e commit fe7281b

1 file changed

Lines changed: 13 additions & 2 deletions

File tree

src/ipc/ipc4/handler-user.c

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,7 +1085,7 @@ __cold static int ipc4_set_vendor_config_module_instance(struct comp_dev *dev,
10851085
* (4 bytes type | 4 bytes length=0 | no value)
10861086
* we do not handle such case
10871087
*/
1088-
if (data_off_size < sizeof(struct sof_tlv))
1088+
if (data_off_size < sizeof(struct sof_tlv) || data_off_size > MAILBOX_HOSTBOX_SIZE)
10891089
return IPC4_INVALID_CONFIG_DATA_STRUCT;
10901090

10911091
/* ===Iterate over payload===
@@ -1097,18 +1097,29 @@ __cold static int ipc4_set_vendor_config_module_instance(struct comp_dev *dev,
10971097
const uint8_t *end_offset = (const uint8_t *)data + data_off_size;
10981098

10991099
while ((const uint8_t *)tlv < end_offset) {
1100+
size_t remaining = (size_t)(end_offset - (const uint8_t *)tlv);
1101+
11001102
/* check for invalid length */
11011103
if (!tlv->length)
11021104
return IPC4_INVALID_CONFIG_DATA_LEN;
11031105

1106+
/* Validate TLV header + value fits within remaining
1107+
* payload to prevent OOB access and pointer wraparound
1108+
* on 32-bit arithmetic (CWE-190). Split into two checks
1109+
* to avoid overflow in the size_t addition itself.
1110+
*/
1111+
if (remaining < sizeof(struct sof_tlv) ||
1112+
tlv->length > remaining - sizeof(struct sof_tlv))
1113+
return IPC4_INVALID_REQUEST;
1114+
11041115
ret = drv->ops.set_large_config(dev, tlv->type, init_block,
11051116
final_block, tlv->length, tlv->value);
11061117
if (ret < 0) {
11071118
ipc_cmd_err(&ipc_tr, "failed to set large_config_module_instance %x : %x",
11081119
(uint32_t)module_id, (uint32_t)instance_id);
11091120
return IPC4_INVALID_RESOURCE_ID;
11101121
}
1111-
/* Move pointer to the end of this tlv */
1122+
/* Move pointer to the end of this tlv (aligned) */
11121123
tlv = (struct sof_tlv *)((const uint8_t *)tlv +
11131124
sizeof(struct sof_tlv) + ALIGN_UP(tlv->length, 4));
11141125
}

0 commit comments

Comments
 (0)