[PATCH 2/4] PCI: endpoint: pci-epf-test: Sanity-check doorbell offset within BAR

Koichiro Den posted 4 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH 2/4] PCI: endpoint: pci-epf-test: Sanity-check doorbell offset within BAR
Posted by Koichiro Den 1 month, 2 weeks ago
pci-epf-test advertises the doorbell target to the RC as a BAR number
and an offset. The RC rings the doorbell with a single DWORD MMIO write
to BAR + offset.

For MSI/MSI-X-based doorbells, the message address is required to be
DWORD-aligned, so the computed offset should not straddle a BAR boundary
in normal operation.

However, with support for doorbells based on mechanisms other than
MSI/MSI-X (via pci_epf_alloc_doorbell()), the returned message address
may not necessarily be DWORD-aligned. In such a case, offset plus the
32-bit write width could cross the end of the BAR aperture. The offset
returned by pci_epf_align_inbound_addr() is guaranteed to be within the
BAR size, but this alone does not ensure that a 32-bit write starting at
that offset stays within the BAR.

Add a bounds check to ensure that the 32-bit doorbell write always stays
within the BAR aperture. While this should not trigger for
spec-compliant MSI/MSI-X addresses, it provides a defensive guard
against unexpected offsets from future doorbell implementations.

Suggested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 0cb7af0919dc..148a34e51f6b 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -761,6 +761,9 @@ static void pci_epf_test_enable_doorbell(struct pci_epf_test *epf_test,
 	if (ret)
 		goto err_doorbell_cleanup;
 
+	if (size_add(offset, sizeof(u32)) > epf->bar[bar].size)
+		goto err_doorbell_cleanup;
+
 	reg->doorbell_offset = cpu_to_le32(offset);
 
 	epf_test->db_bar.barno = bar;
-- 
2.51.0
Re: [PATCH 2/4] PCI: endpoint: pci-epf-test: Sanity-check doorbell offset within BAR
Posted by Niklas Cassel 1 month, 2 weeks ago
On Mon, Feb 16, 2026 at 12:09:12AM +0900, Koichiro Den wrote:
> pci-epf-test advertises the doorbell target to the RC as a BAR number
> and an offset. The RC rings the doorbell with a single DWORD MMIO write
> to BAR + offset.
> 
> For MSI/MSI-X-based doorbells, the message address is required to be
> DWORD-aligned, so the computed offset should not straddle a BAR boundary
> in normal operation.
> 
> However, with support for doorbells based on mechanisms other than
> MSI/MSI-X (via pci_epf_alloc_doorbell()), the returned message address
> may not necessarily be DWORD-aligned. In such a case, offset plus the
> 32-bit write width could cross the end of the BAR aperture. The offset
> returned by pci_epf_align_inbound_addr() is guaranteed to be within the
> BAR size, but this alone does not ensure that a 32-bit write starting at
> that offset stays within the BAR.
> 
> Add a bounds check to ensure that the 32-bit doorbell write always stays
> within the BAR aperture. While this should not trigger for
> spec-compliant MSI/MSI-X addresses, it provides a defensive guard
> against unexpected offsets from future doorbell implementations.

I think everything you write is true,
and I know that I suggested this...

But for the MMIO address, will it ever not be 32-bit aligned?

Even in the eDMA case, the eDMA registers are 32-bit aligned.

Did I perhaps have a brain fart and overthink this?


I guess theoretically, some future pci_epf_alloc_doorbell() implementation
could return something that is not 32-bit aligned...

But if we really want to add a safety check for that... perhaps a 32-bit
alignment check would be better suited to have in pci_epf_alloc_doorbell() ?


Perhaps this check is better added in pci_epf_alloc_doorbell() or
pci_epf_alloc_doorbell_embedded(), in the series that adds support for
embedded doorbells ?


Kind regards,
Niklas
Re: [PATCH 2/4] PCI: endpoint: pci-epf-test: Sanity-check doorbell offset within BAR
Posted by Koichiro Den 1 month, 2 weeks ago
On Mon, Feb 16, 2026 at 02:14:14PM +0100, Niklas Cassel wrote:
> On Mon, Feb 16, 2026 at 12:09:12AM +0900, Koichiro Den wrote:
> > pci-epf-test advertises the doorbell target to the RC as a BAR number
> > and an offset. The RC rings the doorbell with a single DWORD MMIO write
> > to BAR + offset.
> > 
> > For MSI/MSI-X-based doorbells, the message address is required to be
> > DWORD-aligned, so the computed offset should not straddle a BAR boundary
> > in normal operation.
> > 
> > However, with support for doorbells based on mechanisms other than
> > MSI/MSI-X (via pci_epf_alloc_doorbell()), the returned message address
> > may not necessarily be DWORD-aligned. In such a case, offset plus the
> > 32-bit write width could cross the end of the BAR aperture. The offset
> > returned by pci_epf_align_inbound_addr() is guaranteed to be within the
> > BAR size, but this alone does not ensure that a 32-bit write starting at
> > that offset stays within the BAR.
> > 
> > Add a bounds check to ensure that the 32-bit doorbell write always stays
> > within the BAR aperture. While this should not trigger for
> > spec-compliant MSI/MSI-X addresses, it provides a defensive guard
> > against unexpected offsets from future doorbell implementations.
> 
> I think everything you write is true,
> and I know that I suggested this...
> 
> But for the MMIO address, will it ever not be 32-bit aligned?

Yes I see your point.

> 
> Even in the eDMA case, the eDMA registers are 32-bit aligned.
> 
> Did I perhaps have a brain fart and overthink this?
> 
> 
> I guess theoretically, some future pci_epf_alloc_doorbell() implementation
> could return something that is not 32-bit aligned...

Yes, that's my impression as well. This looks more like a theoritical safeguard
in case pci_epf_align_inbound_addr() or nearby code paths gets accidentally
broken by future changes..

> 
> But if we really want to add a safety check for that... perhaps a 32-bit
> alignment check would be better suited to have in pci_epf_alloc_doorbell() ?
> 
> 
> Perhaps this check is better added in pci_epf_alloc_doorbell() or
> pci_epf_alloc_doorbell_embedded(), in the series that adds support for
> embedded doorbells ?

Hm, ok let's drop this patch at least from this series for now. I'll reconsider
it as part of the feature series instead.

Thanks,
Koichiro

> 
> 
> Kind regards,
> Niklas