On Wed, Mar 06, 2024 at 03:36:51PM +0100, Alexander Bluhm wrote:
> Hi,
> 
> When ix(4) rx links an mbuf into a chain, it is also referenced by
> nxbuf->buf and may be freed later.  As m_freem(rxbuf->fmp) frees
> this mbuf as part of the chain it is a double free.
> 
> As claudio@ mentioned that linking an mbuf with M_PKTHDR as chain
> member is a bug, we can us this flag as indication whether we need
> an individual free of the nxbuf->buf.
> 
> Unfortunatelly the error case with these free calls is not triggered,
> so I cannot test it.
> 

The below is my version of the same fix. This uses the fact that if fmp is
set then the mbuf in buf is part of that chain. No need for the M_PKTHDR
check. I clear the M_PKTHDR at a slightly different spot but that should
also not matter that much.

As bluhm@ said this code path is not hit so the problem is somewhere else.
-- 
:wq Claudio

Index: if_ix.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_ix.c,v
diff -u -p -r1.209 if_ix.c
--- if_ix.c	15 Feb 2024 10:56:53 -0000	1.209
+++ if_ix.c	6 Mar 2024 10:31:25 -0000
@@ -3172,12 +3188,12 @@ ixgbe_rxeof(struct rx_ring *rxr)
 		rsccnt >>= IXGBE_RXDADV_RSCCNT_SHIFT;
 
 		if (staterr & IXGBE_RXDADV_ERR_FRAME_ERR_MASK) {
 			if (rxbuf->fmp) {
 				m_freem(rxbuf->fmp);
-				rxbuf->fmp = NULL;
+			} else {
+				m_freem(mp);
 			}
-
-			m_freem(mp);
+			rxbuf->fmp = NULL;
 			rxbuf->buf = NULL;
 			goto next_desc;
 		}
@@ -3224,6 +3241,8 @@ ixgbe_rxeof(struct rx_ring *rxr)
 			sendmp = mp;
 			sendmp->m_pkthdr.len = 0;
 			sendmp->m_pkthdr.ph_mss = 0;
+		} else {
+			mp->m_flags &= ~M_PKTHDR;
 		}
 		sendmp->m_pkthdr.len += mp->m_len;
 		/*