Re: [PATCH v3 3/5] PCI: endpoint: Add API to delegate EPC DMA channels to the host

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Error pointer dereference in `pci_epc_delegate_dma_chan` due to incorrect validation order.
--

commit b5fefb01d3a350b55650bc026830cacbecfcc978
Author: Koichiro Den <den@valinux.co.jp>

PCI: endpoint: Add API to delegate EPC DMA channels to the host

This commit adds pci_epc_delegate_dma_chan() and pci_epc_reclaim_dma_chan()
to allow generic endpoint functions to delegate integrated DMA channels to
the host. It introduces matching EPC operations and uses an opaque handle
for channel state management to avoid backend-specific dependencies.

> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 831b40458dcd8..0f71c4633ee54 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c

[ ... ]

> @@ -236,6 +243,99 @@ int pci_epc_get_aux_resources(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>  }
>  EXPORT_SYMBOL_GPL(pci_epc_get_aux_resources);
>  
> +/**
> + * pci_epc_delegate_dma_chan() - delegate an EPC-owned DMA channel to the host

[ ... ]

> +int pci_epc_delegate_dma_chan(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> +			      enum pci_epc_aux_dma_dir dir, u16 hw_ch,
> +			      struct pci_epc_dma_chan **chan)
> +{
> +	struct pci_epc_dma_chan *epc_chan;
> +	void *data = NULL;
> +	int ret;
> +
> +	if (!chan)
> +		return -EINVAL;
> +	*chan = NULL;
> +
> +	if (!epc || !epc->ops)
> +		return -EINVAL;

[Severity: High]
If a caller passes an error pointer for epc, will the !epc->ops check
trigger a crash?

Functions like pci_epc_get() return ERR_PTR(-EINVAL) on failure rather
than NULL. If epc is an error pointer, the !epc check evaluates to false,
causing execution to proceed to the !epc->ops check. Dereferencing epc->ops
on an error pointer will trigger a kernel crash.

> +
> +	if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
> +		return -EINVAL;

The subsequent call to pci_epc_function_is_valid() internally uses
IS_ERR_OR_NULL(epc) to safely handle error pointers, but placing the ops
dereference before it circumvents this safety.

Would it be better to move the pci_epc_function_is_valid() check before
dereferencing epc->ops, or explicitly check IS_ERR_OR_NULL(epc) first?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620170438.3756593-1-den@valinux.co.jp?part=3