From nobody Mon Nov 06 23:32:57 2023 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 4SPSMG0httz50fWL; Mon, 6 Nov 2023 23:32:58 +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 "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4SPSMG0DDnz4PCF; Mon, 6 Nov 2023 23:32:58 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1699313578; 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=RO4LAufepDuKdLS9Kvv7HT3Exb2gy+USFrbAG2Mghi8=; b=S0+47Axnolopp4VQMWyObigKQnbhska/xoeSwJKYDRGANWekKVyFNF6QJIMqS1AItlr2Xh GGNimPx7+5e3sWR6vFd+qNAOHxPrRQnH+VqgDKe9Fyqqe/w8zZ0xFBsUTQ/UWhjmeSybo3 okf+NM+OASG9coz8H71s7rjLD4mmygZU4THqtJCM8cXfGbuGMj35gCqPxzE2+fIyexrsvH OScpdGg8O6sUfgqD5buxl21B2ak4Ra9CaTIXstKV2wW0QNTTDHEDFms3lazydRYaJwFU4u 6X/rr+H6Uw7RGLV7/bqQF8N4963Cg/Yle+jPtB+O1XbBd9Ithshfh8iGBUYwIg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1699313578; 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=RO4LAufepDuKdLS9Kvv7HT3Exb2gy+USFrbAG2Mghi8=; b=P5Y4HnVbuPAXKG/anCgDhuT/SkuRBIgyK7HSoLFG9iRUkVs70uKqqu6hUhec+0Uc05tbAX x7ozHeNUGlz18Lg3F3fr1O9v6B68aizpLz366BSjuDQMNZLyGIEo+hM9GY0k3DCS/4XgVy cgXHZphmMtUdExcTjxuO0PKPOguXhIWaLB+yq87hblrDhrzflPRVPkbf9PZg+1jZVjyCEf p+Zx5iSftAXmRaxa4Ek+0pE0i+1mBXHIUpnZoJE7soVFcQvpKJsC0Omzm2P2f24wqC8o2f RrDp2YzG90kQN0v7+P8dPgih98uX6H1PT07eFYZLCR0TxWHEJcYzqDb7HmTLqQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1699313578; a=rsa-sha256; cv=none; b=g6hLVsjDm0H4ZQJEI+aehJ1pGLjDzYR9LuRAAZVb8RqRt/p+vA6MqWP44HMOMVp8nqt81N gat50xvWjZsMdKCH9/y5/sNicarG97pun8QEeei/WCRE/LkQaQSUXB6MKFQv5DRn8VUPVy nMFx1dcSGWQmMZpYhKCR8HTrv70Vr2mrK4/i9uI6K1xURyRrQJIneNwvVzBLYCg7uoEt1H PKT4mOmVfS/i6022+9R8vBAd9/pDkMZPClebaYrUjNarDsfx1t2Horv7Z0R+BgVfzb0Prj EukQRD88kqRNaxte5xy9ms7jLAXT5zJ57RfnJAN1dUUG5wMkce+Z3lZo1UL0Cw== 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 4SPSMF6PjRzCRn; Mon, 6 Nov 2023 23:32:57 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 3A6NWvO9044330; Mon, 6 Nov 2023 23:32:57 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 3A6NWvCX044327; Mon, 6 Nov 2023 23:32:57 GMT (envelope-from git) Date: Mon, 6 Nov 2023 23:32:57 GMT Message-Id: <202311062332.3A6NWvCX044327@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mark Johnston Subject: git: 0d4c814512b4 - releng/14.0 - pfsync: Avoid transmitting uninitialized bytes in pfsync_sendout() 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: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/releng/14.0 X-Git-Reftype: branch X-Git-Commit: 0d4c814512b4e8a3fe65ad0c9fca0c18def3621e Auto-Submitted: auto-generated The branch releng/14.0 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=0d4c814512b4e8a3fe65ad0c9fca0c18def3621e commit 0d4c814512b4e8a3fe65ad0c9fca0c18def3621e Author: Mark Johnston AuthorDate: 2023-11-04 14:28:24 +0000 Commit: Mark Johnston CommitDate: 2023-11-06 23:32:38 +0000 pfsync: Avoid transmitting uninitialized bytes in pfsync_sendout() When IPv6 support was added to pfsync, PFSYNC_MINPKT increased such that we always allocate enough space for either IPv4 or IPv6 headers. IPv6 headers are 20 bytes larger than IPv4 headers. When pfsync_sendout() does its thing, it ends up allocating enough space for either; thus when transmitting an IPv4 packet, the last 20 bytes of the buffer are left uninitialized. Fix the problem by stashing the length in a local variable and adjusting it depending on the address family in use. While here, just zero the entire buffer in one go rather than being careful to initialize each subheader. This seems simpler and less error prone. Approved by: re (gjb) Approved by: so Reported by: KMSAN Reviewed by: kp Fixes: 6fc7fc2dbb2b ("pfsync: transport over IPv6") MFC after: 3 days Differential Revision: https://reviews.freebsd.org/D42461 (cherry picked from commit bd80263606d73c0391d3fa8a156fcca89a821810) (cherry picked from commit 09cca16bfc26f669a5aea2a47e9891e34c3bebeb) --- sys/netpfil/pf/if_pfsync.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/sys/netpfil/pf/if_pfsync.c b/sys/netpfil/pf/if_pfsync.c index e29c00fcb879..75c361b394e0 100644 --- a/sys/netpfil/pf/if_pfsync.c +++ b/sys/netpfil/pf/if_pfsync.c @@ -1777,6 +1777,7 @@ pfsync_sendout(int schedswi, int c) struct pf_kstate *st, *st_next; struct pfsync_upd_req_item *ur; struct pfsync_bucket *b = &sc->sc_buckets[c]; + size_t len; int aflen, offset, count = 0; enum pfsync_q_id q; @@ -1797,7 +1798,9 @@ pfsync_sendout(int schedswi, int c) return; } m->m_data += max_linkhdr; - m->m_len = m->m_pkthdr.len = b->b_len; + bzero(m->m_data, b->b_len); + + len = b->b_len; /* build the ip header */ switch (sc->sc_sync_peer.ss_family) { @@ -1810,7 +1813,8 @@ pfsync_sendout(int schedswi, int c) bcopy(&sc->sc_template.ipv4, ip, sizeof(*ip)); aflen = offset = sizeof(*ip); - ip->ip_len = htons(m->m_pkthdr.len); + len -= sizeof(union inet_template) - sizeof(struct ip); + ip->ip_len = htons(len); ip_fillid(ip); break; } @@ -1824,7 +1828,8 @@ pfsync_sendout(int schedswi, int c) bcopy(&sc->sc_template.ipv6, ip6, sizeof(*ip6)); aflen = offset = sizeof(*ip6); - ip6->ip6_plen = htons(m->m_pkthdr.len); + len -= sizeof(union inet_template) - sizeof(struct ip6_hdr); + ip6->ip6_plen = htons(len); break; } #endif @@ -1832,14 +1837,14 @@ pfsync_sendout(int schedswi, int c) m_freem(m); return; } + m->m_len = m->m_pkthdr.len = len; /* build the pfsync header */ ph = (struct pfsync_header *)(m->m_data + offset); - bzero(ph, sizeof(*ph)); offset += sizeof(*ph); ph->version = PFSYNC_VERSION; - ph->len = htons(b->b_len - aflen); + ph->len = htons(len - aflen); bcopy(V_pf_status.pf_chksum, ph->pfcksum, PF_MD5_DIGEST_LENGTH); /* walk the queues */ @@ -1867,7 +1872,6 @@ pfsync_sendout(int schedswi, int c) } TAILQ_INIT(&b->b_qs[q]); - bzero(subh, sizeof(*subh)); subh->action = pfsync_qs[q].action; subh->count = htons(count); V_pfsyncstats.pfsyncs_oacts[pfsync_qs[q].action] += count; @@ -1888,7 +1892,6 @@ pfsync_sendout(int schedswi, int c) count++; } - bzero(subh, sizeof(*subh)); subh->action = PFSYNC_ACT_UPD_REQ; subh->count = htons(count); V_pfsyncstats.pfsyncs_oacts[PFSYNC_ACT_UPD_REQ] += count; @@ -1905,7 +1908,6 @@ pfsync_sendout(int schedswi, int c) subh = (struct pfsync_subheader *)(m->m_data + offset); offset += sizeof(*subh); - bzero(subh, sizeof(*subh)); subh->action = PFSYNC_ACT_EOF; subh->count = htons(1); V_pfsyncstats.pfsyncs_oacts[PFSYNC_ACT_EOF]++; @@ -1913,10 +1915,10 @@ pfsync_sendout(int schedswi, int c) /* we're done, let's put it on the wire */ if (ifp->if_bpf) { m->m_data += aflen; - m->m_len = m->m_pkthdr.len = b->b_len - aflen; + m->m_len = m->m_pkthdr.len = len - aflen; BPF_MTAP(ifp, m); m->m_data -= aflen; - m->m_len = m->m_pkthdr.len = b->b_len; + m->m_len = m->m_pkthdr.len = len; } if (sc->sc_sync_if == NULL) {