Bug bounty#1422
Conversation
SWSPLAT-12163: amdxdna_cmd_get_payload() skips payload size validation when the size argument is NULL. Add a minimum BO size check unconditionally to prevent out-of-bounds access on undersized user BOs. Additionally, amdxdna_cmd_set_error() dereferences the return value of amdxdna_cmd_get_payload(..., NULL) without checking for NULL, which can cause a kernel NULL pointer dereference if vmap fails. Add NULL guard.
…inline SWSPLAT-12163: Three additional unguarded dereferences of the return value of amdxdna_cmd_get_payload(..., NULL) in the src driver tree: - ve2_hwctx.c: ve2_hwctx_job_release_locked() derefs cmd_chain->command_count without a NULL check; guard with if (cmd_chain) consistent with all other call sites in the file. - aie4_hwctx.c: submit_one_cmd() derefs dpu->chained immediately after get_payload with no NULL check; return -ENOMEM if payload is NULL. - amdxdna_ctx.h: the inline amdxdna_cmd_get_payload() skips the BO size bounds check entirely when size==NULL; move the minimum-size guard before the if (size) block so it always executes, mirroring the fix applied to the upstream drivers/ tree in the previous commit.
There was a problem hiding this comment.
Pull request overview
This PR aims to harden command handling in the AMD XDNA driver by preventing NULL dereferences in VE2/AIE4 HW context paths and by adding an input-size guard in amdxdna_cmd_get_payload() to avoid out-of-bounds access on malformed command BOs.
Changes:
- Add a minimum-size validation in
amdxdna_cmd_get_payload()(both thesrc/driverinline version and thedrivers/accelimplementation). - Add NULL checks around payload parsing in VE2 job release and AIE4 command submission paths.
- Add a NULL check in
amdxdna_cmd_set_error()when handling chained commands.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/driver/amdxdna/ve2_hwctx.c |
Adds NULL guard before dereferencing chain payload during job release accounting. |
src/driver/amdxdna/amdxdna_ctx.h |
Adds a BO-size check in the inline amdxdna_cmd_get_payload() helper. |
src/driver/amdxdna/aie4_hwctx.c |
Adds NULL guard after parsing DPU payload in submit_one_cmd(). |
drivers/accel/amdxdna/amdxdna_ctx.c |
Adds a BO-size check in amdxdna_cmd_get_payload() and NULL guard in amdxdna_cmd_set_error() for chain payload. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else | ||
| num_masks = 1 + FIELD_GET(AMDXDNA_CMD_EXTRA_CU_MASK, cmd->header); | ||
|
|
||
| if (abo->mem.size < offsetof(struct amdxdna_cmd, data[num_masks])) |
There was a problem hiding this comment.
abo->mem.size is 4K aligned. So it will never smaller than offsetof(struct amdxdna_cmd, data[num_masks]).
How about adding a comment to avoid the future AI scans?
|
|
||
| if (amdxdna_cmd_get_op(abo) == ERT_CMD_CHAIN) { | ||
| cc = amdxdna_cmd_get_payload(abo, NULL); | ||
| if (!cc) |
There was a problem hiding this comment.
here, cmd != NULL and size == NULL. So amdxdna_cmd_get_payload() will never return NULL, correct?
Fixed a bug details as follws:
fix null deref in ve2/aie4 hwctx and src tree payload inline