All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Koichiro Den" <den@valinux.co.jp>
Cc: dmaengine@vger.kernel.org, linux-pci@vger.kernel.org,
	vkoul@kernel.org, Frank.Li@kernel.org
Subject: Re: [PATCH v4 3/6] PCI: endpoint: Add API to delegate EPC DMA channels to the host
Date: Fri, 10 Jul 2026 08:32:05 +0000	[thread overview]
Message-ID: <20260710083206.198D31F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260710082156.2395844-4-den@valinux.co.jp>

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but pre-existing API functions `pci_epc_get_aux_resources()` and `pci_epc_get_aux_resources_count()` unconditionally dereference `epc` before checking if it is an `ERR_PTR`.
--

--- Patch [3]: [PATCH v4 3/6] PCI: endpoint: Add API to delegate EPC DMA channels to the host ---
commit 1d374b51cba0f1ce466c2d806368e6604aa12539
Author: Koichiro Den <den@valinux.co.jp>

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

Some endpoint functions expose an EPC-integrated DMA controller to the
host. The endpoint function should not need to know the backend-specific
mechanism used to reserve a channel locally and hand its programming
interface to the host.

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

[ ... ]

> @@ -236,6 +243,101 @@ 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
> + * @epc: EPC device
> + * @func_no: function number
> + * @vfunc_no: virtual function number
> + * @dir: DMA channel direction relative to the endpoint
> + * @hw_ch: hardware channel number
> + * @chan: output delegated-channel handle
> + *
> + * Some EPC backends integrate DMA channels that can be exposed to the host.
> + * This helper asks the backend to reserve the specified channel locally and
> + * place it in a state where the host driver may program it through the exposed
> + * register window.
> + *
> + * Return: 0 on success, -EOPNOTSUPP if the backend does not support DMA channel
> + * delegation, or another -errno on failure.
> + */
> +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 (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
> +		return -EINVAL;

[Severity: High]
This is a pre-existing issue, but while you correctly call
pci_epc_function_is_valid() here first to avoid dereferencing an error
pointer, could the pre-existing pci_epc_get_aux_resources() and
pci_epc_get_aux_resources_count() functions be crashing for the same
reason?

Looking at pci_epc_get_aux_resources() in drivers/pci/endpoint/pci-epc-core.c:

	if (!epc || !epc->ops)
		return -EINVAL;

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

If epc is an ERR_PTR, the !epc NULL check passes, and it crashes when
dereferencing epc->ops before reaching pci_epc_function_is_valid().

Does it make sense to update these sibling functions as well to ensure they
safely handle an ERR_PTR?

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

  reply	other threads:[~2026-07-10  8:32 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-10  8:21 [PATCH v4 0/6] PCI: endpoint: Expose endpoint DMA resources (part 2/3) Koichiro Den
2026-07-10  8:21 ` [PATCH v4 1/6] PCI: endpoint: Define endpoint DMA BAR metadata format Koichiro Den
2026-07-10  8:24   ` sashiko-bot
2026-07-10  8:21 ` [PATCH v4 2/6] PCI: endpoint: Add DMA auxiliary resource metadata Koichiro Den
2026-07-10  8:27   ` sashiko-bot
2026-07-10  8:21 ` [PATCH v4 3/6] PCI: endpoint: Add API to delegate EPC DMA channels to the host Koichiro Den
2026-07-10  8:32   ` sashiko-bot [this message]
2026-07-10  8:21 ` [PATCH v4 4/6] PCI: dwc: Expose endpoint DMA resources Koichiro Den
2026-07-10  8:29   ` sashiko-bot
2026-07-10  8:21 ` [PATCH v4 5/6] dmaengine: dw-edma: Add delegated channel request helpers Koichiro Den
2026-07-10 10:43   ` sashiko-bot
2026-07-10  8:21 ` [PATCH v4 6/6] PCI: dwc: Implement endpoint DMA channel delegation Koichiro Den
2026-07-10  8:30   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260710083206.198D31F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=den@valinux.co.jp \
    --cc=dmaengine@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.