summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYoussef Samir <youssef.abdulrahman@oss.qualcomm.com>2026-07-31 17:23:44 +0200
committerJeff Hugo <jeff.hugo@oss.qualcomm.com>2026-08-14 13:09:43 -0600
commitab243f74ab4084ca5c8dec608cb5b0deb27db067 (patch)
treed2bf250e2b55cffc7a856be88bf4de3cc9166425
parent6a736d2f9d0c6e6217fe7532bc4c50ceca71db78 (diff)
accel/qaic: Address potential out-of-bounds read in resp_worker()
Although 'commit 2feec5ae5df7 ("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: 2feec5ae5df7 ("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
-rw-r--r--drivers/accel/qaic/qaic_control.c46
1 files changed, 24 insertions, 22 deletions
diff --git a/drivers/accel/qaic/qaic_control.c b/drivers/accel/qaic/qaic_control.c
index 50bf3340e49c..2ccc55486aac 100644
--- a/drivers/accel/qaic/qaic_control.c
+++ b/drivers/accel/qaic/qaic_control.c
@@ -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 @@ dma_xfer_continue:
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);
}