From nobody Tue Aug 26 04:37:16 2025 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4c9vyh3RH7z66Cf6; Tue, 26 Aug 2025 04:37:16 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R12" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4c9vyh1m29z3H2l; Tue, 26 Aug 2025 04:37:16 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1756183036; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=np/SPcoqK9ZdNjAfzV4NbB2o7SAKdjWHO1nCgLvXlG0=; b=dAeQtBDsv7pDWw//nSvdMIgF8B0pnVhGGuWNQ4VHvRFcSyjNcZBRxWFD2rXAQcwkk9Hjys mbOYl5zkulOLDbvfUHHbTE+LQ+MiLH6EbILdi4EmSo2KxvYgHAU0lVMQK3WFKtO/X5oWtL Rm461A1JtvVQKzGDyvn4PkyM+k5rHMi3600E3n1bjtqJRh49LFrSOLzlbvX0oHjlYVc6qA QBRSGk8iU+ACbCPcv18jOYwBy64xCVpOq27HPx/UOkj9mWelN2bT3O0etozYrFi6XAGL+o uQ21eGkqD4a8MIu9FzZkQo0ctmcsQk84HafxV1gBYNBex0g8yMldklnktCaTAQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1756183036; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=np/SPcoqK9ZdNjAfzV4NbB2o7SAKdjWHO1nCgLvXlG0=; b=GIv8DRsD9DVm33i3KcORsZhVIoty6OgRtzQOYQMf8O9rJ9ey7wDINV3dOPY2SAePtmcPdO Wb4U/MldqeTiMJRXSC75E2zYiDgQfTbzMe0xwy7Am9OnQE+ouxBZIf51NdVXIxsZJ1zfFP KiNiLgaLsA5toPblkiIiwlh3zwxgx8rg6J8WI1A/cWeuTSAytatTmvSwXHIfaO4OjgsTpC 8ByhdzKEg0Wm9B1FDnuYafYjx/ybM4R5YkmA5ZM4YU9+aQrjdLLZVgcb4Y1dsJuROY7+LF JpBu6BNUttb/R51YohqlGc7aeFkYQSdrcQ8YOh7vcqMiKOuUnNFWlcNYLwv6+w== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1756183036; a=rsa-sha256; cv=none; b=I8z7kshdxorM0UpGlCylVfNZJez/Fc+ZJVMwGEfdPBNqOQoeYNhExmcl2II7l6KIO/Mk6P zvnBVHU4uEaJJUrTjbmGxZTfzoBBzHUHSy0Xfzl85r6vuGFsrcwh2lKYd0OkV5ewaqXBQu fv11RPFrAH7aPjK8sBdiLj291AF28f3Bn7Rc/uPXP5/GG1tZYwRBjujkiSjBsVAdm+fYLF wXGzhT7PPRkFWo4b/lT/eOGN2K8FMaZRVHghAjSRhBeUxTkKWyCt10Wi0guHwo9I4gtNeZ /MtrKmO2u06HxPH2YUiTjtXCvdLma7qS7C1h+mBTnQUnGj5YPQrHf8d9YKEu4A== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4c9vyh0sfvz580; Tue, 26 Aug 2025 04:37:16 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 57Q4bGPi043107; Tue, 26 Aug 2025 04:37:16 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 57Q4bG0s043104; Tue, 26 Aug 2025 04:37:16 GMT (envelope-from git) Date: Tue, 26 Aug 2025 04:37:16 GMT Message-Id: <202508260437.57Q4bG0s043104@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Maxim Sobolev Subject: git: f74c0dc583d6 - main - ng_nat: fix potential crash when attaching to L2 directly List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: sobomax X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: f74c0dc583d69400b9cea41e2f009d8c4757ce26 Auto-Submitted: auto-generated The branch main has been updated by sobomax: URL: https://cgit.FreeBSD.org/src/commit/?id=f74c0dc583d69400b9cea41e2f009d8c4757ce26 commit f74c0dc583d69400b9cea41e2f009d8c4757ce26 Author: Maxim Sobolev AuthorDate: 2025-08-25 22:08:12 +0000 Commit: Maxim Sobolev 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)