Re: [PATCH 1/2] misc: pci_endpoint_test: validate BAR index in doorbell test

[Koichiro Den]

On Fri, Apr 03, 2026 at 06:20:01PM -0700, carlos.bilbao@xxxxxxxxxx wrote:
> From: Carlos Bilbao <carlos.bilbao@xxxxxxxxxx>
> 
> pci_endpoint_test_doorbell() reads the BAR number directly from an endpoint
> test register and uses it as an index into test->bar[] without bounds
> checking. Since the value is a raw u32 from device MMIO, any value is
> possible and if greater than or equal to PCI_STD_NUM_BARS the access goes
> out of bounds.
> 
> Add a bounds check before dereferencing test->bar[bar].
> 
> Fixes: eefb83790a0d ("misc: pci_endpoint_test: Add doorbell test case")
> Signed-off-by: Carlos Bilbao (Lambda) <carlos.bilbao@xxxxxxxxxx>
> ---
>  drivers/misc/pci_endpoint_test.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index 74ab5b5b9011..276bed3f1c18 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -1089,6 +1089,11 @@ static int pci_endpoint_test_doorbell(struct pci_endpoint_test *test)
>  	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_STATUS, 0);
>  
>  	bar = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_BAR);
> +	if (bar >= PCI_STD_NUM_BARS) {
> +		dev_err(dev, "BAR %u reported by endpoint out of range [0, %u]\n",
> +			bar, PCI_STD_NUM_BARS - 1);
> +		return -EINVAL;
> +	}

Hi,

On the EP-side (pci-epf-test.c), doorbell_bar is set to NO_BAR or a valid BAR
number in the range 0..(PCI_STD_NUM_BARS-1), so if we add such a safeguard here,
we might as well handle NO_BAR explicitly.

That said, I'm not sure this check is really needed. At this point
COMMAND_ENABLE_DOORBELL should have succeeded, so bar is expected to always be
within the valid range. Whether such defensive checks are desirable probably
depends on the maintainers' preference. In any case, I don't think a Fixes tag
is appropriate here.

Also, even if the intention is also to guard against an "all 1's" value scenario
on errors, then I think it might be better to handle that in a more centralized
way, since the same concern would apply to other similar reads as well.

Best regards,
Koichiro

>  
>  	writel(data, test->bar[bar] + addr);
>  
> -- 
> 2.43.0
> 
>