Re: [PATCH net] net: mana: hardening: Validate SHM offset from BAR0 register to prevent crash due to alignment fault

[Dipayaan Roy]

On Thu, Apr 23, 2026 at 09:44:04PM +0200, Andrew Lunn wrote:
> On Thu, Apr 23, 2026 at 12:14:16PM -0700, Dipayaan Roy wrote:
> > On Thu, Apr 23, 2026 at 06:37:04PM +0200, Andrew Lunn wrote:
> > > > The root cause is in mana_gd_init_vf_regs(), which computes:
> > > > 
> > > >   gc->shm_base = gc->bar0_va + mana_gd_r64(gc, GDMA_REG_SHM_OFFSET);
> > > > 
> > > > without validating the offset read from hardware. If the register
> > > > returns a garbage value that is neither within bar 0 bounds nor aligned
> > > > to the 4-byte granularity, thus causing the alignment fault.
> > > 
> > > Is GDMA_REG_SHM_OFFSET special?
> > Hi Andrew,
> > GDMA_REG_SHM_OFFSET is not special. It was simply the only register
> > read that had no validation at all. The other two registers
> > (GDMA_REG_DB_PAGE_SIZE, GDMA_REG_DB_PAGE_OFFSET) already have checks
> > in place.
> 
> I must be missing something:
> 
> grep page_size *
> 
> gdma_main.c:	gc->db_page_size = mana_gd_r32(gc, GDMA_PF_REG_DB_PAGE_SIZE) & 0xFFFF;
> gdma_main.c:	gc->db_page_size = mana_gd_r32(gc, GDMA_REG_DB_PAGE_SIZE) & 0xFFFF;
> gdma_main.c:	void __iomem *addr = gc->db_page_base + gc->db_page_size * db_index;
> 

Hi Andrew,
There are 2 upstream commits regarding these, I think you missed
them please check once:

commit fb4b4a05aeeb8b0f253c5ddce21f4635dadc9550
Author: Erni Sri Satya Vennela <ernis@xxxxxxxxxxxxxxxxxxx>
Date:   Wed Mar 25 11:04:17 2026 -0700
 
    net: mana: Use at least SZ_4K in doorbell ID range check

commit 89fe91c65992a37863241e35aec151210efc53ce
Author: Erni Sri Satya Vennela <ernis@xxxxxxxxxxxxxxxxxxx>
Date:   Fri Mar 6 13:12:06 2026 -0800
 
    net: mana: hardening: Validate doorbell ID from GDMA_REGISTER_DEVICE response

> So if GDMA_REG_DB_PAGE_SIZE returns garbage, it is at least masked,
> but it is still a random number.
> 
> mana_gd_ring_doorbell() takes this random number, multiples by
> db_index, adds, gc->db_page_base and then does:
> 
> writeq(e.as_uint64, addr);
> 
> So you write to a random address. 
> 
> I don't see any sanity checks here. Cannot you check that db_page_size
> is at least one of the expected page sizes?
As mentioned above checks are already present in this commit: 89fe91c65992a37863241e35aec151210efc53ce
> 
>    Andrew

Regards
Dipayaan Roy