From: Eva Crystal <0xiviel@gmail.com>
To: Min Ma <mamin506@gmail.com>, Lizhi Hou <lizhi.hou@amd.com>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
Eva Crystal <0xiviel@gmail.com>
Subject: [PATCH 4/4] accel/amdxdna: check the command payload before using it in the exec requests
Date: Sat, 12 Sep 2026 20:10:12 +1200 [thread overview]
Message-ID: <20260912081012.2274075-5-0xiviel@gmail.com> (raw)
In-Reply-To: <20260912081012.2274075-1-0xiviel@gmail.com>
aie2_init_exec_dpu_req() takes the payload of a command BO and subtracts
the fixed header from its length before establishing that the length
covers the header at all:
sn = amdxdna_cmd_get_payload(cmd_bo, &cmd_len);
if (cmd_len - sizeof(*sn) > sizeof(dpu_req->payload))
return -EINVAL;
...
dpu_req->inst_buf_addr = sn->buffer;
memcpy(dpu_req->payload, sn->prop_args, cmd_len - sizeof(*sn));
It also dereferences sn without testing it, although
amdxdna_cmd_get_payload() returns NULL for a command header whose count
field does not fit the BO, setting cmd_len to 0 as it does so.
Both work out today, and for the same reason: cmd_len is a u32 and
sizeof() is a size_t, so the subtraction is evaluated in 64-bit. A short
command produces a value near 2^64, which is larger than the payload
field, so the test returns -EINVAL before sn is dereferenced and before
the memcpy. The NULL case is caught by the same comparison, because
cmd_len is then 0.
The guarantee is therefore supplied entirely by the operand types. A
later change that computes the difference into a u32 first - as the
sibling slot-filling functions in this file already do - would truncate
the underflow to a small value, and the function would dereference NULL
and copy from a short payload. Those siblings, aie2_cmdlist_fill_dpu()
and friends, all carry an explicit "cmd_len < sizeof(*sn)" test for
exactly this reason; this path was left without one.
Add the explicit length and NULL tests here, matching the siblings, and
add the missing NULL test to aie2_init_exec_cu_req(), which copies
cmd_len bytes from a pointer it likewise never checks. No behavioural
change is intended: every input rejected by the new tests is already
rejected today.
Signed-off-by: Eva Crystal <0xiviel@gmail.com>
---
drivers/accel/amdxdna/aie2_message.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c
index 3028968..ab9e7c4 100644
--- a/drivers/accel/amdxdna/aie2_message.c
+++ b/drivers/accel/amdxdna/aie2_message.c
@@ -556,7 +556,7 @@ static int aie2_init_exec_cu_req(struct amdxdna_gem_obj *cmd_bo, void *req,
void *cmd;
cmd = amdxdna_cmd_get_payload(cmd_bo, &cmd_len);
- if (cmd_len > sizeof(cu_req->payload))
+ if (!cmd || cmd_len > sizeof(cu_req->payload))
return -EINVAL;
cu_req->cu_idx = amdxdna_cmd_get_cu_idx(cmd_bo);
@@ -578,7 +578,8 @@ static int aie2_init_exec_dpu_req(struct amdxdna_gem_obj *cmd_bo, void *req,
u32 cmd_len;
sn = amdxdna_cmd_get_payload(cmd_bo, &cmd_len);
- if (cmd_len - sizeof(*sn) > sizeof(dpu_req->payload))
+ if (!sn || cmd_len < sizeof(*sn) ||
+ cmd_len - sizeof(*sn) > sizeof(dpu_req->payload))
return -EINVAL;
dpu_req->cu_idx = amdxdna_cmd_get_cu_idx(cmd_bo);
--
2.53.0
prev parent reply other threads:[~2026-09-13 20:53 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-12 8:10 [PATCH 0/4] accel/amdxdna: harden command BO payload validation Eva Crystal
2026-09-12 8:10 ` [PATCH 1/4] accel/amdxdna: validate the command payload regardless of the size argument Eva Crystal
2026-09-12 8:10 ` [PATCH 2/4] accel/amdxdna: bound the command error payload length Eva Crystal
2026-09-12 8:10 ` [PATCH 3/4] accel/amdxdna: release the chained command BO when vmap fails Eva Crystal
2026-09-12 8:10 ` Eva Crystal [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260912081012.2274075-5-0xiviel@gmail.com \
--to=0xiviel@gmail.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lizhi.hou@amd.com \
--cc=mamin506@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.