git: 8a9495517b0a - main - ipsec: Clear pad bytes in PF_KEY messages

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Mon, 16 Jan 2023 16:37:18 UTC
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=8a9495517b0ad54da9759a7ba2cc0b56f8e7c8f9

commit 8a9495517b0ad54da9759a7ba2cc0b56f8e7c8f9
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2023-01-16 15:46:20 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2023-01-16 16:27:54 +0000

    ipsec: Clear pad bytes in PF_KEY messages
    
    Various handlers for SADB messages will allocate a new mbuf and populate
    some structures in it.  Some of these structures, such as struct
    sadb_supported, contain small reserved fields that are not initialized
    and are thus leaked to userspace.
    
    Fix the problem by adding a helper to allocate zeroed mbufs.  This
    reduces code duplication and the overhead of zeroing these messages
    isn't harmful.
    
    Reviewed by:    zlei, melifaro
    Reported by:    KMSAN
    Sponsored by:   The FreeBSD Foundation
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D38068
---
 sys/netipsec/key.c | 69 +++++++++++++++++++++++-------------------------------
 1 file changed, 29 insertions(+), 40 deletions(-)

diff --git a/sys/netipsec/key.c b/sys/netipsec/key.c
index 98bd97f465bf..345c302c2a80 100644
--- a/sys/netipsec/key.c
+++ b/sys/netipsec/key.c
@@ -818,6 +818,25 @@ key_havesp_any(void)
 	return (V_spd_size != 0);
 }
 
+/*
+ * Allocate a single mbuf with a buffer of the desired length.  The buffer is
+ * pre-zeroed to help ensure that uninitialized pad bytes are not leaked.
+ */
+static struct mbuf *
+key_mget(u_int len)
+{
+	struct mbuf *m;
+
+	KASSERT(len <= MCLBYTES,
+	    ("%s: invalid buffer length %u", __func__, len));
+
+	m = m_get2(len, M_NOWAIT, MT_DATA, M_PKTHDR);
+	if (m == NULL)
+		return (NULL);
+	memset(mtod(m, void *), 0, len);
+	return (m);
+}
+
 /* %%% IPsec policy management */
 /*
  * Return current SPDB generation.
@@ -2353,14 +2372,8 @@ key_spddelete2(struct socket *so, struct mbuf *m,
 	/* create new sadb_msg to reply. */
 	len = PFKEY_ALIGN8(sizeof(struct sadb_msg));
 
-	MGETHDR(n, M_NOWAIT, MT_DATA);
-	if (n && len > MHLEN) {
-		if (!(MCLGET(n, M_NOWAIT))) {
-			m_freem(n);
-			n = NULL;
-		}
-	}
-	if (!n)
+	n = key_mget(len);
+	if (n == NULL)
 		return key_senderror(so, m, ENOBUFS);
 
 	n->m_len = len;
@@ -3792,14 +3805,8 @@ key_setsadbmsg(u_int8_t type, u_int16_t tlen, u_int8_t satype, u_int32_t seq,
 	len = PFKEY_ALIGN8(sizeof(struct sadb_msg));
 	if (len > MCLBYTES)
 		return NULL;
-	MGETHDR(m, M_NOWAIT, MT_DATA);
-	if (m && len > MHLEN) {
-		if (!(MCLGET(m, M_NOWAIT))) {
-			m_freem(m);
-			m = NULL;
-		}
-	}
-	if (!m)
+	m = key_mget(len);
+	if (m == NULL)
 		return NULL;
 	m->m_pkthdr.len = m->m_len = len;
 	m->m_next = NULL;
@@ -4979,14 +4986,8 @@ key_getspi(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 	len = PFKEY_ALIGN8(sizeof(struct sadb_msg)) +
 	    PFKEY_ALIGN8(sizeof(struct sadb_sa));
 
-	MGETHDR(n, M_NOWAIT, MT_DATA);
-	if (len > MHLEN) {
-		if (!(MCLGET(n, M_NOWAIT))) {
-			m_freem(n);
-			n = NULL;
-		}
-	}
-	if (!n) {
+	n = key_mget(len);
+	if (n == NULL) {
 		error = ENOBUFS;
 		goto fail;
 	}
@@ -7201,14 +7202,8 @@ key_register(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 	if (len > MCLBYTES)
 		return key_senderror(so, m, ENOBUFS);
 
-	MGETHDR(n, M_NOWAIT, MT_DATA);
-	if (n != NULL && len > MHLEN) {
-		if (!(MCLGET(n, M_NOWAIT))) {
-			m_freem(n);
-			n = NULL;
-		}
-	}
-	if (!n)
+	n = key_mget(len);
+	if (n == NULL)
 		return key_senderror(so, m, ENOBUFS);
 
 	n->m_pkthdr.len = n->m_len = len;
@@ -7839,14 +7834,8 @@ key_parse(struct mbuf *m, struct socket *so)
 	if (m->m_next) {
 		struct mbuf *n;
 
-		MGETHDR(n, M_NOWAIT, MT_DATA);
-		if (n && m->m_pkthdr.len > MHLEN) {
-			if (!(MCLGET(n, M_NOWAIT))) {
-				m_free(n);
-				n = NULL;
-			}
-		}
-		if (!n) {
+		n = key_mget(m->m_pkthdr.len);
+		if (n == NULL) {
 			m_freem(m);
 			return ENOBUFS;
 		}