git: 73cceb046cd8 - main - iflib: Fix several memory handling issues around iflib_encap()

The branch main has been updated by gallatin:

URL: https://cgit.FreeBSD.org/src/commit/?id=73cceb046cd86187bc3e3bad07dd2422ee102552

commit 73cceb046cd86187bc3e3bad07dd2422ee102552
Author:     Andrew Gallatin <[email protected]>
AuthorDate: 2026-08-14 19:38:34 +0000
Commit:     Andrew Gallatin <[email protected]>
CommitDate: 2026-08-14 19:42:50 +0000

    iflib: Fix several memory handling issues around iflib_encap()
    
    - Fixed memory leaks around m_dup() not freeing the original chain on
      failure. If we return ENOMEM, we are expected to have freed the
      chain, else the mbuf would be leaked. Also updated iflib_ether_pad()
      to follow the same structure.
    
    - In iflib_parse_header()
        o Fixed a bug where the ip/ip6 and th pointers may point into a
          freed chain after m_pullup.  Those pointers must be reset to
          point into the new chain.
    
        o Eliminate ENXIO returns for non-TCP TSO sends (which would violate
          the mbuf ownership contract if they could happen). Since they
          cannot happen, I made them assertions instead.
    
    - in iflib_ether_pad(), return ENOMEM after freeing mbuf, so that
      mp_ring knows it is free. An ENOBUFS error will cause the mp_ring
      path to retain the mbuf and retry
    
    - in iflib_encap():
         o Fix a leak when bus_dmamap_load_mbuf_sg() returns ENOMEM
         o Fix a use-after-free in the mp_ring path when a driver using
           ktls frees an mbuf and returns ENOBUFS via iflib_encap()
    
    After this change the expection from iflib_encap is that:
    
    mp_ring: ENOBUFS can be returned only when we run out of descriptors
             (ENOBUFS causes mp_ring to retain the mbuf).
    
    simple_tx: iflib_encap() always consumes the mbuf, regardless of the
               return
    
    Note that iflib_debugnet_transmit(), like simple_tx, expects that
    iflib_encap() always consumes mbufs.  This will be true after mp_ring
    is removed, and its such a rare special case (overrunning the ring
    during panic dumps) that I don't think its worth fixing in the
    meantime.
    
    Sponsored by: Netflix
    Reviewed by: kbowling, sumit.saxena_broadcom.com
    Differential Revision: https://reviews.freebsd.org/D58843
    Fixes: 074ff8746388
---
 sys/net/iflib.c | 50 ++++++++++++++++++++++++++++----------------------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/sys/net/iflib.c b/sys/net/iflib.c
index abc56f159547..d2ac8d884a60 100644
--- a/sys/net/iflib.c
+++ b/sys/net/iflib.c
@@ -3262,13 +3262,12 @@ iflib_parse_header_partial(if_pkt_info_t pi, struct mbuf **mp, uint64_t *pullups
 	*pullups = 0;
 	m = *mp;
 	if (!M_WRITABLE(m)) {
-		if ((m = m_dup(m, M_NOWAIT)) == NULL) {
+		m = m_dup(m, M_NOWAIT);
+		m_freem(*mp);
+		DBG_COUNTER_INC(tx_frees);
+		*mp = m;
+		if (m == NULL)
 			return (ENOMEM);
-		} else {
-			m_freem(*mp);
-			DBG_COUNTER_INC(tx_frees);
-			*mp = m;
-		}
 	}
 
 	/* Fills out pi->ipi_etype */
@@ -3364,13 +3363,12 @@ iflib_parse_header(iflib_txq_t txq, if_pkt_info_t pi, struct mbuf **mp)
 	m = *mp;
 	if ((sctx->isc_flags & IFLIB_NEED_SCRATCH) &&
 	    M_WRITABLE(m) == 0) {
-		if ((m = m_dup(m, M_NOWAIT)) == NULL) {
+		m = m_dup(m, M_NOWAIT);
+		m_freem(*mp);
+		DBG_COUNTER_INC(tx_frees);
+		*mp = m;
+		if (m == NULL)
 			return (ENOMEM);
-		} else {
-			m_freem(*mp);
-			DBG_COUNTER_INC(tx_frees);
-			*mp = m;
-		}
 	}
 
 	/* Fills out pi->ipi_etype */
@@ -3405,6 +3403,9 @@ iflib_parse_header(iflib_txq_t txq, if_pkt_info_t pi, struct mbuf **mp)
 			txq->ift_pullups++;
 			if ((m = m_pullup(m, hlen)) == NULL)
 				return (ENOMEM);
+			/* reset pointers after pullup */
+			ip = (struct ip *)(m->m_data + pi->ipi_ehdrlen);
+			th = (struct tcphdr *)((char *)ip + (ip->ip_hl << 2));
 		}
 		pi->ipi_ip_hlen = ip->ip_hl << 2;
 		pi->ipi_ipproto = ip->ip_p;
@@ -3419,8 +3420,7 @@ iflib_parse_header(iflib_txq_t txq, if_pkt_info_t pi, struct mbuf **mp)
 				pi->ipi_tcp_seq = th->th_seq;
 			}
 			if (IS_TSO4(pi)) {
-				if (__predict_false(ip->ip_p != IPPROTO_TCP))
-					return (ENXIO);
+				MPASS(ip->ip_p == IPPROTO_TCP);
 				/*
 				 * TSO always requires hardware checksum offload.
 				 */
@@ -3451,6 +3451,8 @@ iflib_parse_header(iflib_txq_t txq, if_pkt_info_t pi, struct mbuf **mp)
 			txq->ift_pullups++;
 			if (__predict_false((m = m_pullup(m, pi->ipi_ehdrlen + sizeof(struct ip6_hdr))) == NULL))
 				return (ENOMEM);
+			/* reset pointers after pullup */
+			ip6 = (struct ip6_hdr *)(m->m_data + pi->ipi_ehdrlen);
 		}
 		th = (struct tcphdr *)((caddr_t)ip6 + pi->ipi_ip_hlen);
 
@@ -3466,14 +3468,16 @@ iflib_parse_header(iflib_txq_t txq, if_pkt_info_t pi, struct mbuf **mp)
 					txq->ift_pullups++;
 					if (__predict_false((m = m_pullup(m, pi->ipi_ehdrlen + sizeof(struct ip6_hdr) + sizeof(struct tcphdr))) == NULL))
 						return (ENOMEM);
+					/* reset pointers after pullup */
+					ip6 = (struct ip6_hdr *)(m->m_data + pi->ipi_ehdrlen);
+					th = (struct tcphdr *)((caddr_t)ip6 + pi->ipi_ip_hlen);
 				}
 				pi->ipi_tcp_hflags = tcp_get_flags(th);
 				pi->ipi_tcp_hlen = th->th_off << 2;
 				pi->ipi_tcp_seq = th->th_seq;
 			}
 			if (IS_TSO6(pi)) {
-				if (__predict_false(ip6->ip6_nxt != IPPROTO_TCP))
-					return (ENXIO);
+				MPASS(ip6->ip6_nxt == IPPROTO_TCP);
 				/*
 				 * TSO always requires hardware checksum offload.
 				 */
@@ -3539,15 +3543,14 @@ iflib_ether_pad(device_t dev, struct mbuf **m_head, uint16_t min_frame_size)
 
 	if (!M_WRITABLE(*m_head)) {
 		new_head = m_dup(*m_head, M_NOWAIT);
+		m_freem(*m_head);
+		*m_head = new_head;
 		if (new_head == NULL) {
-			m_freem(*m_head);
 			device_printf(dev, "cannot pad short frame, m_dup() failed");
 			DBG_COUNTER_INC(encap_pad_mbuf_fail);
 			DBG_COUNTER_INC(tx_frees);
 			return (ENOMEM);
 		}
-		m_freem(*m_head);
-		*m_head = new_head;
 	}
 
 	for (n = min_frame_size - (*m_head)->m_pkthdr.len;
@@ -3557,10 +3560,11 @@ iflib_ether_pad(device_t dev, struct mbuf **m_head, uint16_t min_frame_size)
 
 	if (n > 0) {
 		m_freem(*m_head);
+		*m_head = NULL;
 		device_printf(dev, "cannot pad short frame\n");
 		DBG_COUNTER_INC(encap_pad_mbuf_fail);
 		DBG_COUNTER_INC(tx_frees);
-		return (ENOBUFS);
+		return (ENOMEM);
 	}
 
 	return (0);
@@ -3665,8 +3669,7 @@ defrag:
 			goto retry;
 			break;
 		case ENOMEM:
-			txq->ift_no_tx_dma_setup++;
-			break;
+			/* FALLTHROUGH */
 		default:
 			txq->ift_no_tx_dma_setup++;
 			m_freem(*m_headp);
@@ -3776,6 +3779,9 @@ defrag:
 			}
 			goto defrag_failed;
 		}
+		/* mp_ring assumes ENOBUFS means we didn't consume the mbuf */
+		if (err == ENOBUFS && !ctx->ifc_sysctl_simple_tx)
+			err = ENOMEM;
 		goto out_with_error;
 	}
 	/*