Date: Wed, 1 Feb 2023 17:31:57 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 584ca49a07f4 - stable/13 - ipsec: Clear pad bytes in PF_KEY messages Message-ID: <202302011731.311HVvmj057078@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=584ca49a07f42c0b6d43687ae1763fd800089484 commit 584ca49a07f42c0b6d43687ae1763fd800089484 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2023-01-16 15:46:20 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2023-02-01 17:22:31 +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 (cherry picked from commit 8a9495517b0ad54da9759a7ba2cc0b56f8e7c8f9) --- 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 869d6b850aa0..efda68f09078 100644 --- a/sys/netipsec/key.c +++ b/sys/netipsec/key.c @@ -804,6 +804,25 @@ key_havesp(u_int dir) TAILQ_FIRST(&V_sptree[dir]) != NULL : 1); } +/* + * 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. @@ -2315,14 +2334,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; @@ -3753,14 +3766,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; @@ -4937,14 +4944,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; } @@ -7148,14 +7149,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; @@ -7786,14 +7781,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; }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202302011731.311HVvmj057078>