Re: [PATCH 1/3] PCI: endpoint: pci-epf-vntb: Pass PF/VF number when BAR programming

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] An off-by-one loop initialization error in `epf_ntb_init_epc_bar()` leaves memory window BARs beyond the first one unassigned (`NO_BAR` or `-1`). This results in negative array indexing and out-of-bounds memory accesses when `epf_ntb_mw_bar_init()` and `vntb_epf_mw_set_trans()` index into `ntb->epf->bar[barno]`.
- [High] Missing NULL checks for `epc_features` returned by `pci_epc_get_features()`, leading to a kernel panic if the underlying EPC driver does not implement the `.get_features` callback.
- [High] The virtual PCI driver `vntb_pci_driver` lacks a `.remove` callback, resulting in resource leaks of the virtual PCI bus and Use-After-Free of the NTB device structures upon module unload or EPF unbind.
--

--- Patch [1]: [PATCH 1/3] PCI: endpoint: pci-epf-vntb: Pass PF/VF number when BAR programming ---
commit fc9ce298c179bf87e88da3f2964f2fbbd870806a
Author: Koichiro Den <den@valinux.co.jp>
PCI: endpoint: pci-epf-vntb: Pass PF/VF number when BAR programming

This commit updates vntb_epf_mw_set_trans() to pass the endpoint function's
own func_no and vfunc_no instead of hardcoded zeros when programming the
memory-window BAR. This ensures the BAR is correctly configured when the
virtual NTB endpoint function is bound to a function other than PF0.

> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index c3caec927d748..fba65abfb6b21 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -1427,7 +1427,8 @@ static int vntb_epf_mw_set_trans(struct ntb_dev *ndev, int pidx, int idx,
>  	epf_bar->barno = barno;
>  	epf_bar->size = size;

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for barno to
be -1 here, leading to an out-of-bounds write?

Looking at epf_ntb_init_epc_bar(), the loop iterating over optional memory
window BARs incorrectly starts from BAR_MW1 instead of BAR_MW2:

drivers/pci/endpoint/functions/pci-epf-vntb.c:epf_ntb_init_epc_bar() {
	...
	for (bar = BAR_MW1, i = 1; i < num_mws; bar++, i++) {
	...
}

If a user configures num_mws > 1, this leaves BAR_MW2 unassigned (-1). Later,
when vntb_epf_mw_set_trans() is called for idx = 1, barno is evaluated as -1.
Will epf_bar = &ntb->epf->bar[barno] then access out of bounds and corrupt
memory?

> -	ret = pci_epc_set_bar(ntb->epf->epc, 0, 0, epf_bar);
> +	ret = pci_epc_set_bar(ntb->epf->epc, ntb->epf->func_no,
> +			      ntb->epf->vfunc_no, epf_bar);
>  	if (ret) {
>  		dev_err(dev, "failure set mw trans\n");
>  		return ret;

Also, while reviewing this file, I noticed a few other pre-existing issues:

[Severity: High]
This is a pre-existing issue, but how is the case where epc_features returns
NULL handled?

drivers/pci/endpoint/functions/pci-epf-vntb.c:epf_ntb_configure_interrupt() {
	...
	epc_features = pci_epc_get_features(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no);

	if (!(epc_features->msix_capable || epc_features->msi_capable)) {
	...
}

If the underlying EPC driver lacks the .get_features callback,
pci_epc_get_features() returns NULL, which would cause a NULL pointer
dereference here. A similar dereference happens in
epf_ntb_config_spad_bar_alloc().


[Severity: High]
This is a pre-existing issue, but does vntb_pci_driver intentionally omit 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,
	};

When epf_ntb_unbind() unregisters this driver:

drivers/pci/endpoint/functions/pci-epf-vntb.c:epf_ntb_unbind() {
	...
	pci_unregister_driver(&vntb_pci_driver);
}

Since there is no .remove callback, ntb_unregister_device() is never called to
clean up the virtual PCI bus and its associated devices. Could this lead to a
resource leak and dangling sysdata pointers upon module unload or EPF unbind?

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