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; } /*
