Re: [PATCH v5 1/2] PCI: endpoint: pci-epf-vntb: Use a local epf pointer in interrupt setup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci-epf-vntb: missing NULL check for epc_features causes dereference crash
--

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

PCI: endpoint: pci-epf-vntb: Use a local epf pointer in interrupt setup

This commit introduces a local epf pointer to simplify device access and
shorten repeated pointer dereferences in epf_ntb_configure_interrupt.

> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index a11f9260ee860..d74402ea10bf6 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -545,12 +545,14 @@ static int epf_ntb_config_spad_bar_alloc(struct epf_ntb *ntb)
>  static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
>  {
>  	const struct pci_epc_features *epc_features;
> +	struct pci_epf *epf = ntb->epf;
>  	struct device *dev;
>  	int ret;
>  
> -	dev = &ntb->epf->dev;
> +	dev = &epf->dev;
>  
> -	epc_features = pci_epc_get_features(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no);
> +	epc_features = pci_epc_get_features(epf->epc, epf->func_no,
> +					    epf->vfunc_no);
>  
>  	if (!(epc_features->msix_capable || epc_features->msi_capable)) {

[Severity: High]
Could this regression result in a NULL pointer dereference crash?

Looking at pci_epc_get_features() in pci-epc-core.c, it returns NULL for
invalid configurations or if the driver is missing get_features operations:

pci_epc_get_features() {
        if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
                return NULL;

        if (!epc->ops->get_features)
                return NULL;
        ...
}

If a user configures the endpoint with an invalid function number via
configfs, pci_epc_get_features() will return NULL. Because the returned
epc_features pointer is unconditionally dereferenced in the if statement
above, would it be safer to add a NULL check before inspecting the
structure?

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