The branch main has been updated by sobomax:
URL:
https://cgit.FreeBSD.org/src/commit/?id=f74c0dc583d69400b9cea41e2f009d8c4757ce26
commit f74c0dc583d69400b9cea41e2f009d8c4757ce26
Author: Maxim Sobolev <sobo...@freebsd.org>
AuthorDate: 2025-08-25 22:08:12 +0000
Commit: Maxim Sobolev <sobo...@freebsd.org>
CommitDate: 2025-08-26 04:34:45 +0000
ng_nat: fix potential crash when attaching to L2 directly
Fix potential crash in the ng_nat module when attaching directly
to the layer 2 (ethernet) while calculating TCP checksum.
The issue is due to in_delayed_cksum() expecting to access IP
header at the offset 0 from the mbuf start, while if we are
attached to the L2 directly, the IP header at going to be at the
certain offset.
Reviewed by: markj, tuexen
Approved by: tuexen
Sponsored by: Sippy Software, Inc.
Differential Revision: https://reviews.freebsd.org/D49677
MFC After: 2 weeks
---
sys/netgraph/ng_nat.c | 95 ++++++++++++++++++++++++++++++---------------------
1 file changed, 57 insertions(+), 38 deletions(-)
diff --git a/sys/netgraph/ng_nat.c b/sys/netgraph/ng_nat.c
index defbe817becd..8b82d777caeb 100644
--- a/sys/netgraph/ng_nat.c
+++ b/sys/netgraph/ng_nat.c
@@ -818,7 +818,8 @@ ng_nat_rcvdata(hook_p hook, item_p item )
if (ip->ip_v != IPVERSION)
goto send; /* other IP version, let it pass */
- if (m->m_pkthdr.len < ipofs + ntohs(ip->ip_len))
+ uint16_t ip_len = ntohs(ip->ip_len);
+ if (m->m_pkthdr.len < (ipofs + ip_len))
goto send; /* packet too short (i.e. fragmented or
broken) */
/*
@@ -852,50 +853,68 @@ ng_nat_rcvdata(hook_p hook, item_p item )
if (rval == PKT_ALIAS_RESPOND)
m->m_flags |= M_SKIP_FIREWALL;
- m->m_pkthdr.len = m->m_len = ntohs(ip->ip_len) + ipofs;
- if ((ip->ip_off & htons(IP_OFFMASK)) == 0 &&
- ip->ip_p == IPPROTO_TCP) {
- struct tcphdr *th = (struct tcphdr *)((caddr_t)ip +
- (ip->ip_hl << 2));
+ /* Re-read just in case it has been updated */
+ ip_len = ntohs(ip->ip_len);
+ int new_m_len = ip_len + ipofs;
+ if (new_m_len > (m->m_len + M_TRAILINGSPACE(m))) {
/*
- * Here is our terrible HACK.
- *
- * Sometimes LibAlias edits contents of TCP packet.
- * In this case it needs to recompute full TCP
- * checksum. However, the problem is that LibAlias
- * doesn't have any idea about checksum offloading
- * in kernel. To workaround this, we do not do
- * checksumming in LibAlias, but only mark the
- * packets with TH_RES1 in the th_x2 field. If we
- * receive a marked packet, we calculate correct
- * checksum for it aware of offloading.
- *
- * Why do I do such a terrible hack instead of
- * recalculating checksum for each packet?
- * Because the previous checksum was not checked!
- * Recalculating checksums for EVERY packet will
- * hide ALL transmission errors. Yes, marked packets
- * still suffer from this problem. But, sigh, natd(8)
- * has this problem, too.
+ * This is just a safety railguard to make sure LibAlias has not
+ * screwed the IP packet up somehow, should probably be
KASSERT()
+ * at some point. Calling in_delayed_cksum() will parse IP
packet
+ * again and reliably panic if there is less data than the IP
+ * header declares, there might be some other places too.
*/
+ log(LOG_ERR, "ng_nat_rcvdata: outgoing packet corrupted, "
+ "not enough data: expected %d, available (%d - %d)\n",
+ ip_len, m->m_len + (int)M_TRAILINGSPACE(m), ipofs);
+ NG_FREE_ITEM(item);
+ return (ENXIO);
+ }
+
+ m->m_pkthdr.len = m->m_len = new_m_len;
- if (tcp_get_flags(th) & TH_RES1) {
- uint16_t ip_len = ntohs(ip->ip_len);
+ if ((ip->ip_off & htons(IP_OFFMASK)) != 0 || ip->ip_p != IPPROTO_TCP)
+ goto send;
- tcp_set_flags(th, tcp_get_flags(th) & ~TH_RES1);
- th->th_sum = in_pseudo(ip->ip_src.s_addr,
- ip->ip_dst.s_addr, htons(IPPROTO_TCP +
- ip_len - (ip->ip_hl << 2)));
+ uint16_t pl_offset = ip->ip_hl << 2;
+ struct tcphdr *th = (struct tcphdr *)((caddr_t)ip + pl_offset);
- if ((m->m_pkthdr.csum_flags & CSUM_TCP) == 0) {
- m->m_pkthdr.csum_data = offsetof(struct tcphdr,
- th_sum);
- in_delayed_cksum(m);
- }
- }
- }
+ /*
+ * Here is our terrible HACK.
+ *
+ * Sometimes LibAlias edits contents of TCP packet.
+ * In this case it needs to recompute full TCP
+ * checksum. However, the problem is that LibAlias
+ * doesn't have any idea about checksum offloading
+ * in kernel. To workaround this, we do not do
+ * checksumming in LibAlias, but only mark the
+ * packets with TH_RES1 in the th_x2 field. If we
+ * receive a marked packet, we calculate correct
+ * checksum for it aware of offloading.
+ *
+ * Why do I do such a terrible hack instead of
+ * recalculating checksum for each packet?
+ * Because the previous checksum was not checked!
+ * Recalculating checksums for EVERY packet will
+ * hide ALL transmission errors. Yes, marked packets
+ * still suffer from this problem. But, sigh, natd(8)
+ * has this problem, too.
+ */
+
+ if (!(tcp_get_flags(th) & TH_RES1))
+ goto send;
+
+ tcp_set_flags(th, tcp_get_flags(th) & ~TH_RES1);
+ th->th_sum = in_pseudo(ip->ip_src.s_addr, ip->ip_dst.s_addr,
+ htons(IPPROTO_TCP + ip_len - pl_offset));
+
+ if ((m->m_pkthdr.csum_flags & CSUM_TCP) != 0)
+ goto send;
+
+ m->m_pkthdr.csum_data = offsetof(struct tcphdr, th_sum);
+ in_delayed_cksum_o(m, ipofs);
send:
if (hook == priv->in)
