Introduce mutex(9) to protect pipex(4) session content.

With bluhm@'s diff for parallel forwarding pipex(4) could be accessed in
parallel through (*ifp->if_input)() -> ether_input() ->
pipex_pppoe_input(). PPPOE pipex(4) sessions are mostly immutable except
MPPE crypt.

The new per-session `pxs_mtx' mutex(9) used to protect session's
`ccp-id' which is incremented each time we send CCP reset-request.

The new `pxm_mtx' mutex(9) used to protect MPPE context. Each pipex(4)
session has two of them: one for the input and one for output path.

Where is no lock order limitations because those new mutex(9)'es never
held together.

ok bluhm@

diff --git a/sys/net/pipex.c b/sys/net/pipex.c
index c08261f..e2d0c5c 100644 (file)
--- a/sys/net/pipex.c
+++ b/sys/net/pipex.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pipex.c,v 1.134 2021/07/20 16:44:55 mvs Exp $ */
+/*     $OpenBSD: pipex.c,v 1.135 2021/07/27 09:29:09 mvs Exp $ */
 
 /*-
  * Copyright (c) 2009 Internet Initiative Japan Inc.
@@ -263,6 +263,7 @@ pipex_init_session(struct pipex_session **rsession,
 
        /* prepare a new session */
        session = pool_get(&pipex_session_pool, PR_WAITOK | PR_ZERO);
+       mtx_init(&session->pxs_mtx, IPL_SOFTNET);
        session->state = PIPEX_STATE_INITIAL;
        session->protocol = req->pr_protocol;
        session->session_id = req->pr_session_id;
@@ -2099,6 +2100,7 @@ pipex_mppe_init(struct pipex_mppe *mppe, int stateless, int keylenbits,
     u_char *master_key, int has_oldkey)
 {
        memset(mppe, 0, sizeof(struct pipex_mppe));
+       mtx_init(&mppe->pxm_mtx, IPL_SOFTNET);
        if (stateless)
                mppe->stateless = 1;
        if (has_oldkey)
@@ -2238,11 +2240,14 @@ pipex_mppe_input(struct mbuf *m0, struct pipex_session *session)
        coher_cnt &= PIPEX_COHERENCY_CNT_MASK;
        pktloss = 0;
 
+       mtx_enter(&mppe->pxm_mtx);
+
        PIPEX_MPPE_DBG((session, LOG_DEBUG, "in coher_cnt=%03x %s%s",
            mppe->coher_cnt, (flushed) ? "[flushed]" : "",
            (encrypt) ? "[encrypt]" : ""));
 
        if (encrypt == 0) {
+               mtx_leave(&mppe->pxm_mtx);
                pipex_session_log(session, LOG_DEBUG,
                    "Received unexpected MPPE packet.(no ecrypt)");
                goto drop;
@@ -2274,6 +2279,7 @@ pipex_mppe_input(struct mbuf *m0, struct pipex_session *session)
                        pipex_session_log(session, LOG_DEBUG,
                            "Workaround the out-of-sequence PPP framing problem: "
                            "%d => %d", mppe->coher_cnt, coher_cnt);
+                       mtx_leave(&mppe->pxm_mtx);
                        goto drop;
                }
                rewind = 1;
@@ -2305,10 +2311,19 @@ pipex_mppe_input(struct mbuf *m0, struct pipex_session *session)
                        coher_cnt &= PIPEX_COHERENCY_CNT_MASK;
                        mppe->coher_cnt = coher_cnt;
                } else if (mppe->coher_cnt != coher_cnt) {
+                       int ccp_id;
+
+                       mtx_leave(&mppe->pxm_mtx);
+
                        /* Send CCP ResetReq */
                        PIPEX_DBG((session, LOG_DEBUG, "CCP SendResetReq"));
-                       pipex_ccp_output(session, CCP_RESETREQ,
-                           session->ccp_id++);
+
+                       mtx_enter(&session->pxs_mtx);
+                       ccp_id = session->ccp_id;
+                       session->ccp_id++;
+                       mtx_leave(&session->pxs_mtx);
+
+                       pipex_ccp_output(session, CCP_RESETREQ, ccp_id);
                        goto drop;
                }
                if ((coher_cnt & 0xff) == 0xff) {
@@ -2336,6 +2351,9 @@ pipex_mppe_input(struct mbuf *m0, struct pipex_session *session)
                mppe->coher_cnt++;
                mppe->coher_cnt &= PIPEX_COHERENCY_CNT_MASK;
        }
+
+       mtx_leave(&mppe->pxm_mtx);
+
        if (m0->m_pkthdr.len < PIPEX_PPPMINLEN)
                goto drop;
 
@@ -2387,6 +2405,8 @@ pipex_mppe_output(struct mbuf *m0, struct pipex_session *session,
        flushed = 0;
        encrypt = 1;
 
+       mtx_enter(&mppe->pxm_mtx);
+
        if (mppe->stateless != 0) {
                flushed = 1;
                mppe_key_change(mppe);
@@ -2429,6 +2449,8 @@ pipex_mppe_output(struct mbuf *m0, struct pipex_session *session,
                pipex_mppe_crypt(mppe, len, cp, cp);
        }
 
+       mtx_leave(&mppe->pxm_mtx);
+
        pipex_ppp_output(m0, session, PPP_COMP);
 
        return;
@@ -2455,7 +2477,9 @@ pipex_ccp_input(struct mbuf *m0, struct pipex_session *session)
        switch (code) {
        case CCP_RESETREQ:
                PIPEX_DBG((session, LOG_DEBUG, "CCP RecvResetReq"));
+               mtx_enter(&session->mppe_send.pxm_mtx);
                session->mppe_send.resetreq = 1;
+               mtx_leave(&session->mppe_send.pxm_mtx);
 #ifndef PIPEX_NO_CCP_RESETACK
                PIPEX_DBG((session, LOG_DEBUG, "CCP SendResetAck"));
                pipex_ccp_output(session, CCP_RESETACK, id);

diff --git a/sys/net/pipex_local.h b/sys/net/pipex_local.h
index 1727cd7..dd940b6 100644 (file)
--- a/sys/net/pipex_local.h
+++ b/sys/net/pipex_local.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pipex_local.h,v 1.42 2021/07/20 16:44:55 mvs Exp $    */
+/*     $OpenBSD: pipex_local.h,v 1.43 2021/07/27 09:29:09 mvs Exp $    */
 
 /*
  * Copyright (c) 2009 Internet Initiative Japan Inc.
@@ -57,22 +57,25 @@
  * Locks used to protect struct members:
  *      I       immutable after creation
  *      N       net lock
+ *      s       this pipex_session' `pxs_mtx'
+ *      m       this pipex_mppe' `pxm_mtx'
  */
 
 #ifdef PIPEX_MPPE
 /* mppe rc4 key */
 struct pipex_mppe {
+       struct mutex pxm_mtx;
        int16_t stateless:1,                    /* [I] key change mode */
-               resetreq:1,                     /* [N] */
+               resetreq:1,                     /* [m] */
                reserved:14;
        int16_t keylenbits;                     /* [I] key length */
        int16_t keylen;                         /* [I] */
-       uint16_t coher_cnt;                     /* [N] cohency counter */
-       struct  rc4_ctx rc4ctx;                 /* [N] */
-       u_char master_key[PIPEX_MPPE_KEYLEN];   /* [N] master key of MPPE */
-       u_char session_key[PIPEX_MPPE_KEYLEN];  /* [N] session key of MPPE */
+       uint16_t coher_cnt;                     /* [m] cohency counter */
+       struct  rc4_ctx rc4ctx;                 /* [m] */
+       u_char master_key[PIPEX_MPPE_KEYLEN];   /* [m] master key of MPPE */
+       u_char session_key[PIPEX_MPPE_KEYLEN];  /* [m] session key of MPPE */
        u_char (*old_session_keys)[PIPEX_MPPE_KEYLEN];
-                                               /* [N] old session keys */
+                                               /* [m] old session keys */
 };
 #endif /* PIPEX_MPPE */
 
@@ -156,6 +159,8 @@ struct pipex_session {
                                        /* [N] tree glue, and other values */
        struct radix_node       ps6_rn[2];
                                        /* [N] tree glue, and other values */
+       struct mutex pxs_mtx;
+
        LIST_ENTRY(pipex_session) session_list; /* [N] all session chain */
        LIST_ENTRY(pipex_session) state_list;   /* [N] state list chain */
        LIST_ENTRY(pipex_session) id_chain;     /* [N] id hash chain */
@@ -191,7 +196,7 @@ struct pipex_session {
 
        uint32_t        ppp_flags;              /* [I] configure flags */
 #ifdef PIPEX_MPPE
-       int ccp_id;                             /* [N] CCP packet id */
+       int ccp_id;                             /* [s] CCP packet id */
        struct pipex_mppe
            mppe_recv,                          /* MPPE context for incoming */
            mppe_send;                          /* MPPE context for outgoing */