git: 73cceb046cd8 - main - iflib: Fix several memory handling issues around iflib_encap()
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 14 Aug 2026 19:44:54 UTC
The branch main has been updated by gallatin:
URL: https://cgit.FreeBSD.org/src/commit/?id=73cceb046cd86187bc3e3bad07dd2422ee102552
commit 73cceb046cd86187bc3e3bad07dd2422ee102552
Author: Andrew Gallatin <gallatin@FreeBSD.org>
AuthorDate: 2026-08-14 19:38:34 +0000
Commit: Andrew Gallatin <gallatin@FreeBSD.org>
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;
}
/*