[src/trunk]: src/sys/net/npf - npf_conn_init(): fix a race when initialising ...

[christos <christos%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/c5401537c477
branches:  trunk
changeset: 844162:c5401537c477
user:      christos <christos%NetBSD.org@localhost>
date:      Tue Aug 06 11:40:15 2019 +0000

description:
- npf_conn_init(): fix a race when initialising the G/C thread.
- Fix a bug when partially initialised connection is destroyed on error.
(from rmind@)

diffstat:

 sys/net/npf/npf.c      |   8 ++++++--
 sys/net/npf/npf_conn.c |  25 ++++++++-----------------
 sys/net/npf/npf_conn.h |   5 +++--
 3 files changed, 17 insertions(+), 21 deletions(-)

diffs (147 lines):

diff -r 05618bcecd68 -r c5401537c477 sys/net/npf/npf.c
--- a/sys/net/npf/npf.c Tue Aug 06 11:21:59 2019 +0000
+++ b/sys/net/npf/npf.c Tue Aug 06 11:40:15 2019 +0000
@@ -33,7 +33,7 @@
 
 #ifdef _KERNEL
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf.c,v 1.38 2019/07/23 00:52:01 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf.c,v 1.39 2019/08/06 11:40:15 christos Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -79,13 +79,17 @@
        npf_param_init(npf);
        npf_state_sysinit(npf);
        npf_ifmap_init(npf, ifops);
-       npf_conn_init(npf, flags);
+       npf_conn_init(npf);
        npf_portmap_init(npf);
        npf_alg_init(npf);
        npf_ext_init(npf);
 
        /* Load an empty configuration. */
        npf_config_init(npf);
+
+       if ((flags & NPF_NO_GC) == 0) {
+               npf_worker_register(npf, npf_conn_worker);
+       }
        return npf;
 }
 
diff -r 05618bcecd68 -r c5401537c477 sys/net/npf/npf_conn.c
--- a/sys/net/npf/npf_conn.c    Tue Aug 06 11:21:59 2019 +0000
+++ b/sys/net/npf/npf_conn.c    Tue Aug 06 11:40:15 2019 +0000
@@ -107,7 +107,7 @@
 
 #ifdef _KERNEL
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_conn.c,v 1.28 2019/08/06 10:25:13 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_conn.c,v 1.29 2019/08/06 11:40:15 christos Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -143,14 +143,13 @@
 enum { CONN_TRACKING_OFF, CONN_TRACKING_ON };
 
 static nvlist_t *npf_conn_export(npf_t *, npf_conn_t *);
-static void npf_conn_destroy_idx(npf_t *, npf_conn_t *, unsigned);
 
 /*
  * npf_conn_sys{init,fini}: initialise/destroy connection tracking.
  */
 
 void
-npf_conn_init(npf_t *npf, int flags)
+npf_conn_init(npf_t *npf)
 {
        npf->conn_cache[0] = pool_cache_init(
            offsetof(npf_conn_t, c_keys[NPF_CONNKEY_V4WORDS * 2]),
@@ -162,10 +161,6 @@
        mutex_init(&npf->conn_lock, MUTEX_DEFAULT, IPL_NONE);
        npf->conn_tracking = CONN_TRACKING_OFF;
        npf->conn_db = npf_conndb_create();
-
-       if ((flags & NPF_NO_GC) == 0) {
-               npf_worker_register(npf, npf_conn_worker);
-       }
        npf_conndb_sysinit(npf);
 }
 
@@ -430,10 +425,11 @@
 
        con->c_proto = npc->npc_proto;
        CTASSERT(sizeof(con->c_proto) >= sizeof(npc->npc_proto));
+       con->c_alen = alen;
 
        /* Initialize the protocol state. */
        if (!npf_state_init(npc, &con->c_state)) {
-               npf_conn_destroy_idx(npf, con, idx);
+               npf_conn_destroy(npf, con);
                return NULL;
        }
        KASSERT(npf_iscached(npc, NPC_IP46));
@@ -447,7 +443,7 @@
         */
        if (!npf_conn_conkey(npc, fw, true) ||
            !npf_conn_conkey(npc, bk, false)) {
-               npf_conn_destroy_idx(npf, con, idx);
+               npf_conn_destroy(npf, con);
                return NULL;
        }
        con->c_ifid = global ? nbuf->nb_ifid : 0;
@@ -500,14 +496,8 @@
 void
 npf_conn_destroy(npf_t *npf, npf_conn_t *con)
 {
-       const npf_connkey_t *key = npf_conn_getforwkey(con);
-       const unsigned alen = NPF_CONNKEY_ALEN(key);
-       npf_conn_destroy_idx(npf, con, NPF_CONNCACHE(alen));
-}
+       const unsigned idx __unused = NPF_CONNCACHE(con->c_alen);
 
-static void
-npf_conn_destroy_idx(npf_t *npf, npf_conn_t *con, unsigned idx)
-{
        KASSERT(con->c_refcnt == 0);
 
        if (con->c_nat) {
@@ -799,6 +789,7 @@
 
        fw = npf_conn_getforwkey(con);
        alen = NPF_CONNKEY_ALEN(fw);
+       KASSERT(alen == con->c_alen);
        bk = npf_conn_getbackkey(con, alen);
 
        kdict = npf_connkey_export(fw);
@@ -899,7 +890,7 @@
        npf_conndb_enqueue(cd, con);
        return 0;
 err:
-       npf_conn_destroy_idx(npf, con, idx);
+       npf_conn_destroy(npf, con);
        return EINVAL;
 }
 
diff -r 05618bcecd68 -r c5401537c477 sys/net/npf/npf_conn.h
--- a/sys/net/npf/npf_conn.h    Tue Aug 06 11:21:59 2019 +0000
+++ b/sys/net/npf/npf_conn.h    Tue Aug 06 11:40:15 2019 +0000
@@ -50,7 +50,8 @@
         * Protocol, address length, the interface ID (if zero,
         * then the state is global) and connection flags.
         */
-       unsigned                c_proto;
+       uint16_t                c_proto;
+       uint16_t                c_alen;
        unsigned                c_ifid;
        unsigned                c_flags;
 
@@ -123,7 +124,7 @@
 /*
  * Connection tracking interface.
  */
-void           npf_conn_init(npf_t *, int);
+void           npf_conn_init(npf_t *);
 void           npf_conn_fini(npf_t *);
 void           npf_conn_tracking(npf_t *, bool);
 void           npf_conn_load(npf_t *, npf_conndb_t *, bool);