Protect the write access to the TDB flags field with a mutex per

TDB.  Clearing the timeout flags just before pool put in tdb_free()
does not make sense.  Move this to tdb_delete().  While there make
the parentheses in the flag check consistent.
tested by Hrvoje Popovski; OK tobhe@

diff --git a/sys/net/pfkeyv2_convert.c b/sys/net/pfkeyv2_convert.c
index 0cd9455..2d648d5 100644 (file)
--- a/sys/net/pfkeyv2_convert.c
+++ b/sys/net/pfkeyv2_convert.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pfkeyv2_convert.c,v 1.76 2021/11/25 13:46:02 bluhm Exp $      */
+/*     $OpenBSD: pfkeyv2_convert.c,v 1.77 2021/12/11 16:33:46 bluhm Exp $      */
 /*
  * The author of this code is Angelos D. Keromytis (angelos@keromytis.org)
  *
@@ -122,6 +122,7 @@ import_sa(struct tdb *tdb, struct sadb_sa *sadb_sa, struct ipsecinit *ii)
        if (!sadb_sa)
                return;
 
+       mtx_enter(&tdb->tdb_mtx);
        if (ii) {
                ii->ii_encalg = sadb_sa->sadb_sa_encrypt;
                ii->ii_authalg = sadb_sa->sadb_sa_auth;
@@ -145,6 +146,7 @@ import_sa(struct tdb *tdb, struct sadb_sa *sadb_sa, struct ipsecinit *ii)
 
        if (sadb_sa->sadb_sa_state != SADB_SASTATE_MATURE)
                tdb->tdb_flags |= TDBF_INVALID;
+       mtx_leave(&tdb->tdb_mtx);
 }
 
 /*
@@ -282,6 +284,7 @@ import_lifetime(struct tdb *tdb, struct sadb_lifetime *sadb_lifetime, int type)
        if (!sadb_lifetime)
                return;
 
+       mtx_enter(&tdb->tdb_mtx);
        switch (type) {
        case PFKEYV2_LIFETIME_HARD:
                if ((tdb->tdb_exp_allocations =
@@ -348,6 +351,7 @@ import_lifetime(struct tdb *tdb, struct sadb_lifetime *sadb_lifetime, int type)
                tdb->tdb_established = sadb_lifetime->sadb_lifetime_addtime;
                tdb->tdb_first_use = sadb_lifetime->sadb_lifetime_usetime;
        }
+       mtx_leave(&tdb->tdb_mtx);
 }
 
 /*

diff --git a/sys/netinet/ip_ah.c b/sys/netinet/ip_ah.c
index 009f5c1..63ef3eb 100644 (file)
--- a/sys/netinet/ip_ah.c
+++ b/sys/netinet/ip_ah.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_ah.c,v 1.168 2021/12/02 12:39:15 bluhm Exp $ */
+/*     $OpenBSD: ip_ah.c,v 1.169 2021/12/11 16:33:46 bluhm Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -612,8 +612,8 @@ ah_input(struct mbuf **mp, struct tdb *tdb, int skip, int protoff)
        ahstat_add(ahs_ibytes, ibytes);
 
        /* Hard expiration. */
-       if (tdb->tdb_flags & TDBF_BYTES &&
-           tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes) {
+       if ((tdb->tdb_flags & TDBF_BYTES) &&
+           (tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes)) {
                ipsecstat_inc(ipsec_exctdb);
                pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
                tdb_delete(tdb);
@@ -621,11 +621,15 @@ ah_input(struct mbuf **mp, struct tdb *tdb, int skip, int protoff)
        }
 
        /* Notify on expiration. */
-       if (tdb->tdb_flags & TDBF_SOFT_BYTES &&
-           tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes) {
+       mtx_enter(&tdb->tdb_mtx);
+       if ((tdb->tdb_flags & TDBF_SOFT_BYTES) &&
+           (tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes)) {
+               tdb->tdb_flags &= ~TDBF_SOFT_BYTES;  /* Turn off checking */
+               mtx_leave(&tdb->tdb_mtx);
+               /* may sleep in solock() for the pfkey socket */
                pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
-               tdb->tdb_flags &= ~TDBF_SOFT_BYTES;  /* Turn off checking. */
-       }
+       } else
+               mtx_leave(&tdb->tdb_mtx);
 
        /* Get crypto descriptors. */
        crp = crypto_getreq(1);
@@ -952,8 +956,8 @@ ah_output(struct mbuf *m, struct tdb *tdb, int skip, int protoff)
        ahstat_add(ahs_obytes, m->m_pkthdr.len - skip);
 
        /* Hard expiration. */
-       if (tdb->tdb_flags & TDBF_BYTES &&
-           tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes) {
+       if ((tdb->tdb_flags & TDBF_BYTES) &&
+           (tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes)) {
                ipsecstat_inc(ipsec_exctdb);
                pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
                tdb_delete(tdb);
@@ -962,11 +966,15 @@ ah_output(struct mbuf *m, struct tdb *tdb, int skip, int protoff)
        }
 
        /* Notify on expiration. */
-       if (tdb->tdb_flags & TDBF_SOFT_BYTES &&
-           tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes) {
+       mtx_enter(&tdb->tdb_mtx);
+       if ((tdb->tdb_flags & TDBF_SOFT_BYTES) &&
+           (tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes)) {
+               tdb->tdb_flags &= ~TDBF_SOFT_BYTES;  /* Turn off checking */
+               mtx_leave(&tdb->tdb_mtx);
+               /* may sleep in solock() for the pfkey socket */
                pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
-               tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking */
-       }
+       } else
+               mtx_leave(&tdb->tdb_mtx);
 
        /*
         * Loop through mbuf chain; if we find a readonly mbuf,

diff --git a/sys/netinet/ip_esp.c b/sys/netinet/ip_esp.c
index 4c8df54..1aa4d4b 100644 (file)
--- a/sys/netinet/ip_esp.c
+++ b/sys/netinet/ip_esp.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_esp.c,v 1.188 2021/11/21 16:17:48 mvs Exp $ */
+/*     $OpenBSD: ip_esp.c,v 1.189 2021/12/11 16:33:47 bluhm Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -433,11 +433,15 @@ esp_input(struct mbuf **mp, struct tdb *tdb, int skip, int protoff)
        }
 
        /* Notify on soft expiration */
+       mtx_enter(&tdb->tdb_mtx);
        if ((tdb->tdb_flags & TDBF_SOFT_BYTES) &&
            (tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes)) {
+               tdb->tdb_flags &= ~TDBF_SOFT_BYTES;  /* Turn off checking */
+               mtx_leave(&tdb->tdb_mtx);
+               /* may sleep in solock() for the pfkey socket */
                pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
-               tdb->tdb_flags &= ~TDBF_SOFT_BYTES;       /* Turn off checking */
-       }
+       } else
+               mtx_leave(&tdb->tdb_mtx);
 
        /* Get crypto descriptors */
        crp = crypto_getreq(esph && espx ? 2 : 1);
@@ -781,8 +785,8 @@ esp_output(struct mbuf *m, struct tdb *tdb, int skip, int protoff)
        espstat_add(esps_obytes, m->m_pkthdr.len - skip);
 
        /* Hard byte expiration. */
-       if (tdb->tdb_flags & TDBF_BYTES &&
-           tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes) {
+       if ((tdb->tdb_flags & TDBF_BYTES) &&
+           (tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes)) {
                ipsecstat_inc(ipsec_exctdb);
                pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
                tdb_delete(tdb);
@@ -791,11 +795,15 @@ esp_output(struct mbuf *m, struct tdb *tdb, int skip, int protoff)
        }
 
        /* Soft byte expiration. */
-       if (tdb->tdb_flags & TDBF_SOFT_BYTES &&
-           tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes) {
+       mtx_enter(&tdb->tdb_mtx);
+       if ((tdb->tdb_flags & TDBF_SOFT_BYTES) &&
+           (tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes)) {
+               tdb->tdb_flags &= ~TDBF_SOFT_BYTES;  /* Turn off checking */
+               mtx_leave(&tdb->tdb_mtx);
+               /* may sleep in solock() for the pfkey socket */
                pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
-               tdb->tdb_flags &= ~TDBF_SOFT_BYTES;    /* Turn off checking. */
-       }
+       } else
+               mtx_leave(&tdb->tdb_mtx);
 
        /*
         * Loop through mbuf chain; if we find a readonly mbuf,

diff --git a/sys/netinet/ip_ipcomp.c b/sys/netinet/ip_ipcomp.c
index 3bfb99c..6502646 100644 (file)
--- a/sys/netinet/ip_ipcomp.c
+++ b/sys/netinet/ip_ipcomp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ip_ipcomp.c,v 1.88 2021/11/21 16:17:48 mvs Exp $ */
+/* $OpenBSD: ip_ipcomp.c,v 1.89 2021/12/11 16:33:47 bluhm Exp $ */
 
 /*
  * Copyright (c) 2001 Jean-Jacques Bernard-Gundol (jj@wabbitt.org)
@@ -205,11 +205,15 @@ ipcomp_input(struct mbuf **mp, struct tdb *tdb, int skip, int protoff)
                goto drop;
        }
        /* Notify on soft expiration */
+       mtx_enter(&tdb->tdb_mtx);
        if ((tdb->tdb_flags & TDBF_SOFT_BYTES) &&
            (tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes)) {
+               tdb->tdb_flags &= ~TDBF_SOFT_BYTES;  /* Turn off checking */
+               mtx_leave(&tdb->tdb_mtx);
+               /* may sleep in solock() for the pfkey socket */
                pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
-               tdb->tdb_flags &= ~TDBF_SOFT_BYTES;     /* Turn off checking */
-       }
+       } else
+               mtx_leave(&tdb->tdb_mtx);
 
        /* In case it's not done already, adjust the size of the mbuf chain */
        m->m_pkthdr.len = clen + hlen + skip;
@@ -393,12 +397,18 @@ ipcomp_output(struct mbuf *m, struct tdb *tdb, int skip, int protoff)
                error = EINVAL;
                goto drop;
        }
+
        /* Soft byte expiration */
+       mtx_enter(&tdb->tdb_mtx);
        if ((tdb->tdb_flags & TDBF_SOFT_BYTES) &&
            (tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes)) {
+               tdb->tdb_flags &= ~TDBF_SOFT_BYTES;  /* Turn off checking */
+               mtx_leave(&tdb->tdb_mtx);
+               /* may sleep in solock() for the pfkey socket */
                pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
-               tdb->tdb_flags &= ~TDBF_SOFT_BYTES;     /* Turn off checking */
-       }
+       } else
+               mtx_leave(&tdb->tdb_mtx);
+
        /*
         * Loop through mbuf chain; if we find a readonly mbuf,
         * copy the packet.

diff --git a/sys/netinet/ip_ipsp.c b/sys/netinet/ip_ipsp.c
index 5146dcb..971112e 100644 (file)
--- a/sys/netinet/ip_ipsp.c
+++ b/sys/netinet/ip_ipsp.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_ipsp.c,v 1.263 2021/12/08 14:24:18 bluhm Exp $     */
+/*     $OpenBSD: ip_ipsp.c,v 1.264 2021/12/11 16:33:47 bluhm Exp $     */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr),
@@ -311,11 +311,13 @@ reserve_spi(u_int rdomain, u_int32_t sspi, u_int32_t tspi,
 #ifdef IPSEC
                /* Setup a "silent" expiration (since TDBF_INVALID's set). */
                if (ipsec_keep_invalid > 0) {
+                       mtx_enter(&tdbp->tdb_mtx);
                        tdbp->tdb_flags |= TDBF_TIMER;
                        tdbp->tdb_exp_timeout = ipsec_keep_invalid;
                        if (timeout_add_sec(&tdbp->tdb_timer_tmo,
                            ipsec_keep_invalid))
                                tdb_ref(tdbp);
+                       mtx_leave(&tdbp->tdb_mtx);
                }
 #endif
 
@@ -701,11 +703,14 @@ tdb_soft_timeout(void *v)
        struct tdb *tdb = v;
 
        NET_LOCK();
+       mtx_enter(&tdb->tdb_mtx);
        if (tdb->tdb_flags & TDBF_SOFT_TIMER) {
+               tdb->tdb_flags &= ~TDBF_SOFT_TIMER;
+               mtx_leave(&tdb->tdb_mtx);
                /* Soft expirations. */
                pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
-               tdb->tdb_flags &= ~TDBF_SOFT_TIMER;
-       }
+       } else
+               mtx_leave(&tdb->tdb_mtx);
        /* decrement refcount of the timeout argument */
        tdb_unref(tdb);
        NET_UNLOCK();
@@ -717,12 +722,15 @@ tdb_soft_firstuse(void *v)
        struct tdb *tdb = v;
 
        NET_LOCK();
+       mtx_enter(&tdb->tdb_mtx);
        if (tdb->tdb_flags & TDBF_SOFT_FIRSTUSE) {
+               tdb->tdb_flags &= ~TDBF_SOFT_FIRSTUSE;
+               mtx_leave(&tdb->tdb_mtx);
                /* If the TDB hasn't been used, don't renew it. */
                if (tdb->tdb_first_use != 0)
                        pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
-               tdb->tdb_flags &= ~TDBF_SOFT_FIRSTUSE;
-       }
+       } else
+               mtx_leave(&tdb->tdb_mtx);
        /* decrement refcount of the timeout argument */
        tdb_unref(tdb);
        NET_UNLOCK();
@@ -958,6 +966,9 @@ tdb_unbundle(struct tdb *tdbp)
 void
 tdb_deltimeouts(struct tdb *tdbp)
 {
+       mtx_enter(&tdbp->tdb_mtx);
+       tdbp->tdb_flags &= ~(TDBF_FIRSTUSE | TDBF_SOFT_FIRSTUSE | TDBF_TIMER |
+           TDBF_SOFT_TIMER);
        if (timeout_del(&tdbp->tdb_timer_tmo))
                tdb_unref(tdbp);
        if (timeout_del(&tdbp->tdb_first_tmo))
@@ -966,6 +977,7 @@ tdb_deltimeouts(struct tdb *tdbp)
                tdb_unref(tdbp);
        if (timeout_del(&tdbp->tdb_sfirst_tmo))
                tdb_unref(tdbp);
+       mtx_leave(&tdbp->tdb_mtx);
 }
 
 struct tdb *
@@ -1005,9 +1017,13 @@ tdb_dodelete(struct tdb *tdbp, int locked)
 {
        NET_ASSERT_LOCKED();
 
-       if (tdbp->tdb_flags & TDBF_DELETED)
+       mtx_enter(&tdbp->tdb_mtx);
+       if (tdbp->tdb_flags & TDBF_DELETED) {
+               mtx_leave(&tdbp->tdb_mtx);
                return;
+       }
        tdbp->tdb_flags |= TDBF_DELETED;
+       mtx_leave(&tdbp->tdb_mtx);
        if (locked)
                tdb_unlink_locked(tdbp);
        else
@@ -1034,6 +1050,7 @@ tdb_alloc(u_int rdomain)
        tdbp = pool_get(&tdb_pool, PR_WAITOK | PR_ZERO);
 
        refcnt_init(&tdbp->tdb_refcnt);
+       mtx_init(&tdbp->tdb_mtx, IPL_SOFTNET);
        TAILQ_INIT(&tdbp->tdb_policy_head);
 
        /* Record establishment time. */
@@ -1085,8 +1102,6 @@ tdb_free(struct tdb *tdbp)
        KASSERT(tdbp->tdb_inext == NULL);
 
        /* Remove expiration timeouts. */
-       tdbp->tdb_flags &= ~(TDBF_FIRSTUSE | TDBF_SOFT_FIRSTUSE | TDBF_TIMER |
-           TDBF_SOFT_TIMER);
        KASSERT(timeout_pending(&tdbp->tdb_timer_tmo) == 0);
        KASSERT(timeout_pending(&tdbp->tdb_first_tmo) == 0);
        KASSERT(timeout_pending(&tdbp->tdb_stimer_tmo) == 0);

diff --git a/sys/netinet/ip_ipsp.h b/sys/netinet/ip_ipsp.h
index f8ccdbf..fd822cf 100644 (file)
--- a/sys/netinet/ip_ipsp.h
+++ b/sys/netinet/ip_ipsp.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_ipsp.h,v 1.229 2021/12/08 14:24:18 bluhm Exp $     */
+/*     $OpenBSD: ip_ipsp.h,v 1.230 2021/12/11 16:33:47 bluhm Exp $     */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr),
@@ -314,6 +314,7 @@ struct ipsec_policy {
  *     I       immutable after creation
  *     N       net lock
  *     s       tdb_sadb_mtx
+ *     m       tdb_mtx
  */
 struct tdb {                           /* tunnel descriptor block */
        /*
@@ -331,6 +332,7 @@ struct tdb {                                /* tunnel descriptor block */
        struct tdb      *tdb_onext;
 
        struct refcnt   tdb_refcnt;
+       struct mutex    tdb_mtx;
 
        const struct xformsw    *tdb_xform;             /* Transform to use */
        const struct enc_xform  *tdb_encalgxform;       /* Enc algorithm */
@@ -364,7 +366,7 @@ struct tdb {                                /* tunnel descriptor block */
        "\21USEDTUNNEL\22UDPENCAP\23PFSYNC\24PFSYNC_RPL" \
        "\25ESN")
 
-       u_int32_t       tdb_flags;      /* Flags related to this TDB */
+       u_int32_t       tdb_flags;      /* [m] Flags related to this TDB */
 
        struct timeout  tdb_timer_tmo;
        struct timeout  tdb_first_tmo;

diff --git a/sys/netinet/ipsec_output.c b/sys/netinet/ipsec_output.c
index 6217553..7d863f1 100644 (file)
--- a/sys/netinet/ipsec_output.c
+++ b/sys/netinet/ipsec_output.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ipsec_output.c,v 1.93 2021/12/02 12:39:15 bluhm Exp $ */
+/*     $OpenBSD: ipsec_output.c,v 1.94 2021/12/11 16:33:47 bluhm Exp $ */
 /*
  * The author of this code is Angelos D. Keromytis (angelos@cis.upenn.edu)
  *
@@ -268,7 +268,9 @@ ipsp_process_packet(struct mbuf *m, struct tdb *tdb, int af, int tunalready)
                        }
 
                        /* Remember that we appended a tunnel header. */
+                       mtx_enter(&tdb->tdb_mtx);
                        tdb->tdb_flags |= TDBF_USEDTUNNEL;
+                       mtx_leave(&tdb->tdb_mtx);
                }
        }