git: feecef8df3d8 - releng/14.5 - PF_KEY socket: limit the length of copied socket address

Colin Percival <[email protected]>
Newsgroups gmane.os.freebsd.devel.cvs.src
Message-ID <6a83b51e.1820c.4782cd8c__33610.629981716$1787016496$gmane$org@gitrepo.freebsd.org>
The branch releng/14.5 has been updated by cperciva:

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

commit feecef8df3d8bddf5842f937e93d19e918674a65
Author:     Konstantin Belousov <[email protected]>
AuthorDate: 2026-08-04 15:52:53 +0000
Commit:     Colin Percival <[email protected]>
CommitDate: 2026-08-18 01:27:38 +0000

    PF_KEY socket: limit the length of copied socket address
    
    Approved by:    re (cperciva)
    PR:     297264
    
    (cherry picked from commit cba481a7bff2fcf31420ee8b2714660e2666452b)
    (cherry picked from commit 022f935a1fbc75d39a055eee0a327cb2062a4596)
---
 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 74f4e2527401..2aa0e933ec44 100644
--- a/sys/netipsec/key.c
+++ b/sys/netipsec/key.c
@@ -546,32 +546,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 {
@@ -1182,7 +1192,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;
@@ -2056,13 +2066,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) {
@@ -2274,13 +2280,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) {
@@ -4947,7 +4949,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();
@@ -5180,36 +5182,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 */
@@ -5437,7 +5447,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",
@@ -5669,7 +5679,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
@@ -6057,7 +6067,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
@@ -6286,7 +6296,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)
@@ -7103,7 +7113,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();
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.