From: Kirill A. Korinsky <kirill@korins.ky>
Subject: Re: sys/cnmac: read next RX buffer pointer before release
To: Visa Hankala <visa@hankala.org>
Cc: tech@openbsd.org
Date: Fri, 19 Jun 2026 17:19:49 +0200

On Fri, 19 Jun 2026 15:32:36 +0200,
Visa Hankala <visa@hankala.org> wrote:
> 
> On Thu, Jun 18, 2026 at 11:02:36AM +0200, Kirill A. Korinsky wrote:
> > tech@, Visa,
> > 
> > My ER-4 which I use as home's gw crashed some time ago with stacktrace:
> > 
> > ddb{0}> trace
> > cnmac_recv_mbuf+0x13c (77e060040f88a,1,980000000fd979f8,980000000fd979f4)  ra 0
> > xffffffff810f13a0 sp 0x980000000fd97990, sz 32
> > cnmac_recv+0x88 (77e060040f88a,1,980000000fd979f8,b14f8e2ae1694c1c)  ra 0xfffff
> > fff810eddb0 sp 0x980000000fd979b0, sz 144
> > cnmac_intr+0x100 (77e060040f88a,593aedd32134a2c6,980000000fd979f8,4)  ra 0xffff
> > ffff8124a06c sp 0x980000000fd97a40, sz 112
> > octciu_intr_bank+0x274 (77e060040f88a,593aedd32134a2c6,980000000fd979f8,47bf232
> > 1b40c9360)  ra 0xffffffff8124976c sp 0x980000000fd97ab0, sz 160
> > octciu_intr0+0x94 (77e060040f88a,593aedd32134a2c6,57aacbc77ca5909c,47bf2321b40c
> > 9360)  ra 0xffffffff811a0f68 sp 0x980000000fd97b50, sz 64
> > interrupt+0x170 (77e060040f88a,1da67593047e9497,57aacbc77ca5909c,47bf2321b40c93
> > 60)  ra 0xffffffff813461f4 sp 0x980000000fd97b90, sz 64
> > k_intr+0xb4 (980000000fd97bf8,1da67593047e9497,57aacbc77ca5909c,ffffffff811e948
> > c)  ra 0x0 sp 0x980000000fd97bd0, sz 0
> > (KERNEL INTERRUPT)
> > cpu_idle_cycle_wait+0x4 (980000000fd97bf8,1da67593047e9497,57aacbc77ca5909c,fff
> > fffff811e948c)  ra 0xffffffff810a7ccc sp 0x980000000fd97d50, sz 0
> > sched_idle+0x314 (980000000fd97bf8,1da67593047e9497,57aacbc77ca5909c,ffffffff81
> > 1e948c)  ra 0xffffffff811e95bc sp 0x980000000fd97d50, sz 96
> > proc_trampoline+0x1c (980000000fd97bf8,1da67593047e9497,57aacbc77ca5909c,ffffff
> > ff811e948c)  ra 0x0 sp 0x980000000fd97db0, sz 0
> > User-level: pid 7056
> > ddb{0}> 
> > 
> > I not completley sure that had happened, but here some thoughts that can fix
> > that, as blind shot, or it can be absolutley wrong.
> > 
> > The idea that in both RX drop and receive paths, copy the next WQE word
> > before the current packet buffer is returned to FPA or its hidden mbuf
> > pointer is cleared.
> > 
> > Returning the buffer first allows hardware to reuse it before the driver
> > has consumed the chained buffer pointer; that can corrupt word3 and
> > later fault in cnmac_recv_mbuf() while recovering the mbuf from pktbuf.
> 
> cnmac_buf_free_work() has a use-after-free. It should happen only when
> a jumbo frame gets dropped. (A single hardware packet buffer has
> enough space for a non-jumbo-sized frame.)
> 
> The crash could in principle happen already in cnmac_buf_free_work()
> if the hardware's bookkeeping operations were very quick to clobber
> the next word3.
>

This is just home gw, which runs wg and NAT. Here no jumbo frames.

It is quite stable, and it just hungs, and because I was near I had that trace.

> > @@ -1244,6 +1244,9 @@ cnmac_recv_mbuf(struct cnmac_softc *sc, 
> >  		addr = word3 & PIP_WQE_WORD3_ADDR;
> >  		back = (word3 & PIP_WQE_WORD3_BACK) >> PIP_WQE_WORD3_BACK_SHIFT;
> >  		pktbuf = (addr & ~(CACHELINESIZE - 1)) - back * CACHELINESIZE;
> > +		if (i + 1 < nbufs)
> > +			memcpy(&word3, (void *)PHYS_TO_XKPHYS(addr -
> > +			    sizeof(word3), CCA_CACHED), sizeof(word3));
> >  		pm = (struct mbuf **)PHYS_TO_XKPHYS(pktbuf, CCA_CACHED) - 1;
> >  		m = *pm;
> >  		*pm = NULL;
> > @@ -1276,10 +1279,6 @@ cnmac_recv_mbuf(struct cnmac_softc *sc, 
> >  			mprev->m_next = m;
> >  		}
> >  		mprev = m;
> > -
> > -		if (i + 1 < nbufs)
> > -			memcpy(&word3, (void *)PHYS_TO_XKPHYS(addr -
> > -			    sizeof(word3), CCA_CACHED), sizeof(word3));
> >  	}
> >  
> >  	m0->m_pkthdr.len = total;
> 
> I think it is clearer to update word3 only after cnmac_recv_mbuf() has
> finished with the current segment. Please keep the function as is.
> 
> The locations of word3 and pm should never overlap. word3 is inside
> the hardware buffer region, whereas pm has to be outside of it.
> Otherwise the hardware could clobber pm.
> 

Because it was a blind shot, I'll ignore this one and see.

Thanks for review!

-- 
wbr, Kirill