To fix crashes seen by Hrvoje with pfsync, IPsec and parallel

forwarding, protect tdb flags and lists in pfsync with a mutex.
help and OK sashan@

diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c
index 4c538ca..edeb220 100644 (file)
--- a/sys/net/if_pfsync.c
+++ b/sys/net/if_pfsync.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_pfsync.c,v 1.299 2021/11/25 13:46:02 bluhm Exp $   */
+/*     $OpenBSD: if_pfsync.c,v 1.300 2022/02/25 22:18:44 bluhm Exp $   */
 
 /*
  * Copyright (c) 2002 Michael Shalayeff
@@ -295,7 +295,7 @@ void        pfsync_bulk_update(void *);
 void   pfsync_bulk_fail(void *);
 
 void   pfsync_grab_snapshot(struct pfsync_snapshot *, struct pfsync_softc *);
-void   pfsync_drop_snapshot(struct pfsync_snapshot *);
+void   pfsync_drop_snapshot(struct pfsync_snapshot *, struct pfsync_softc *);
 
 void   pfsync_send_dispatch(void *);
 void   pfsync_send_pkt(struct mbuf *);
@@ -1618,7 +1618,7 @@ pfsync_grab_snapshot(struct pfsync_snapshot *sn, struct pfsync_softc *sc)
 }
 
 void
-pfsync_drop_snapshot(struct pfsync_snapshot *sn)
+pfsync_drop_snapshot(struct pfsync_snapshot *sn, struct pfsync_softc * sc)
 {
        struct pf_state *st;
        struct pfsync_upd_req_item *ur;
@@ -1645,10 +1645,14 @@ pfsync_drop_snapshot(struct pfsync_snapshot *sn)
                pool_put(&sn->sn_sc->sc_pool, ur);
        }
 
+       mtx_enter(&sc->sc_tdb_mtx);
        while ((t = TAILQ_FIRST(&sn->sn_tdb_q)) != NULL) {
                TAILQ_REMOVE(&sn->sn_tdb_q, t, tdb_sync_entry);
+               mtx_enter(&t->tdb_mtx);
                CLR(t->tdb_flags, TDBF_PFSYNC);
+               mtx_leave(&t->tdb_mtx);
        }
+       mtx_leave(&sc->sc_tdb_mtx);
 }
 
 int
@@ -1675,7 +1679,7 @@ pfsync_drop(struct pfsync_softc *sc)
        struct pfsync_snapshot  sn;
 
        pfsync_grab_snapshot(&sn, sc);
-       pfsync_drop_snapshot(&sn);
+       pfsync_drop_snapshot(&sn, sc);
 }
 
 void
@@ -1767,7 +1771,7 @@ pfsync_sendout(void)
        if (m == NULL) {
                sc->sc_if.if_oerrors++;
                pfsyncstat_inc(pfsyncs_onomem);
-               pfsync_drop_snapshot(&sn);
+               pfsync_drop_snapshot(&sn, sc);
                return;
        }
 
@@ -1777,7 +1781,7 @@ pfsync_sendout(void)
                        m_free(m);
                        sc->sc_if.if_oerrors++;
                        pfsyncstat_inc(pfsyncs_onomem);
-                       pfsync_drop_snapshot(&sn);
+                       pfsync_drop_snapshot(&sn, sc);
                        return;
                }
        }
@@ -1835,14 +1839,18 @@ pfsync_sendout(void)
                subh = (struct pfsync_subheader *)(m->m_data + offset);
                offset += sizeof(*subh);
 
+               mtx_enter(&sc->sc_tdb_mtx);
                count = 0;
                while ((t = TAILQ_FIRST(&sn.sn_tdb_q)) != NULL) {
                        TAILQ_REMOVE(&sn.sn_tdb_q, t, tdb_sync_entry);
                        pfsync_out_tdb(t, m->m_data + offset);
                        offset += sizeof(struct pfsync_tdb);
+                       mtx_enter(&t->tdb_mtx);
                        CLR(t->tdb_flags, TDBF_PFSYNC);
+                       mtx_leave(&t->tdb_mtx);
                        count++;
                }
+               mtx_leave(&sc->sc_tdb_mtx);
 
                bzero(subh, sizeof(*subh));
                subh->action = PFSYNC_ACT_TDB;
@@ -2477,21 +2485,31 @@ pfsync_update_tdb(struct tdb *t, int output)
                        mtx_enter(&sc->sc_tdb_mtx);
                        nlen = sizeof(struct pfsync_tdb);
 
+                       mtx_enter(&t->tdb_mtx);
+                       if (ISSET(t->tdb_flags, TDBF_PFSYNC)) {
+                               /* we've lost race, no action for us then */
+                               mtx_leave(&t->tdb_mtx);
+                               mtx_leave(&sc->sc_tdb_mtx);
+                               break;
+                       }
+
                        if (TAILQ_EMPTY(&sc->sc_tdb_q))
                                nlen += sizeof(struct pfsync_subheader);
 
                        sc_len = atomic_add_long_nv(&sc->sc_len, nlen);
                        if (sc_len > sc->sc_if.if_mtu) {
                                atomic_sub_long(&sc->sc_len, nlen);
+                               mtx_leave(&t->tdb_mtx);
                                mtx_leave(&sc->sc_tdb_mtx);
                                pfsync_sendout();
                                continue;
                        }
 
                        TAILQ_INSERT_TAIL(&sc->sc_tdb_q, t, tdb_sync_entry);
-                       mtx_leave(&sc->sc_tdb_mtx);
-
                        SET(t->tdb_flags, TDBF_PFSYNC);
+                       mtx_leave(&t->tdb_mtx);
+
+                       mtx_leave(&sc->sc_tdb_mtx);
                        t->tdb_updates = 0;
                } while (0);
        } else {
@@ -2499,10 +2517,12 @@ pfsync_update_tdb(struct tdb *t, int output)
                        schednetisr(NETISR_PFSYNC);
        }
 
+       mtx_enter(&t->tdb_mtx);
        if (output)
                SET(t->tdb_flags, TDBF_PFSYNC_RPL);
        else
                CLR(t->tdb_flags, TDBF_PFSYNC_RPL);
+       mtx_leave(&t->tdb_mtx);
 }
 
 void
@@ -2517,7 +2537,9 @@ pfsync_delete_tdb(struct tdb *t)
        mtx_enter(&sc->sc_tdb_mtx);
 
        TAILQ_REMOVE(&sc->sc_tdb_q, t, tdb_sync_entry);
+       mtx_enter(&t->tdb_mtx);
        CLR(t->tdb_flags, TDBF_PFSYNC);
+       mtx_leave(&t->tdb_mtx);
 
        nlen = sizeof(struct pfsync_tdb);
        if (TAILQ_EMPTY(&sc->sc_tdb_q))