From: mvs Date: Tue, 27 Jul 2021 09:29:09 +0000 (+0000) Subject: Introduce mutex(9) to protect pipex(4) session content. X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=2960d3c857274aa9ebcec7077ec6d85bc95c7a4f;p=openbsd 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 c08261f81fb..e2d0c5c958d 100644 --- 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 1727cd74ce4..dd940b6a74a 100644 --- 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 */