From nobody Sat Nov 4 14:29:06 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 4SN0Ng12mtz50cnd; Sat, 4 Nov 2023 14:29:07 +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 4SN0Ng0SpDz4TnC; Sat, 4 Nov 2023 14:29:07 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1699108147; 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=o4TxgoxKZlah61uq4nS8HxxLKPXorserNUAVOcBUwrg=; b=Cgs34PgNzj2YVtvavr4eqM0DugCGh5utEoUUceHnX6WH6iGuxXK34+U9hHHX1dDid5pLMC rSGiedWcRXf0t7WePSg3S1y4jYBYD9F149IptcBcAt/UDtZdVxM7/tOSWNHkaplyl+fBVA 98MPPWvP6hpJsJrzfm/hPQe7qObEQPcENmiDUaKVLgi/fo+ae973Ksgy27nOjNu+cBOpnj rKa47eVWn2NWBBhPoXsKsFW8HqUkHZkbHlJ7aFdPnov+sNj+u4JI8tyhTORlfAk1VsNTXj c4wPs9MCP5iTjUdYSXR0/UUQ4lzBJM7W4vDAn03kBzcDs5W3TTHFlFMjcNEvDA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1699108147; 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=o4TxgoxKZlah61uq4nS8HxxLKPXorserNUAVOcBUwrg=; b=SqIvxAsF8eP3Icv97LzFsDXxfW+CpyYVmLZsgHWRYPqxd1XDkWj/gS5gNPnl7PWARU/b4y QeVHvLWBN6Pe++FHULltjiNX9stkEziwrLJY8fxUv8X6ByV1mvTyIGUca9kSwPOHyT2FMw ctw1YFv7Q2GLA+qcgWYhG4rsLigeVhyQgT5OCWL3eqcwg/8AFXG58Q4SAsyeGt/HiG8vcN +5W03TphNnSauIo/sIWfDMNW7pKD3KE3IAwDuf6TBshr3sCpOPihEMtk15hbk04uC/v3J5 7yqroIYXYARcnnJKiyc1xV4ybJN8CQhuEJbYHwGvyRnn7cNX2RIGcU63y6pByw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1699108147; a=rsa-sha256; cv=none; b=OrYaqfoesftOFumDpad6Sk9H2ocI7t5oxICDCsbWQTrQYABcr3I9PixO0foV8uHlHZbPve 1eim7XxLaSx5Vbx62TqO488LUVB8rz8gzRX/DQB+IJUAdc0rypmbi6MyHv5IoAlJtMTu1u t9V9g3EPh1ddxBU7nzSj+FzZrs7lqD5AAcoKESL5bk1t96Yx5rwzoqASZaefU1xC0Difee WbbIBtnarrl/+ly4PIaUT0OYztGPQPn+uwJtC41oWNVoCwPcDWx7xppztC40exFDr9Fq1d dXf4fy3IB4yyctHb+L9ibu5+Eovl1rEgg3riVRVmU1M+Lr6RUmwJfDTLvkO+lw== 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 4SN0Nf6gdGz11Vg; Sat, 4 Nov 2023 14:29:06 +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 3A4ET6kG019637; Sat, 4 Nov 2023 14:29:06 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 3A4ET6XR019634; Sat, 4 Nov 2023 14:29:06 GMT (envelope-from git) Date: Sat, 4 Nov 2023 14:29:06 GMT Message-Id: <202311041429.3A4ET6XR019634@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mark Johnston Subject: git: bd80263606d7 - main - 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/main X-Git-Reftype: branch X-Git-Commit: bd80263606d73c0391d3fa8a156fcca89a821810 Auto-Submitted: auto-generated The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=bd80263606d73c0391d3fa8a156fcca89a821810 commit bd80263606d73c0391d3fa8a156fcca89a821810 Author: Mark Johnston AuthorDate: 2023-11-04 14:28:24 +0000 Commit: Mark Johnston CommitDate: 2023-11-04 14:28:24 +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. Reported by: KMSAN Reviewed by: kp Fixes: 6fc7fc2dbb2b ("pfsync: transport over IPv6") MFC after: 3 days Differential Revision: https://reviews.freebsd.org/D42461 --- 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) {