git: 584ca49a07f4 - stable/13 - ipsec: Clear pad bytes in PF_KEY messages
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 01 Feb 2023 17:31:57 UTC
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;
}