git: cba481a7bff2 - main - PF_KEY socket: limit the length of copied socket address

The branch main has been updated by kib:

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

commit cba481a7bff2fcf31420ee8b2714660e2666452b
Author:     Konstantin Belousov <[email protected]>
AuthorDate: 2026-08-04 15:52:53 +0000
Commit:     Konstantin Belousov <[email protected]>
CommitDate: 2026-08-14 14:35:38 +0000

    PF_KEY socket: limit the length of copied socket address
    
    for SADB_UPDATE op SADB_X_EXT_NEW_ADDRESS_SRC/DST extensions, by
    checking the sa_len matching the address family requirements before
    doing the copy.
    
    Also convert KEY_SETSECASIDX() and KEY_SETSECSPIDX() to functions and
    apply the sa_len clamping there.
    
    See https://github.com/0xdeadbeefnetwork/pfkey-sadb-overflow
    
    PR:     297264
    Tested by:      Wafa Hamzah <[email protected]> (previous version)
    Reviewed by:    markj
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D58646
---
 sys/netipsec/key.c | 128 +++++++++++++++++++++++++++++------------------------
 1 file changed, 69 insertions(+), 59 deletions(-)

diff --git a/sys/netipsec/key.c b/sys/netipsec/key.c
index 3cab8b4b6738..de1481ba4725 100644
--- a/sys/netipsec/key.c
+++ b/sys/netipsec/key.c
@@ -574,32 +574,42 @@ static int key_checksockaddrs(const struct sockaddr *src,
  * set parameters into secpolicyindex buffer.
  * Must allocate secpolicyindex buffer passed to this function.
  */
-#define KEY_SETSECSPIDX(_dir, s, d, ps, pd, ulp, idx) \
-do { \
-	bzero((idx), sizeof(struct secpolicyindex));                         \
-	(idx)->dir = (_dir);                                                 \
-	(idx)->prefs = (ps);                                                 \
-	(idx)->prefd = (pd);                                                 \
-	(idx)->ul_proto = (ulp);                                             \
-	bcopy((s), &(idx)->src, ((const struct sockaddr *)(s))->sa_len);     \
-	bcopy((d), &(idx)->dst, ((const struct sockaddr *)(d))->sa_len);     \
-} while (0)
+static void
+key_setsecspidx(uint8_t dir, const void *s, const void *d, uint8_t prefs,
+    uint8_t prefd, uint8_t ul_proto, struct secpolicyindex *idx)
+{
+	MPASS(key_checksockaddrs((const struct sockaddr *)s,
+	    (const struct sockaddr *)d) == 0);
+
+	memset(idx, 0, sizeof(*idx));
+	idx->dir = dir;
+	idx->prefs = prefs;
+	idx->prefd = prefd;
+	idx->ul_proto = ul_proto;
+	memcpy(&idx->src, s, ((const struct sockaddr *)s)->sa_len);
+	memcpy(&idx->dst, d, ((const struct sockaddr *)d)->sa_len);
+}
 
 /*
  * set parameters into secasindex buffer.
  * Must allocate secasindex buffer before calling this function.
  */
-#define KEY_SETSECASIDX(p, m, r, s, d, idx) \
-do { \
-	bzero((idx), sizeof(struct secasindex));                             \
-	(idx)->proto = (p);                                                  \
-	(idx)->mode = (m);                                                   \
-	(idx)->reqid = (r);                                                  \
-	bcopy((s), &(idx)->src, ((const struct sockaddr *)(s))->sa_len);     \
-	bcopy((d), &(idx)->dst, ((const struct sockaddr *)(d))->sa_len);     \
-	key_porttosaddr(&(idx)->src.sa, 0);				     \
-	key_porttosaddr(&(idx)->dst.sa, 0);				     \
-} while (0)
+static void
+key_setsecasidx(uint8_t proto, uint8_t mode, uint32_t reqid,
+    const void *s, const void *d, struct secasindex *idx)
+{
+	MPASS(key_checksockaddrs((const struct sockaddr *)s,
+	    (const struct sockaddr *)d) == 0);
+
+	memset(idx, 0, sizeof(*idx));
+	idx->proto = proto;
+	idx->mode = mode;
+	idx->reqid = reqid;
+	memcpy(&idx->src, s, ((const struct sockaddr *)s)->sa_len);
+	memcpy(&idx->dst, d, ((const struct sockaddr *)d)->sa_len);
+	key_porttosaddr(&idx->src.sa, 0);
+	key_porttosaddr(&idx->dst.sa, 0);
+}
 
 /* key statistics */
 struct _keystat {
@@ -1209,7 +1219,7 @@ key_allocsa_tunnel(union sockaddr_union *src, union sockaddr_union *dst,
 	IPSEC_ASSERT(src != NULL, ("null src address"));
 	IPSEC_ASSERT(dst != NULL, ("null dst address"));
 
-	KEY_SETSECASIDX(proto, IPSEC_MODE_TUNNEL, 0, &src->sa,
+	key_setsecasidx(proto, IPSEC_MODE_TUNNEL, 0, &src->sa,
 	    &dst->sa, &saidx);
 
 	sav = NULL;
@@ -2110,13 +2120,9 @@ key_spdadd(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 		return key_senderror(so, m, error);
 	}
 	/* make secindex */
-	KEY_SETSECSPIDX(xpl0->sadb_x_policy_dir,
-	                src0 + 1,
-	                dst0 + 1,
-	                src0->sadb_address_prefixlen,
-	                dst0->sadb_address_prefixlen,
-	                src0->sadb_address_proto,
-	                &spidx);
+	key_setsecspidx(xpl0->sadb_x_policy_dir, src0 + 1, dst0 + 1,
+	    src0->sadb_address_prefixlen, dst0->sadb_address_prefixlen,
+	    src0->sadb_address_proto, &spidx);
 	/* Checking there is SP already or not. */
 	oldsp = key_getsp(&spidx);
 	if (oldsp != NULL) {
@@ -2351,13 +2357,9 @@ key_spddelete(struct socket *so, struct mbuf *m,
 		return key_senderror(so, m, EINVAL);
 	}
 	/* make secindex */
-	KEY_SETSECSPIDX(xpl0->sadb_x_policy_dir,
-	                src0 + 1,
-	                dst0 + 1,
-	                src0->sadb_address_prefixlen,
-	                dst0->sadb_address_prefixlen,
-	                src0->sadb_address_proto,
-	                &spidx);
+	key_setsecspidx(xpl0->sadb_x_policy_dir, src0 + 1, dst0 + 1,
+	    src0->sadb_address_prefixlen, dst0->sadb_address_prefixlen,
+	    src0->sadb_address_proto, &spidx);
 
 	/* Is there SP in SPD ? */
 	if ((sp = key_getsp(&spidx)) == NULL) {
@@ -5121,7 +5123,7 @@ key_getspi(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 		error = EINVAL;
 		goto fail;
 	}
-	KEY_SETSECASIDX(proto, mode, reqid, src0 + 1, dst0 + 1, &saidx);
+	key_setsecasidx(proto, mode, reqid, src0 + 1, dst0 + 1, &saidx);
 
 	/* SPI allocation */
 	SPI_ALLOC_LOCK();
@@ -5354,36 +5356,44 @@ key_updateaddresses(struct socket *so, struct mbuf *m,
     const struct sadb_msghdr *mhp, struct secasvar *sav,
     struct secasindex *saidx)
 {
-	struct sockaddr *newaddr;
+	struct sockaddr *dnewaddr, *snewaddr;
 	struct secashead *sah;
 	struct secasvar *newsav, *tmp;
 	struct mbuf *n;
 	int error, isnew;
+	bool newsaidx;
 
+	newsaidx = false;
 	/* Check that we need to change SAH */
 	if (!SADB_CHECKHDR(mhp, SADB_X_EXT_NEW_ADDRESS_SRC)) {
-		newaddr = (struct sockaddr *)(
-		    ((struct sadb_address *)
+		snewaddr = (struct sockaddr *)(((struct sadb_address *)
 		    mhp->ext[SADB_X_EXT_NEW_ADDRESS_SRC]) + 1);
-		bcopy(newaddr, &saidx->src, newaddr->sa_len);
-		key_porttosaddr(&saidx->src.sa, 0);
+		newsaidx = true;
+	} else {
+		snewaddr = &saidx->src.sa;
 	}
 	if (!SADB_CHECKHDR(mhp, SADB_X_EXT_NEW_ADDRESS_DST)) {
-		newaddr = (struct sockaddr *)(
-		    ((struct sadb_address *)
+		dnewaddr = (struct sockaddr *)(((struct sadb_address *)
 		    mhp->ext[SADB_X_EXT_NEW_ADDRESS_DST]) + 1);
-		bcopy(newaddr, &saidx->dst, newaddr->sa_len);
+		newsaidx = true;
+	} else {
+		dnewaddr = &saidx->dst.sa;
+	}
+	error = key_checksockaddrs(snewaddr, dnewaddr);
+	if (error != 0) {
+		ipseclog((LOG_DEBUG, "%s: invalid new sockaddr.\n",
+		    __func__));
+		return (error);
+	}
+	if (!SADB_CHECKHDR(mhp, SADB_X_EXT_NEW_ADDRESS_SRC)) {
+		memcpy(&saidx->src, snewaddr, snewaddr->sa_len);
+		key_porttosaddr(&saidx->src.sa, 0);
+	}
+	if (!SADB_CHECKHDR(mhp, SADB_X_EXT_NEW_ADDRESS_DST)) {
+		memcpy(&saidx->dst, dnewaddr, dnewaddr->sa_len);
 		key_porttosaddr(&saidx->dst.sa, 0);
 	}
-	if (!SADB_CHECKHDR(mhp, SADB_X_EXT_NEW_ADDRESS_SRC) ||
-	    !SADB_CHECKHDR(mhp, SADB_X_EXT_NEW_ADDRESS_DST)) {
-		error = key_checksockaddrs(&saidx->src.sa, &saidx->dst.sa);
-		if (error != 0) {
-			ipseclog((LOG_DEBUG, "%s: invalid new sockaddr.\n",
-			    __func__));
-			return (error);
-		}
-
+	if (newsaidx) {
 		sah = key_getsah(saidx);
 		if (sah == NULL) {
 			/* create a new SA index */
@@ -5640,7 +5650,7 @@ key_update(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 		ipseclog((LOG_DEBUG, "%s: invalid sockaddr.\n", __func__));
 		return key_senderror(so, m, error);
 	}
-	KEY_SETSECASIDX(proto, mode, reqid, src0 + 1, dst0 + 1, &saidx);
+	key_setsecasidx(proto, mode, reqid, src0 + 1, dst0 + 1, &saidx);
 	sav = key_getsavbyspi(sa0->sadb_sa_spi);
 	if (sav == NULL) {
 		ipseclog((LOG_DEBUG, "%s: no SA found for SPI %u\n",
@@ -5873,7 +5883,7 @@ key_add(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 		ipseclog((LOG_DEBUG, "%s: invalid sockaddr.\n", __func__));
 		return key_senderror(so, m, error);
 	}
-	KEY_SETSECASIDX(proto, mode, reqid, src0 + 1, dst0 + 1, &saidx);
+	key_setsecasidx(proto, mode, reqid, src0 + 1, dst0 + 1, &saidx);
 	spi = sa0->sadb_sa_spi;
 	/*
 	 * For TCP-MD5 SAs we don't use SPI. Check the uniqueness using
@@ -6328,7 +6338,7 @@ key_delete(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 		ipseclog((LOG_DEBUG, "%s: invalid sockaddr.\n", __func__));
 		return (key_senderror(so, m, EINVAL));
 	}
-	KEY_SETSECASIDX(proto, IPSEC_MODE_ANY, 0, src0 + 1, dst0 + 1, &saidx);
+	key_setsecasidx(proto, IPSEC_MODE_ANY, 0, src0 + 1, dst0 + 1, &saidx);
 	if (SADB_CHECKHDR(mhp, SADB_EXT_SA)) {
 		/*
 		 * Caller wants us to delete all non-LARVAL SAs
@@ -6559,7 +6569,7 @@ key_get(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 		ipseclog((LOG_DEBUG, "%s: invalid sockaddr.\n", __func__));
 		return key_senderror(so, m, EINVAL);
 	}
-	KEY_SETSECASIDX(proto, IPSEC_MODE_ANY, 0, src0 + 1, dst0 + 1, &saidx);
+	key_setsecasidx(proto, IPSEC_MODE_ANY, 0, src0 + 1, dst0 + 1, &saidx);
 
 	SPI_ALLOC_LOCK();
 	if (proto == IPPROTO_TCP)
@@ -7376,7 +7386,7 @@ key_acquire2(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 		ipseclog((LOG_DEBUG, "%s: invalid sockaddr.\n", __func__));
 		return key_senderror(so, m, EINVAL);
 	}
-	KEY_SETSECASIDX(proto, mode, reqid, src0 + 1, dst0 + 1, &saidx);
+	key_setsecasidx(proto, mode, reqid, src0 + 1, dst0 + 1, &saidx);
 
 	/* get a SA index */
 	SAHTREE_RLOCK();