Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] amdxdna: DMA Use-After-Free of fw_log_buf on hardware start failure --
commit 399dd24d5b4823ad10e211886f1046c8d43ff630 Author: David Zhang <[email protected]> accel/amdxdna: Enable AIE4 firmware logging to DRAM This commit allocates a DRAM buffer for firmware logging and configures the device to use it with a less verbose log level. > diff --git a/drivers/accel/amdxdna/aie4_pci.c > b/drivers/accel/amdxdna/aie4_pci.c > index 7f4238f76bc22..e09d24d85cdfa 100644 > --- a/drivers/accel/amdxdna/aie4_pci.c > +++ b/drivers/accel/amdxdna/aie4_pci.c [ ... ] > @@ -953,6 +998,8 @@ static int aie4_pf_init(struct amdxdna_dev *xdna) > if (ret) > return ret; > > + aie4_alloc_fw_log_buffer(xdna->dev_handle); > + > ret = aie4_pf_hw_start(xdna->dev_handle); > if (ret) > goto free_work_buf; > @@ -960,6 +1007,7 @@ static int aie4_pf_init(struct amdxdna_dev *xdna) > return 0; > > free_work_buf: > + aie4_free_fw_log_buffer(xdna->dev_handle); [Severity: High] Can this lead to a use-after-free of the DMA buffer if the hardware fails to start? During aie4_pf_hw_start(), aie4_config_fw() is called which instructs the firmware to begin asynchronously DMA-ing logs into fw_log_buf. If a subsequent step in aie4_pf_hw_start() fails (such as aie4_restore_power_mode() timing out), the error path attempts to stop the firmware via aie4_fw_stop(). However, if the PSP teardown inside aie_psp_stop() also times out: aie_psp_stop() { ... ret = psp_exec(psp, reg_vals); if (ret) drm_err(psp->ddev, "release tmr failed, ret %d", ret); } It returns without successfully halting the active firmware. We then reach this error path in aie4_pf_init() and free fw_log_buf back to the page allocator. Does the active firmware continue writing logs into this freed memory? > aie4_free_work_buffer(xdna->dev_handle); > return ret; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=17
