[Bug 258849] IPSec may generate duplicate SPIs

From: <bugzilla-noreply_at_freebsd.org>
Date: Thu, 07 Oct 2021 08:41:26 UTC
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=258849

--- Comment #4 from Mateusz Guzik <mjg@FreeBSD.org> ---
So I don't think this is fixable in a clean manner without significant 
refactoring in the area.

I think the pragmatic thing to do here is to try to hoist the sahtree lock out
of key_do_getnewspi. The problem is that there are several callers and callees
to be adjusted which may end up being rather hairy.

As a cop out one can slap an additional lock around all of this, in this
manner:
diff --git a/sys/netipsec/key.c b/sys/netipsec/key.c
index 72c598586d8e..f5da63d7b8f1 100644
--- a/sys/netipsec/key.c
+++ b/sys/netipsec/key.c
@@ -5628,6 +5628,7 @@ 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) {
@@ -5648,6 +5649,7 @@ key_add(struct socket *so, struct mbuf *m, const struct
sadb_msghdr *mhp)
        }

        sav = key_newsav(mhp, &saidx, spi, &error);
+       SPI_ALLOC_UNLOCK();
        if (sav == NULL)
                return key_senderror(so, m, error);
        KEYDBG(KEY_STAMP,

Note this may be turn out to not be a big problem as key_newsav takes rm lock
for writing which already comes with drastic overhead.

Any thoughts on this, ae?

-- 
You are receiving this mail because:
You are the assignee for the bug.