accel/qaic: Address potential out-of-bounds read in resp_worker()

Although 'commit 2feec5ae5d ("accel/qaic: Handle DBC deactivation if the
owner went away")' fixes the scenario it was intended for by walking the
message and only decoding QAIC_TRANS_DEACTIVATE_FROM_DEV, if present, it
skipped over the bounds checking code that is included in decode_message().
This could lead to issues such as reading past the slab allocation's end,
infinite loops or kernel panics. For those issues to happen, a malformed
wire message is needed to be sent from the device.

Instead of duplicating the bounds checking code already present in
decode_message(), use the function inside resp_worker().

Reported-by: Ruikai Peng <ruikai@pwno.io>
Fixes: 2feec5ae5d ("accel/qaic: Handle DBC deactivation if the owner went away")
Reviewed-by: Jeff Hugo <jeff.hugo@oss.qualcomm.com>
Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
Signed-off-by: Youssef Samir <youssef.abdulrahman@oss.qualcomm.com>
Signed-off-by: Jeff Hugo <jeff.hugo@oss.qualcomm.com>
Link: https://patch.msgid.link/20260731152344.1905882-1-youssef.abdulrahman@oss.qualcomm.com
This commit is contained in:
Youssef Samir
2026-07-31 17:23:44 +02:00
committed by Jeff Hugo
parent 6a736d2f9d
commit ab243f74ab

View File

@@ -963,11 +963,13 @@ static int decode_status(struct qaic_device *qdev, void *trans, struct manage_ms
static int decode_message(struct qaic_device *qdev, struct manage_msg *user_msg,
struct wire_msg *msg, struct ioctl_resources *resources,
struct qaic_user *usr)
struct qaic_user *usr, bool orphaned_deactivate)
{
u32 msg_hdr_count = le32_to_cpu(msg->hdr.count);
u32 msg_hdr_len = le32_to_cpu(msg->hdr.len);
struct wire_trans_hdr *trans_hdr;
u32 msg_len = 0;
int trans_type;
int ret;
int i;
@@ -975,10 +977,12 @@ static int decode_message(struct qaic_device *qdev, struct manage_msg *user_msg,
msg_hdr_len > QAIC_MANAGE_MAX_MSG_LENGTH)
return -EINVAL;
user_msg->len = 0;
user_msg->count = le32_to_cpu(msg->hdr.count);
if (user_msg) {
user_msg->len = 0;
user_msg->count = msg_hdr_count;
}
for (i = 0; i < user_msg->count; ++i) {
for (i = 0; i < msg_hdr_count; ++i) {
u32 hdr_len;
if (msg_len > msg_hdr_len - sizeof(*trans_hdr))
@@ -990,7 +994,20 @@ static int decode_message(struct qaic_device *qdev, struct manage_msg *user_msg,
size_add(msg_len, hdr_len) > msg_hdr_len)
return -EINVAL;
switch (le32_to_cpu(trans_hdr->type)) {
trans_type = le32_to_cpu(trans_hdr->type);
/*
* orphaned_deactivate is the case where a deactivate response
* is received from the device after the user owning the DBC,
* and the message requesting deactivation, has gone away.
* In this case, only process QAIC_TRANS_DEACTIVATE_FROM_DEV
* transaction and skip the others.
*/
if (orphaned_deactivate && trans_type != QAIC_TRANS_DEACTIVATE_FROM_DEV) {
msg_len += hdr_len;
continue;
}
switch (trans_type) {
case QAIC_TRANS_PASSTHROUGH_FROM_DEV:
ret = decode_passthrough(qdev, trans_hdr, user_msg, &msg_len);
break;
@@ -1281,7 +1298,7 @@ static int qaic_manage(struct qaic_device *qdev, struct qaic_user *usr, struct m
goto dma_cont_failed;
}
ret = decode_message(qdev, user_msg, rsp, &resources, usr);
ret = decode_message(qdev, user_msg, rsp, &resources, usr, false);
dma_cont_failed:
free_dbc_buf(qdev, &resources);
@@ -1446,22 +1463,7 @@ static void resp_worker(struct work_struct *work)
* response to the QAIC_TRANS_TERMINATE_TO_DEV transaction,
* otherwise, the user can issue an soc_reset to the device.
*/
u32 msg_count = le32_to_cpu(msg->hdr.count);
u32 msg_len = le32_to_cpu(msg->hdr.len);
u32 len = 0;
int j;
for (j = 0; j < msg_count && len < msg_len; ++j) {
struct wire_trans_hdr *trans_hdr;
trans_hdr = (struct wire_trans_hdr *)(msg->data + len);
if (le32_to_cpu(trans_hdr->type) == QAIC_TRANS_DEACTIVATE_FROM_DEV) {
if (decode_deactivate(qdev, trans_hdr, &len, NULL))
len += le32_to_cpu(trans_hdr->len);
} else {
len += le32_to_cpu(trans_hdr->len);
}
}
decode_message(qdev, NULL, msg, NULL, NULL, true);
/* request must have timed out, drop packet */
kfree(msg);
}