Re: [PATCH v7 06/10] PCI: dwc: Implement endpoint DMA channel delegation

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing validation in dw_pcie_ep_reclaim_dma_chan leading to unsafe downcast and bypass of VF restrictions
--

--- Patch [6]: [PATCH v7 06/10] PCI: dwc: Implement endpoint DMA channel delegation ---
commit ade184c0035acad80c032f5a61a7726af2557e6c
Author: Koichiro Den <[email protected]>
PCI: dwc: Implement endpoint DMA channel delegation

Implement the EPC DMA channel delegation operations for DesignWare endpoint
controllers. Verify that the DMA engine channel reserved by the endpoint
function belongs to the integrated DesignWare eDMA instance, then hand its
interrupt routing to the host.

> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c

[ ... ]

> +static int dw_pcie_ep_delegate_dma_chan(struct pci_epc *epc, u8 func_no,
> +					u8 vfunc_no, struct dma_chan *chan)
> +{
> +	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +	struct dw_edma_chip *edma = &pci->edma;
> +	int ret;
> +
> +	ret = dw_pcie_ep_check_edma_vfunc(vfunc_no);
> +	if (ret)
> +		return ret;
> +
> +	if (!edma->dw)
> +		return -ENODEV;
> +
> +	if (!chan || chan->device->dev != edma->dev)
> +		return -EINVAL;
> +
> +	return dw_edma_delegate_chan(chan);
> +}
> +
> +static void dw_pcie_ep_reclaim_dma_chan(struct pci_epc *epc, u8 func_no,
> +					u8 vfunc_no, struct dma_chan *chan,
> +					bool quiesce)
> +{
> +	dw_edma_reclaim_chan(chan, quiesce);

[Severity: High]
Should this function validate the channel owner and virtual function status 
before passing it to dw_edma_reclaim_chan(), similar to the checks performed 
in dw_pcie_ep_delegate_dma_chan()?

If an endpoint function driver calls this with a generic dma_chan from a
different DMA controller (for example, during error unwinding), couldn't 
dw_edma_reclaim_chan() perform an unsafe downcast when it unconditionally 
processes the pointer?

Additionally, omitting dw_pcie_ep_check_edma_vfunc() appears to allow virtual 
functions to manipulate channels they are restricted from accessing during 
delegation.

> +}

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