Re: [PATCH] ntb: epf: fix PCI resource cleanup on init error path

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The driver reads scratchpad counts and offsets directly from untrusted device MMIO memory without validating them against the mapped BAR size, allowing a malicious PCI device to cause out-of-bounds memory accesses.
- [High] Off-by-one error in `ntb_epf_mw_to_bar()` allows out-of-bounds array access.
--

commit b593a66e63f557a346bb27a3a7845733ec998c5b
Author: Shashwat Agrawal <shashwatagrawal473@gmail.com>

ntb: epf: fix PCI resource cleanup on init error path
    
After pci_request_regions() succeeds, failures from setting the DMA
mask or mapping BARs jumped to a path that only called
pci_disable_device(). This left PCI regions claimed and BARs already
mapped with pci_iomap() were not unmapped. This patch releases regions
on those failures and unwinds iomaps in reverse order.

> diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c
> index c47607d4f8a7f..b42fb5f9fa75d 100644
> --- a/drivers/ntb/hw/epf/ntb_hw_epf.c
> +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c
[ ... ]
> @@ -699,7 +699,7 @@ static int ntb_epf_init_pci(struct ntb_epf_dev *ndev,
>  						ndev->barno_map[BAR_PEER_SPAD], 0);
>  		if (!ndev->peer_spad_reg) {
>  			ret = -EIO;
> -			goto err_pci_regions;
> +			goto err_ctrl_map;
>  		}
>  	} else {
>  		spad_sz = 4 * readl(ndev->ctrl_reg + NTB_EPF_SPAD_COUNT);
>  		spad_off = readl(ndev->ctrl_reg + NTB_EPF_SPAD_OFFSET);
>  		ndev->peer_spad_reg = ndev->ctrl_reg + spad_off  + spad_sz;

[Severity: High]
This is a pre-existing issue, but does this code properly validate the
scratchpad count and offset before using them?

Looking at ntb_epf_init_pci(), the untrusted NTB_EPF_SPAD_COUNT and
NTB_EPF_SPAD_OFFSET values are read directly from device MMIO and added to
ndev->ctrl_reg without being validated against the mapped BAR size.

A maliciously large count or offset from the endpoint device could cause
spad_sz and peer_spad_reg to far exceed the PCI BAR size.

Later, ntb_epf_init_dev() trusts NTB_EPF_SPAD_COUNT as an upper bound, meaning
clients iterating over this count with ntb_epf_peer_spad_read/write() will
dereference an out-of-bounds pointer.

This allows a maliciously controlled remote endpoint to cause unmapped or
arbitrary physical memory access, leading to a kernel panic or memory
corruption.

>  	}
>  
>  	ndev->db_reg = pci_iomap(pdev, ndev->barno_map[BAR_DB], 0);

[ ... ]

[Severity: High]
This is a pre-existing issue, but does ntb_epf_mw_to_bar() have an off-by-one
error in its bounds checking?

Looking at ntb_epf_mw_to_bar():

	if (idx < 0 || idx > ndev->mw_count) {
		dev_err(dev, "Unsupported Memory Window index %d\n", idx);
		return -EINVAL;
	}

	return ndev->barno_map[BAR_MW1 + idx];

The bounds check incorrectly permits idx == ndev->mw_count.

If an NTB client driver requests an operation using this maximum index,
BAR_MW1 + idx will evaluate to a value exceeding the size of the
ndev->barno_map array.

Returning a garbage BAR number here and passing it to pci_resource_len() or
pci_resource_start() causes an out-of-bounds array access, potentially
returning arbitrary attacker-controlled offsets that lead to a kernel crash.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702070628.19840-1-shashwatagrawal473@gmail.com?part=1