Re: [PATCH V4 17/19] accel/amdxdna: Enable AIE4 firmware logging to DRAM

[sashiko-bot@kernel.org]

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 <yidong.zhang@amd.com>

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/20261009225343.3079074-1-yidong.zhang@amd.com?part=17