[PATCH 0/2] misc: pci_endpoint_test: doorbell fixes

[PATCH 0/2] misc: pci_endpoint_test: doorbell fixes

[carlos.bilbao]

From: Carlos Bilbao <carlos.bilbao@kernel.org>

pci_endpoint_test_doorbell() reads the BAR number from the endpoint's test
register space and uses it directly as an index into test->bar[] without
bounds checking.  Since the value is a raw u32 written by the endpoint
firmware, any value is possible; values >= PCI_STD_NUM_BARS result in an
out-of-bounds array access.

Patch 1 adds the missing bounds check.

Patch 2 removes a dead read of the same register that precedes the writel
sequence; the DB_BAR register carries no read side effect and the assigned
value is unconditionally overwritten before use.

 Carlos Bilbao (2):
   misc: pci_endpoint_test: validate BAR index in doorbell test
   misc: pci_endpoint_test: remove dead BAR read before doorbell trigger

---
 drivers/misc/pci_endpoint_test.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

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

[carlos.bilbao]

From: Carlos Bilbao <carlos.bilbao@kernel.org>

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@kernel.org>
---
 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;
+	}
 
 	writel(data, test->bar[bar] + addr);
 
-- 
2.43.0

[PATCH 2/2] misc: pci_endpoint_test: remove dead BAR read before doorbell trigger

[carlos.bilbao]

From: Carlos Bilbao <carlos.bilbao@kernel.org>

The assignment before the writel sequence is dead code (bar is
unconditionally overwritten by the re-read immediately after) so remove the
call entirely.

Note that the DB_BAR register is a plain value written by the endpoint
firmware; reading it carries no side effect.

Signed-off-by: Carlos Bilbao (Lambda) <carlos.bilbao@kernel.org>
---
 drivers/misc/pci_endpoint_test.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index 276bed3f1c18..07c5f19d5c5a 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -1081,7 +1081,6 @@ static int pci_endpoint_test_doorbell(struct pci_endpoint_test *test)
 
 	data = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_DATA);
 	addr = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_OFFSET);
-	bar = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_BAR);
 
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
-- 
2.43.0

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@kernel.org wrote:
> From: Carlos Bilbao <carlos.bilbao@kernel.org>
> 
> 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@kernel.org>
> ---
>  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
> 
> 

Re: [PATCH 2/2] misc: pci_endpoint_test: remove dead BAR read before doorbell trigger

[Koichiro Den]

On Fri, Apr 03, 2026 at 06:20:02PM -0700, carlos.bilbao@kernel.org wrote:
> From: Carlos Bilbao <carlos.bilbao@kernel.org>
> 
> The assignment before the writel sequence is dead code (bar is
> unconditionally overwritten by the re-read immediately after) so remove the
> call entirely.
> 
> Note that the DB_BAR register is a plain value written by the endpoint
> firmware; reading it carries no side effect.
> 
> Signed-off-by: Carlos Bilbao (Lambda) <carlos.bilbao@kernel.org>

Reviewed-by: Koichiro Den <den@valinux.co.jp>

> ---
>  drivers/misc/pci_endpoint_test.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index 276bed3f1c18..07c5f19d5c5a 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -1081,7 +1081,6 @@ static int pci_endpoint_test_doorbell(struct pci_endpoint_test *test)
>  
>  	data = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_DATA);
>  	addr = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_OFFSET);
> -	bar = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_DB_BAR);
>  
>  	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
>  	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
> -- 
> 2.43.0
> 
> 

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

[Carlos Bilbao]

Hello,

On 4/6/26 09:39, Koichiro Den wrote:
> On Fri, Apr 03, 2026 at 06:20:01PM -0700, carlos.bilbao@kernel.org wrote:
>> From: Carlos Bilbao <carlos.bilbao@kernel.org>
>>
>> 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@kernel.org>
>> ---
>>   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.


Thanks for the feedback. I'll send a v2 with changes here to handle bar
< BAR_0 as well as dropping the Fixes tag.


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

Thanks,

Carlos