Re: [PATCH v3 19/23] dmaengine: sdxi: Provide context start and stop APIs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Silent failures in `sdxi_stop_cxt` do not stop the hardware but provide no error to the caller, leading to subsequent use-after-free of context resources.
- [High] DMA memory use-after-free triggered by `sdxi_completion_poll` timeout in `sdxi_start_cxt` and `sdxi_stop_cxt`.
--

commit 6a36436cc92c4877bfb050c28457f1ac8dbaf6a2
Author: Nathan Lynch <nathan.lynch@amd.com>

dmaengine: sdxi: Provide context start and stop APIs

This commit introduces high-level context start and stop APIs for SDXI.
These APIs encapsulate submitting administrative descriptors and waiting
for completion signaling.

> diff --git a/drivers/dma/sdxi/context.c b/drivers/dma/sdxi/context.c
> index 9b0984842d9a7..afd3414163428 100644
> --- a/drivers/dma/sdxi/context.c
> +++ b/drivers/dma/sdxi/context.c

[ ... ]

> +int sdxi_start_cxt(struct sdxi_cxt *cxt)
> +{
> +	struct sdxi_cxt *adm = to_admin_cxt(cxt);
> +	struct sdxi_desc *desc;

[ ... ]

> +	sdxi_completion_attach(desc, sc);
> +	sdxi_desc_make_valid(desc);
> +	sdxi_cxt_push_doorbell(adm, sdxi_ring_resv_dbval(&resv));
> +
> +	return sdxi_completion_poll(sc);
> +}

[Severity: High]
If sdxi_completion_poll() times out here, does this return early and
trigger the scope-based cleanup of the sdxi_completion structure?

If the hardware operation is just delayed and not actually aborted, could
the hardware later write to this completion block address after it has
been freed back to the DMA pool?

> +void sdxi_stop_cxt(struct sdxi_cxt *cxt)
> +{
> +	struct sdxi_cxt *adm = to_admin_cxt(cxt);
> +	struct sdxi_desc *stop, *sync;
> +	struct sdxi_ring_resv resv;
> +	int err;
> +
> +	might_sleep();
> +
> +	struct sdxi_completion *sc __free(sdxi_completion) =
> +		sdxi_completion_alloc(cxt->sdxi);
> +
> +	if (!sc)
> +		return;

[Severity: Critical]
Since sdxi_stop_cxt() returns void, do these silent early returns leave
the caller unaware that the hardware context was never actually stopped?

If a caller proceeds to tear down the context and free its backing
descriptor ring and memory, could the active DMA engine continue processing
descriptors and corrupt memory?

> +	/* This is not how to stop the admin context. */
> +	if (WARN_ON(adm == cxt))
> +		return;
> +
> +	err = sdxi_ring_reserve(adm->ring_state, 2, &resv);
> +	if (WARN_ON_ONCE(err))
> +		return;

[Severity: Critical]
Similarly, if reserving space in the admin ring fails here, the function
returns silently without stopping the hardware. Could this lead to the same
scenario where context memory is freed while still in use by the hardware?

> +	stop = sdxi_ring_resv_next(&resv);
> +	sync = sdxi_ring_resv_next(&resv);

[ ... ]

> +	sdxi_completion_attach(sync, sc);
> +	sdxi_desc_make_valid(stop);
> +	sdxi_desc_make_valid(sync);
> +	sdxi_cxt_push_doorbell(adm, sdxi_ring_resv_dbval(&resv));
> +
> +	WARN_ON(sdxi_completion_poll(sc));
> +}

[Severity: High]
Just as in sdxi_start_cxt(), if the poll function times out here, does the
early return free the in-use DMA memory without aborting the pending DMA
stop/sync operation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605-sdxi-base-v3-0-4d38ca2bdffe@amd.com?part=19