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

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()` an=
d `vntb_epf_mw_clear_trans()` returning success (0) without actually disabl=
ing 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 E=
PF 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, c=
ausing NTB device leaks and use-after-free conditions when the endpoint fun=
ction is unbound.
--

--- Patch [4]: [PATCH 04/11] PCI: endpoint: pci-epf-vntb: Implement MW grou=
p translation callbacks ---
commit 419429d9534b2c856c11cb4a4a4107e84a87fe8e
Author: Koichiro Den <[email protected]>

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 contex=
t,
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 =3D 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 th=
is
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 cr=
eate a
DMA use-after-free window?

When an NTB client unmaps a memory window group and frees the backing memor=
y,
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 struc=
ture
is missing a .remove callback:

drivers/pci/endpoint/functions/pci-epf-vntb.c:
static struct pci_driver vntb_pci_driver =3D {
	.name           =3D "pci-vntb",
	.id_table       =3D pci_vntb_table,
	.probe          =3D 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?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803180439.1963=
[email protected]?part=3D4