git: 10ea195fa269 - main - ipsec: add a lock encompassing SPI allocation

From: Mateusz Guzik <mjg_at_FreeBSD.org>
Date: Wed, 03 Nov 2021 19:51:45 UTC
The branch main has been updated by mjg:

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

commit 10ea195fa269888d362f548279f3d3fa252662d7
Author:     Mateusz Guzik <mjg@FreeBSD.org>
AuthorDate: 2021-11-03 13:10:34 +0000
Commit:     Mateusz Guzik <mjg@FreeBSD.org>
CommitDate: 2021-11-03 19:51:40 +0000

    ipsec: add a lock encompassing SPI allocation
    
    SPIs get allocated and inserted in separate steps. Prior to the change
    there was nothing preventing 2 differnet threads from ending up with the
    same one.
    
    PR:             258849
    Reported by:    Herbie.Robinson@stratus.com
    Reviewed by:    ae
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D32826
---
 sys/netipsec/key.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/sys/netipsec/key.c b/sys/netipsec/key.c
index 48de29305b3c..9a810fa49931 100644
--- a/sys/netipsec/key.c
+++ b/sys/netipsec/key.c
@@ -216,6 +216,13 @@ VNET_DEFINE_STATIC(struct mtx *, spdcache_lock);
 #define	SPDCACHE_LOCK(a)		mtx_lock(&V_spdcache_lock[a]);
 #define	SPDCACHE_UNLOCK(a)		mtx_unlock(&V_spdcache_lock[a]);
 
+static struct sx spi_alloc_lock;
+#define	SPI_ALLOC_LOCK_INIT()		sx_init(&spi_alloc_lock, "spialloc")
+#define	SPI_ALLOC_LOCK_DESTROY()	sx_destroy(&spi_alloc_lock)
+#define	SPI_ALLOC_LOCK()          	sx_xlock(&spi_alloc_lock)
+#define	SPI_ALLOC_UNLOCK()        	sx_unlock(&spi_alloc_lock)
+#define	SPI_ALLOC_LOCK_ASSERT()   	sx_assert(&spi_alloc_lock, SA_XLOCKED)
+
 /* SAD */
 TAILQ_HEAD(secashead_queue, secashead);
 LIST_HEAD(secashead_list, secashead);
@@ -4902,6 +4909,7 @@ key_getspi(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 	KEY_SETSECASIDX(proto, mode, reqid, src0 + 1, dst0 + 1, &saidx);
 
 	/* SPI allocation */
+	SPI_ALLOC_LOCK();
 	spi = key_do_getnewspi(
 	    (struct sadb_spirange *)mhp->ext[SADB_EXT_SPIRANGE], &saidx);
 	if (spi == 0) {
@@ -4909,10 +4917,12 @@ key_getspi(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 		 * Requested SPI or SPI range is not available or
 		 * already used.
 		 */
+		SPI_ALLOC_UNLOCK();
 		error = EEXIST;
 		goto fail;
 	}
 	sav = key_newsav(mhp, &saidx, spi, &error);
+	SPI_ALLOC_UNLOCK();
 	if (sav == NULL)
 		goto fail;
 
@@ -5021,6 +5031,8 @@ key_do_getnewspi(struct sadb_spirange *spirange, struct secasindex *saidx)
 	uint32_t min, max, newspi, t;
 	int tries, limit;
 
+	SPI_ALLOC_LOCK_ASSERT();
+
 	/* set spi range to allocate */
 	if (spirange != NULL) {
 		min = spirange->sadb_spirange_min;
@@ -5629,9 +5641,11 @@ key_add(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 	 * secasindex.
 	 * XXXAE: IPComp seems also doesn't use SPI.
 	 */
+	SPI_ALLOC_LOCK();
 	if (proto == IPPROTO_TCP) {
 		sav = key_getsav_tcpmd5(&saidx, &spi);
 		if (sav == NULL && spi == 0) {
+			SPI_ALLOC_UNLOCK();
 			/* Failed to allocate SPI */
 			ipseclog((LOG_DEBUG, "%s: SA already exists.\n",
 			    __func__));
@@ -5643,12 +5657,14 @@ key_add(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 		sav = key_getsavbyspi(spi);
 	}
 	if (sav != NULL) {
+		SPI_ALLOC_UNLOCK();
 		key_freesav(&sav);
 		ipseclog((LOG_DEBUG, "%s: SA already exists.\n", __func__));
 		return key_senderror(so, m, EEXIST);
 	}
 
 	sav = key_newsav(mhp, &saidx, spi, &error);
+	SPI_ALLOC_UNLOCK();
 	if (sav == NULL)
 		return key_senderror(so, m, error);
 	KEYDBG(KEY_STAMP,
@@ -6023,10 +6039,12 @@ key_delete(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 		return (key_senderror(so, m, EINVAL));
 	}
 	sa0 = (struct sadb_sa *)mhp->ext[SADB_EXT_SA];
+	SPI_ALLOC_LOCK();
 	if (proto == IPPROTO_TCP)
 		sav = key_getsav_tcpmd5(&saidx, NULL);
 	else
 		sav = key_getsavbyspi(sa0->sadb_sa_spi);
+	SPI_ALLOC_UNLOCK();
 	if (sav == NULL) {
 		ipseclog((LOG_DEBUG, "%s: no SA found for SPI %u.\n",
 		    __func__, ntohl(sa0->sadb_sa_spi)));
@@ -6235,10 +6253,12 @@ key_get(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 	}
 	KEY_SETSECASIDX(proto, IPSEC_MODE_ANY, 0, src0 + 1, dst0 + 1, &saidx);
 
+	SPI_ALLOC_LOCK();
 	if (proto == IPPROTO_TCP)
 		sav = key_getsav_tcpmd5(&saidx, NULL);
 	else
 		sav = key_getsavbyspi(sa0->sadb_sa_spi);
+	SPI_ALLOC_UNLOCK();
 	if (sav == NULL) {
 		ipseclog((LOG_DEBUG, "%s: no SA found.\n", __func__));
 		return key_senderror(so, m, ESRCH);
@@ -8318,6 +8338,7 @@ key_init(void)
 	SAHTREE_LOCK_INIT();
 	ACQ_LOCK_INIT();
 	SPACQ_LOCK_INIT();
+	SPI_ALLOC_LOCK_INIT();
 
 #ifndef IPSEC_DEBUG2
 	callout_init(&key_timer, 1);
@@ -8442,6 +8463,7 @@ key_destroy(void)
 	SAHTREE_LOCK_DESTROY();
 	ACQ_LOCK_DESTROY();
 	SPACQ_LOCK_DESTROY();
+	SPI_ALLOC_LOCK_DESTROY();
 }
 #endif