Re: [PATCH 04/11] PCI: endpoint: pci-epf-vntb: Implement MW group translation callbacks

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] DMA Use-After-Free due to `vntb_epf_mw_clear_trans_group()` and `vntb_epf_mw_clear_trans()` returning success (0) without actually disabling the inbound hardware translations.
- [High] The `vntb_epf_mw_set_trans()` function incorrectly hardcodes `func_no` and `vfunc_no` to 0 when invoking `pci_epc_set_bar()`, whereas other EPF APIs (including the newly added group API) correctly use the endpoint's configured function numbers.
- [High] The `vntb_pci_driver` structure is missing a `.remove` callback, causing NTB device leaks and use-after-free conditions when the endpoint function is unbound.
--

--- Patch [4]: [PATCH 04/11] PCI: endpoint: pci-epf-vntb: Implement MW group translation callbacks ---
commit 419429d9534b2c856c11cb4a4a4107e84a87fe8e
Author: Koichiro Den <den@valinux.co.jp>

PCI: endpoint: pci-epf-vntb: Implement MW group translation callbacks

All RC-to-EP MWs in a packed group share one inbound BAR. Setting each
MW separately would keep replacing that BAR and could expose a partial
group. Add group callbacks that program the shared BAR once for the
complete backing range.

> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index f22459e5a8c69..0c72b907c2896 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -1622,6 +1622,31 @@ static int vntb_epf_mw_clear_trans(struct ntb_dev *ntb, int pidx, int idx)
>  	return 0;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but while looking at this context,
I noticed that the existing vntb_epf_mw_set_trans() hardcodes the function
numbers to 0:

drivers/pci/endpoint/functions/pci-epf-vntb.c:vntb_epf_mw_set_trans() {
    ...
	ret = pci_epc_set_bar(ntb->epf->epc, 0, 0, epf_bar);
    ...
}

Does this incorrectly configure the BAR for function 0 when the virtual NTB
device is bound to a non-zero endpoint function or virtual function?

If the device is configured via configfs with a non-zero function, could this
lead to silent failures in memory window translation or corrupt the ATU
configuration of function 0?

> +
> +static int
> +vntb_epf_mw_set_trans_group(struct ntb_dev *ndev, int pidx, int widx,
> +			    dma_addr_t addr, resource_size_t size)
> +{
[ ... ]
> +	return pci_epc_set_bar(ntb->epf->epc, ntb->epf->func_no,
> +			       ntb->epf->vfunc_no, epf_bar);
> +}
> +
> +static int vntb_epf_mw_clear_trans_group(struct ntb_dev *ndev, int pidx,
> +					 int widx)
> +{
> +	return 0;
> +}

[Severity: Critical]
This is a pre-existing issue perpetuated here by the new group API, but does
returning 0 without actually disabling the inbound hardware translations create a
DMA use-after-free window?

When an NTB client unmaps a memory window group and frees the backing memory,
the hardware BAR mapping will remain active.

If the local host frees this memory, won't the endpoint hardware continue
translating incoming PCIe accesses to the freed physical address, allowing a
remote peer to access arbitrary new allocations on the endpoint?

[Severity: High]
This is another pre-existing issue, but I noticed the vntb_pci_driver structure
is missing a .remove callback:

drivers/pci/endpoint/functions/pci-epf-vntb.c:
static struct pci_driver vntb_pci_driver = {
	.name           = "pci-vntb",
	.id_table       = pci_vntb_table,
	.probe          = pci_vntb_probe,
};

Without a .remove callback, ntb_unregister_device() is never called when the
EPF device is unbound via configfs.

Does this cause NTB device leaks and driver state use-after-free?

If the endpoint function is unbound, won't the NTB clients remain active and
bound to a device whose backing EPF structures, DMA masks, and hardware
mappings have been destroyed?

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