Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The `aie2_cmd_wait()` function ignores both the return value of the fence wait and the fence's error status. If a DRM job aborts early (e.g., due to an error in the scheduler before hardware execution), the response handler is skipped and `cmd->result` remains its default zero value. Callers will incorrectly interpret this as a successful hardware execution and proceed to update internal states, leading to state inconsistencies. -- commit 77f1579d79492199ed29e6079ac11fa573ee9e04 Author: Lizhi Hou <[email protected]> accel/amdxdna: Fix use-after-free in debug BO command handling This commit fixes a use-after-free in debug BO command handling by introducing reference counting for drv_cmd objects. It transfers ownership to the job while it is in flight to ensure the command remains valid until the completion handler finishes. > diff --git a/drivers/accel/amdxdna/aie2_ctx.c > b/drivers/accel/amdxdna/aie2_ctx.c > index e9fbd8c143641..54486960cbf5e 100644 > --- a/drivers/accel/amdxdna/aie2_ctx.c > +++ b/drivers/accel/amdxdna/aie2_ctx.c [ ... ] > @@ -912,32 +926,39 @@ static int aie2_hwctx_cfg_debug_bo(struct amdxdna_hwctx > *hwctx, u32 bo_hdl, [ ... ] > - ret = amdxdna_cmd_submit(client, &cmd, AMDXDNA_INVALID_BO_HANDLE, > + ret = amdxdna_cmd_submit(client, cmd, AMDXDNA_INVALID_BO_HANDLE, > &bo_hdl, 1, hwctx->id, &seq); > if (ret) { > XDNA_ERR(xdna, "Submit command failed"); > - goto put_obj; > + goto put_cmd; > } > > aie2_cmd_wait(hwctx, seq); > - if (cmd.result) { > - XDNA_ERR(xdna, "Response failure 0x%x", cmd.result); > + if (cmd->result) { [Severity: High] This is a pre-existing issue, but does trusting cmd->result here correctly handle early job aborts? Looking at aie2_cmd_wait(), it calls dma_fence_wait_timeout() but ignores both the return value (timeouts) and dma_fence_get_error(out_fence). If a DRM job fails early in the scheduler before hardware execution, the fence might signal with an error without executing the hardware response handler. Since cmd->result remains its default zero value, will this cause aie2_hwctx_cfg_debug_bo() to falsely assume the hardware command succeeded and incorrectly update abo->assigned_hwctx? > + XDNA_ERR(xdna, "Response failure 0x%x", cmd->result); > ret = -EINVAL; > - goto put_obj; > + goto put_cmd; > } > > if (attach) -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
