mana_gd_ring_doorbell() accesses doorbell offsets up to 0xFF8 + 8 = 4KB
within a doorbell page. When db_page_size is zero, the validation check
in mana_gd_register_device() reduces to:
  db_page_off + 0 > bar0_size
which passes, even though mana_gd_ring_doorbell() will access
[db_page_off, db_page_off + 4KB) and may go beyond BAR0.

Use max(SZ_4K, db_page_size) in the range check so that a zero or
unexpectedly small db_page_size still results in a rejection when the
doorbell page would fall outside BAR0.

Fixes: 89fe91c65992 ("net: mana: hardening: Validate doorbell ID from GDMA_REGISTER_DEVICE response")
Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
---
 drivers/net/ethernet/microsoft/mana/gdma_main.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 2ba1fa3336f9..49ea3dcbf74a 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -4,6 +4,7 @@
 #include <linux/debugfs.h>
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <linux/sizes.h>
 #include <linux/utsname.h>
 #include <linux/version.h>
 #include <linux/msi.h>
@@ -1255,6 +1256,7 @@ int mana_gd_register_device(struct gdma_dev *gd)
 	struct gdma_context *gc = gd->gdma_context;
 	struct gdma_register_device_resp resp = {};
 	struct gdma_general_req req = {};
+	u64 db_page_sz;
 	int err;
 
 	gd->pdid = INVALID_PDID;
@@ -1278,8 +1280,14 @@ int mana_gd_register_device(struct gdma_dev *gd)
 	 *   addr = db_page_base + db_page_size * db_id
 	 *        = (bar0_va + db_page_off) + (db_page_size * db_id)
 	 * So we need: db_page_off + db_page_size * (db_id + 1) <= bar0_size
+	 *
+	 * mana_gd_ring_doorbell() always accesses [offset, offset + 4KB),
+	 * so use at least SZ_4K to catch a zero or small db_page_size.
 	 */
-	if (gc->db_page_off + gc->db_page_size * ((u64)resp.db_id + 1) > gc->bar0_size) {
+	db_page_sz = max_t(u64, SZ_4K, gc->db_page_size);
+
+	if (gc->db_page_off + db_page_sz * ((u64)resp.db_id + 1) >
+	    gc->bar0_size) {
 		dev_err(gc->dev, "Doorbell ID %u out of range\n", resp.db_id);
 		return -EPROTO;
 	}
-- 
2.34.1

On Fri, Mar 20, 2026 at 05:21:01AM -0700, Erni Sri Satya Vennela wrote:
> mana_gd_ring_doorbell() accesses doorbell offsets up to 0xFF8 + 8 = 4KB
> within a doorbell page. When db_page_size is zero, the validation check
> in mana_gd_register_device() reduces to:
>   db_page_off + 0 > bar0_size
> which passes, even though mana_gd_ring_doorbell() will access
> [db_page_off, db_page_off + 4KB) and may go beyond BAR0.
> 
> Use max(SZ_4K, db_page_size) in the range check so that a zero or
> unexpectedly small db_page_size still results in a rejection when the
> doorbell page would fall outside BAR0.

Thanks Erni,

I understand the maths here. And to that extent this change makes sense to me.
But I am curious to know how a db_page_size of zero works. I was expecting
some space is required there.

> 
> Fixes: 89fe91c65992 ("net: mana: hardening: Validate doorbell ID from GDMA_REGISTER_DEVICE response")
> Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>

...

On 3/21/26 11:04 AM, Simon Horman wrote:
> On Fri, Mar 20, 2026 at 05:21:01AM -0700, Erni Sri Satya Vennela wrote:
>> mana_gd_ring_doorbell() accesses doorbell offsets up to 0xFF8 + 8 = 4KB
>> within a doorbell page. When db_page_size is zero, the validation check
>> in mana_gd_register_device() reduces to:
>>   db_page_off + 0 > bar0_size
>> which passes, even though mana_gd_ring_doorbell() will access
>> [db_page_off, db_page_off + 4KB) and may go beyond BAR0.
>>
>> Use max(SZ_4K, db_page_size) in the range check so that a zero or
>> unexpectedly small db_page_size still results in a rejection when the
>> doorbell page would fall outside BAR0.
> 
> Thanks Erni,
> 
> I understand the maths here. And to that extent this change makes sense to me.
> But I am curious to know how a db_page_size of zero works. I was expecting
> some space is required there.

To rephrase Simon's question, this feels like papering over a
memory/state corruption. I think at best it deserves a cleaner explanation.

/P

On Tue, Mar 24, 2026 at 12:03:37PM +0100, Paolo Abeni wrote:
> 
> 
> On 3/21/26 11:04 AM, Simon Horman wrote:
> > On Fri, Mar 20, 2026 at 05:21:01AM -0700, Erni Sri Satya Vennela wrote:
> >> mana_gd_ring_doorbell() accesses doorbell offsets up to 0xFF8 + 8 = 4KB
> >> within a doorbell page. When db_page_size is zero, the validation check
> >> in mana_gd_register_device() reduces to:
> >>   db_page_off + 0 > bar0_size
> >> which passes, even though mana_gd_ring_doorbell() will access
> >> [db_page_off, db_page_off + 4KB) and may go beyond BAR0.
> >>
> >> Use max(SZ_4K, db_page_size) in the range check so that a zero or
> >> unexpectedly small db_page_size still results in a rejection when the
> >> doorbell page would fall outside BAR0.
> > 
> > Thanks Erni,
> > 
> > I understand the maths here. And to that extent this change makes sense to me.
> > But I am curious to know how a db_page_size of zero works. I was expecting
> > some space is required there.
> 
> To rephrase Simon's question, this feels like papering over a
> memory/state corruption. I think at best it deserves a cleaner explanation.
> 
> /P
Thanks for pointing it out Simon and Paolo.
Now I understand the real issue, when db_page_sz is zero my patch rejects
it, but doesn't explicitly point it out. Such case means something is
wrong in hardware, which is silently escaped in this patch.

I will create another patch where I will reject db_page_size < SZ_4K at
the source.