From nobody Mon Nov 06 20:13:34 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 4SPMxB4vrCz4yy1c; Mon, 6 Nov 2023 20:13:34 +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 4SPMxB4N1Kz3Pfs; Mon, 6 Nov 2023 20:13:34 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1699301614; 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=b9jIWGDc+tu8mWD9ZtdI2JAGbRbZObKGQGSixMAHWpg=; b=osHRwoO/OFwU0YUc/lD/ZbJ0MXUixUFZbThuSaryfrkEeic0Kvpg150NTcyB2YLn73l53G jsY/vszKWQ+NlLPnkeikFJOvpyYflFxXOMoKOd3fn9os1n52Fa0fdvja3FswoAhYwH9km2 d3ympzAJq6D7mo9MDulE0alvuaG6K0awJZOU2DfiJjJmF6c3fZ7NeONwDiNhspXREgaKgc RtKzE5i9dgSdASQxgNrtPPjqLmf38WMfLoBqJRnj/vJTq61AhzkPLsDbuL4FFnl+yInCBd jiF5I5vl/xH0gjhxNkYIXRFlgKn2RkKDmrq4RbC3H2dKLDSITvkrbMEflrFeaA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1699301614; 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=b9jIWGDc+tu8mWD9ZtdI2JAGbRbZObKGQGSixMAHWpg=; b=bN1oCdnRsYjQxMqxkQX3tIAD2RAZjJL04p3tBV2mhNJaD3cYF8PEb27fVZ6Cho4jTdNUS9 DSOZsnYhhhl0nFWF5o3REw8muBQLfnIYzCCSiW40NqBLo8J6L5lYeLeGMC10lli6+z761i 2P5gGsKhovN0UF3/rtanzxpS+WzIlo80LkruNNRH5ECTk/HI47Z9kb0nnUPfWBK56ufWhf zVA9ju+/ztJOH2h4Lk+OGYAl5ey/+WwJlJ2gUwWWkHwFCEvbjiFuh1McvlT+FhUmov7ROd z5WXvPc20bZStOa4ixu3hDk9DXqOLkhSgMqgwCICRH0DzzIYR/BheGXg2w8/Dw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1699301614; a=rsa-sha256; cv=none; b=Wx7mKbDiccoxKpUpNlOcceioTgoCP3nK17JDQ2j4tq5Ynk0q+YzOxTkOXeTRV/SU7J1eKU Yyg5mf3+A3X5WTJ+v+1ZpqboXqi/sG9D67PuhAutEyvzWEIvG2szgL/Hm+z3K8zZCRX1AB 5+yUSzipAn97MsX8VhpjYJa69cI1p5p8d3FlHmEGYebPbkQRDNMy+CHFANLgJwEzLiC367 EtnvgJBno3wzDSbntCJ9p8PySHAOBi0qHrzUN7sZi1AXo07dWvqmhsYWrM5Au/uVVVjFqG NYnleT0YxanrjS6+twAYRtdKc3eDuTReoCrKR8VcKjJKqUQnyTkEYn16swH3MA== 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 4SPMxB38Y6z6GV; Mon, 6 Nov 2023 20:13:34 +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 3A6KDYvt010429; Mon, 6 Nov 2023 20:13:34 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 3A6KDYpt010426; Mon, 6 Nov 2023 20:13:34 GMT (envelope-from git) Date: Mon, 6 Nov 2023 20:13:34 GMT Message-Id: <202311062013.3A6KDYpt010426@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: 09cca16bfc26 - stable/14 - 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/stable/14 X-Git-Reftype: branch X-Git-Commit: 09cca16bfc26f669a5aea2a47e9891e34c3bebeb Auto-Submitted: auto-generated The branch stable/14 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=09cca16bfc26f669a5aea2a47e9891e34c3bebeb commit 09cca16bfc26f669a5aea2a47e9891e34c3bebeb Author: Mark Johnston AuthorDate: 2023-11-04 14:28:24 +0000 Commit: Mark Johnston CommitDate: 2023-11-06 20:11:35 +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 (cherry picked from commit bd80263606d73c0391d3fa8a156fcca89a821810) --- 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) {