git: cba481a7bff2 - main - PF_KEY socket: limit the length of copied socket address
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 14 Aug 2026 14:35:59 UTC
The branch main has been updated by kib:
URL: https://cgit.FreeBSD.org/src/commit/?id=cba481a7bff2fcf31420ee8b2714660e2666452b
commit cba481a7bff2fcf31420ee8b2714660e2666452b
Author: Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2026-08-04 15:52:53 +0000
Commit: Konstantin Belousov <kib@FreeBSD.org>
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 <wafah@nvidia.com> (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();