Hi,
Hrvoje reported some crashes with pfsync, IPsec and parallel
forwarding. Some locks were missing around the tdb flags in pfsync.
ok?
bluhm
Index: net/if_pfsync.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_pfsync.c,v
retrieving revision 1.299
diff -u -p -r1.299 if_pfsync.c
--- net/if_pfsync.c 25 Nov 2021 13:46:02 -0000 1.299
+++ net/if_pfsync.c 24 Feb 2022 20:59:11 -0000
@@ -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_snaps
}
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_snaps
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;
@@ -2489,9 +2497,11 @@ pfsync_update_tdb(struct tdb *t, int out
}
TAILQ_INSERT_TAIL(&sc->sc_tdb_q, t, tdb_sync_entry);
- mtx_leave(&sc->sc_tdb_mtx);
-
+ mtx_enter(&t->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 +2509,12 @@ pfsync_update_tdb(struct tdb *t, int out
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 +2529,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))
