Re: [PATCH V2 4/8] dmaengine: zynqmp_dma: Fix stale kerneldoc comments

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Race condition causing list corruption and double-frees in `zynqmp_dma_chan_desc_cleanup()` when client callbacks terminate transfers.
- [High] PM runtime usage counter leak in `zynqmp_dma_probe()` error paths.
--

--- Patch [4]: [PATCH V2 4/8] dmaengine: zynqmp_dma: Fix stale kerneldoc comments ---
commit 50bd84a068af6c47ff8d29181173cde44baa590d
Author: Golla Nagendra <[email protected]>

dmaengine: zynqmp_dma: Fix stale kerneldoc comments

Correct kerneldoc comments that no longer matched the code.

> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index 22b517c570038..a2fbefbb43b65 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c

[ ... ]

> @@ -617,7 +617,6 @@ static void zynqmp_dma_start_transfer(struct zynqmp_dma_chan *chan)
>  	zynqmp_dma_start(chan);
>  }
>  
> -
>  /**
>   * zynqmp_dma_chan_desc_cleanup - Cleanup the completed descriptors
>   * @chan: ZynqMP DMA channel

[Severity: High]
This is a pre-existing issue, but could there be a race condition in
zynqmp_dma_chan_desc_cleanup() if the client callback terminates the transfer?

When iterating over chan->done_list with list_for_each_entry_safe(), the lock
is dropped before invoking the callback:

	spin_unlock_irqrestore(&chan->lock, irqflags);
	dmaengine_desc_callback_invoke(&cb, NULL);
	spin_lock_irqsave(&chan->lock, irqflags);

If the client callback calls dmaengine_terminate_all(), it acquires the lock,
empties the done_list, and moves pending descriptors to the free_list.

When zynqmp_dma_chan_desc_cleanup() resumes, it will continue iterating using
the stale next pointer which now points into free_list. This could lead to a
double-free when zynqmp_dma_free_descriptor() is called.

[ ... ]

[Severity: High]
This is another pre-existing issue, but does zynqmp_dma_probe() leak a PM
runtime usage counter in its error paths?

If initialization fails after pm_runtime_resume_and_get(zdev->dev) succeeds,
the error path jumps to err_disable_pm:

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_probe() {
    ...
err_disable_pm:
	if (!pm_runtime_enabled(zdev->dev))
		zynqmp_dma_runtime_suspend(zdev->dev);
	pm_runtime_disable(zdev->dev);
	return ret;
}

It calls pm_runtime_disable(), but it doesn't seem to call
pm_runtime_put_noidle() to drop the usage counter acquired earlier.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4