Hi,

Only pppoe sessions are affected. Unfortunately, netlock is required
for pipex(4) pppoe output processing, and we can’t enforce netlock
state anymore for pppoe related (*if_qstart)(). We use netlock to
protect pipex(4) global list and the dereference of `session’
pointers. Now we can’t rely on netlock state while we perform 
(*if_qstart)() and the `session’ dereference is unsafe because it
could be killed by concurrent thread. The diff I posted to this thread
only fixes this problem. I’ll fix pppoe output with the future diffs.

> On 20 Jun 2022, at 21:47, Hrvoje Popovski <[email protected]> wrote:
> 
> On 20.6.2022. 16:46, Vitaliy Makkoveev wrote:
>> And use this mutex(9) to make the `session' dereference safe.
>> 
>> Hrvoje Popovski pointed me, he triggered netlock assertion with pipex(4)
>> pppoe sessions:
>> 
> 
> Hi,
> 
> I can trigger this splassert with plain snapshot and with only pppoe
> clients. npppd setup is without pf.
> 
> r420-1# ifconfig ix0
> ix0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> mtu 1500
>        lladdr 90:e2:ba:19:29:a8
>        index 1 priority 0 llprio 3
>        media: Ethernet autoselect (10GSFP+Cu full-duplex,rxpause,txpause)
>        status: active
>        inet 192.168.100.205 netmask 0xffffff00 broadcast 192.168.100.255
> 
> 
> 
> r420-1# cat /etc/npppd/npppd.conf
> authentication LOCAL type local {
>        users-file "/etc/npppd/npppd-users"
> }
> tunnel PPTP protocol pptp {
> }
> tunnel L2TP protocol l2tp {
>        authentication-method pap chap mschapv2
> }
> tunnel PPPOE protocol pppoe {
>        listen on interface ix0
>        authentication-method pap chap
> }
> 
> ipcp IPCP-L2TP {
>        pool-address 10.53.251.10-10.53.251.100
>        dns-servers 161.53.2.69
> }
> ipcp IPCP-PPTP {
>        pool-address 10.53.252.10-10.53.252.100
>        dns-servers 161.53.2.69
> }
> ipcp IPCP-PPPOE {
>        pool-address 10.53.253.10-10.53.253.100
> }
> 
> interface pppac0 address 10.53.251.1 ipcp IPCP-L2TP
> interface pppac1 address 10.53.252.1 ipcp IPCP-PPTP
> interface pppac2 address 10.53.253.1 ipcp IPCP-PPPOE
> 
> bind tunnel from L2TP authenticated by LOCAL to pppac0
> bind tunnel from PPTP authenticated by LOCAL to pppac1
> bind tunnel from PPPOE authenticated by LOCAL to pppac2
> 
> 
> r420-1# npppctl ses bri
> Ppp Id     Assigned IPv4   Username             Proto Tunnel From
> ---------- --------------- -------------------- -----
> -------------------------
>         0 10.53.253.11    hrvojepppoe11        PPPoE 90:e2:ba:33:af:ec
>         1 10.53.253.12    hrvojepppoe12        PPPoE ec:f4:bb:c8:e9:88
> 
> 
> I'm generating traffic with iperf3 from pppoe hosts to hosts in
> 192.168.100.0/24 network and at same time generating traffic between
> pppoe hosts.
> This triggers splassert after half a hour or so ...
> 
> r420-1# splassert: pipex_ppp_output: want 2 have 0
> Starting stack trace...
> pipex_ppp_output(fffffd80a45eaa00,ffff800022782400,21) at
> pipex_ppp_output+0x5e
> pppac_qstart(ffff800000ec0ab0) at pppac_qstart+0x96
> ifq_serialize(ffff800000ec0ab0,ffff800000ec0b90) at ifq_serialize+0xfd
> taskq_thread(ffff800000030080) at taskq_thread+0x100
> end trace frame: 0x0, count: 253
> End of stack trace.
> 
> 
> 
>>> 
>>> r420-1# splassert: pipex_ppp_output: want 2 have 0
>>> Starting stack trace...
>>> pipex_ppp_output(fffffd80cb7c9500,ffff800022761200,21) at
>>> pipex_ppp_output+0x5e
>>> pppac_qstart(ffff800000e80ab0) at pppac_qstart+0x96
>>> ifq_serialize(ffff800000e80ab0,ffff800000e80b90) at ifq_serialize+0xfd
>>> taskq_thread(ffff800000030080) at taskq_thread+0x100
>>> end trace frame: 0x0, count: 253
>>> End of stack trace.
>>> splassert: pipex_ppp_output: want 2 have 0
>>> Starting stack trace...
>>> pipex_ppp_output(fffffd80c53b2300,ffff800022761200,21) at
>>> pipex_ppp_output+0x5e
>>> pppac_qstart(ffff800000e80ab0) at pppac_qstart+0x96
>>> ifq_serialize(ffff800000e80ab0,ffff800000e80b90) at ifq_serialize+0xfd
>>> taskq_thread(ffff800000030080) at taskq_thread+0x100
>>> end trace frame: 0x0, count: 253
>>> End of stack trace.
>>> 
>>> 
>>> r420-1# npppctl ses bri
>>> Ppp Id     Assigned IPv4   Username             Proto Tunnel From
>>> ---------- --------------- -------------------- -----
>>> -------------------------
>>>         7 10.53.251.22    r6202                L2TP  192.168.100.12:1701
>>>         1 10.53.253.11    hrvojepppoe11        PPPoE 90:e2:ba:33:af:ec
>>>         0 10.53.253.12    hrvojepppoe12        PPPoE ec:f4:bb:da:f7:f8
>> 
>> This means the hack we use to enforce pppac_qstart() and
>> pppx_if_qstart() be called with netlock held doesn't work anymore for
>> pppoe sessions:
>> 
>>      /* XXXSMP: be sure pppac_qstart() called with NET_LOCK held */
>>      ifq_set_maxlen(&ifp->if_snd, 1);
>> 
>> pptp and l2tp sessions are not affected, because the pcb layer is still
>> accessed with exclusive netlock held, but the code is common for all
>> session types. This mean we can't rely on netlock and should rework
>> pipex(4) locking.
>> 
>> The diff below introduces `pipex_list_mtx' mutex(9) to protect global
>> pipex(4) lists and made `session' dereference safe. It doesn't fix the
>> assertion, but only makes us sure the pppoe session can't be killed
>> concurrently while stack processes it.
>> 
>> I'll protect pipex(4) session data and rework pipex(4) output with next
>> diffs.
>> 
>> ok?
>> 
>> Index: sys/net/if_ethersubr.c
>> ===================================================================
>> RCS file: /cvs/src/sys/net/if_ethersubr.c,v
>> retrieving revision 1.279
>> diff -u -p -r1.279 if_ethersubr.c
>> --- sys/net/if_ethersubr.c   22 Apr 2022 12:10:57 -0000      1.279
>> +++ sys/net/if_ethersubr.c   20 Jun 2022 14:44:03 -0000
>> @@ -545,11 +545,14 @@ ether_input(struct ifnet *ifp, struct mb
>>              if (pipex_enable) {
>>                      struct pipex_session *session;
>> 
>> +                    mtx_enter(&pipex_list_mtx);
>>                      if ((session = pipex_pppoe_lookup_session(m)) != NULL) {
>>                              pipex_pppoe_input(m, session);
>> +                            mtx_leave(&pipex_list_mtx);
>>                              KERNEL_UNLOCK();
>>                              return;
>>                      }
>> +                    mtx_leave(&pipex_list_mtx);
>>              }
>> #endif
>>              if (etype == ETHERTYPE_PPPOEDISC)
>> Index: sys/net/if_gre.c
>> ===================================================================
>> RCS file: /cvs/src/sys/net/if_gre.c,v
>> retrieving revision 1.171
>> diff -u -p -r1.171 if_gre.c
>> --- sys/net/if_gre.c 10 Mar 2021 10:21:47 -0000      1.171
>> +++ sys/net/if_gre.c 20 Jun 2022 14:44:03 -0000
>> @@ -973,10 +973,18 @@ gre_input_1(struct gre_tunnel *key, stru
>>              if (pipex_enable) {
>>                      struct pipex_session *session;
>> 
>> +                    mtx_enter(&pipex_list_mtx);
>>                      session = pipex_pptp_lookup_session(m);
>> -                    if (session != NULL &&
>> -                        pipex_pptp_input(m, session) == NULL)
>> -                            return (NULL);
>> +                    if (session != NULL) {
>> +                            struct mbuf *m0;
>> +
>> +                            m0 = pipex_pptp_input(m, session);
>> +                            mtx_leave(&pipex_list_mtx);
>> +
>> +                            if (m0 == NULL)
>> +                                    return (NULL);
>> +                    }
>> +                    mtx_leave(&pipex_list_mtx);
>>              }
>> #endif
>>              break;
>> Index: sys/net/if_pppx.c
>> ===================================================================
>> RCS file: /cvs/src/sys/net/if_pppx.c,v
>> retrieving revision 1.114
>> diff -u -p -r1.114 if_pppx.c
>> --- sys/net/if_pppx.c        22 Feb 2022 01:15:02 -0000      1.114
>> +++ sys/net/if_pppx.c        20 Jun 2022 14:44:03 -0000
>> @@ -1322,9 +1322,7 @@ pppacclose(dev_t dev, int flags, int mod
>>      splx(s);
>> 
>>      pool_put(&pipex_session_pool, sc->sc_multicast_session);
>> -    NET_LOCK();
>>      pipex_destroy_all_sessions(sc);
>> -    NET_UNLOCK();
>> 
>>      LIST_REMOVE(sc, sc_entry);
>>      free(sc, M_DEVBUF, sizeof(*sc));
>> @@ -1384,11 +1382,15 @@ pppac_del_session(struct pppac_softc *sc
>> {
>>      struct pipex_session *session;
>> 
>> +    mtx_enter(&pipex_list_mtx);
>>      session = pipex_lookup_by_session_id(req->pcr_protocol,
>>          req->pcr_session_id);
>> -    if (session == NULL || session->ownersc != sc)
>> +    if (session == NULL || session->ownersc != sc) {
>> +            mtx_leave(&pipex_list_mtx);
>>              return (EINVAL);
>> -    pipex_unlink_session(session);
>> +    }
>> +    pipex_unlink_session_locked(session);
>> +    mtx_leave(&pipex_list_mtx);
>>      pipex_rele_session(session);
>> 
>>      return (0);
>> @@ -1458,11 +1460,13 @@ pppac_qstart(struct ifqueue *ifq)
>>                              else
>>                                      goto bad;
>>                      } else {
>> +                            mtx_enter(&pipex_list_mtx);
>>                              session = pipex_lookup_by_ip_address(ip.ip_dst);
>>                              if (session != NULL) {
>>                                      pipex_ip_output(m, session);
>>                                      m = NULL;
>>                              }
>> +                            mtx_leave(&pipex_list_mtx);
>>                      }
>>                      break;
>>              }
>> Index: sys/net/pipex.c
>> ===================================================================
>> RCS file: /cvs/src/sys/net/pipex.c,v
>> retrieving revision 1.136
>> diff -u -p -r1.136 pipex.c
>> --- sys/net/pipex.c  2 Jan 2022 22:36:04 -0000       1.136
>> +++ sys/net/pipex.c  20 Jun 2022 14:44:03 -0000
>> @@ -40,6 +40,7 @@
>> #include <sys/kernel.h>
>> #include <sys/pool.h>
>> #include <sys/percpu.h>
>> +#include <sys/mutex.h>
>> 
>> #include <net/if.h>
>> #include <net/if_types.h>
>> @@ -79,6 +80,8 @@
>> #include <net/pipex.h>
>> #include "pipex_local.h"
>> 
>> +struct mutex pipex_list_mtx = MUTEX_INITIALIZER(IPL_SOFTNET);
>> +
>> struct pool pipex_session_pool;
>> struct pool mppe_key_pool;
>> 
>> @@ -88,17 +91,18 @@ struct pool mppe_key_pool;
>>  *       A       atomic operation
>>  *       I       immutable after creation
>>  *       N       net lock
>> + *       L       pipex_list_mtx
>>  */
>> 
>> int  pipex_enable = 0;                       /* [N] */
>> struct pipex_hash_head
>> -    pipex_session_list,                             /* [N] master session 
>> list */
>> -    pipex_close_wait_list,                  /* [N] expired session list */
>> -    pipex_peer_addr_hashtable[PIPEX_HASH_SIZE],     /* [N] peer's address 
>> hash */
>> -    pipex_id_hashtable[PIPEX_HASH_SIZE];    /* [N] peer id hash */
>> +    pipex_session_list,                             /* [L] master session 
>> list */
>> +    pipex_close_wait_list,                  /* [L] expired session list */
>> +    pipex_peer_addr_hashtable[PIPEX_HASH_SIZE],     /* [L] peer's address 
>> hash */
>> +    pipex_id_hashtable[PIPEX_HASH_SIZE];    /* [L] peer id hash */
>> 
>> -struct radix_node_head      *pipex_rd_head4 = NULL; /* [N] */
>> -struct radix_node_head      *pipex_rd_head6 = NULL; /* [N] */
>> +struct radix_node_head      *pipex_rd_head4 = NULL; /* [L] */
>> +struct radix_node_head      *pipex_rd_head6 = NULL; /* [L] */
>> struct timeout pipex_timer_ch;               /* callout timer context */
>> int pipex_prune = 1;                 /* [I] walk list every seconds */
>> 
>> @@ -145,16 +149,18 @@ pipex_destroy_all_sessions(void *ownersc
>> {
>>      struct pipex_session *session, *session_tmp;
>> 
>> -    NET_ASSERT_LOCKED();
>> +    mtx_enter(&pipex_list_mtx);
>> 
>>      LIST_FOREACH_SAFE(session, &pipex_session_list, session_list,
>>          session_tmp) {
>>              if (session->ownersc == ownersc) {
>>                      KASSERT(session->is_pppx == 0);
>> -                    pipex_unlink_session(session);
>> +                    pipex_unlink_session_locked(session);
>>                      pipex_rele_session(session);
>>              }
>>      }
>> +
>> +    mtx_leave(&pipex_list_mtx);
>> }
>> 
>> int
>> @@ -375,8 +381,9 @@ pipex_link_session(struct pipex_session 
>> {
>>      struct pipex_hash_head *chain;
>>      struct radix_node *rn;
>> +    int error = 0;
>> 
>> -    NET_ASSERT_LOCKED();
>> +    mtx_enter(&pipex_list_mtx);
>> 
>>      if (pipex_rd_head4 == NULL) {
>>              if (!rn_inithead((void **)&pipex_rd_head4,
>> @@ -389,8 +396,10 @@ pipex_link_session(struct pipex_session 
>>                      panic("rn_inithead() failed on pipex_link_session()");
>>      }
>>      if (pipex_lookup_by_session_id(session->protocol,
>> -        session->session_id))
>> -            return (EEXIST);
>> +        session->session_id)) {
>> +            error = EEXIST;
>> +            goto out;
>> +    }
>> 
>>      session->ownersc = ownersc;
>>      session->ifindex = ifp->if_index;
>> @@ -400,12 +409,16 @@ pipex_link_session(struct pipex_session 
>>      if (session->is_pppx == 0 &&
>>          !in_nullhost(session->ip_address.sin_addr)) {
>>              if (pipex_lookup_by_ip_address(session->ip_address.sin_addr)
>> -                != NULL)
>> -                    return (EADDRINUSE);
>> +                != NULL) {
>> +                    error = EADDRINUSE;
>> +                    goto out;
>> +            }
>>              rn = rn_addroute(&session->ip_address, &session->ip_netmask,
>>                  pipex_rd_head4, session->ps4_rn, RTP_STATIC);
>> -            if (rn == NULL)
>> -                    return (ENOMEM);
>> +            if (rn == NULL) {
>> +                    error = ENOMEM;
>> +                    goto out;
>> +            }
>>      }
>> 
>>      LIST_INSERT_HEAD(&pipex_session_list, session, session_list);
>> @@ -426,17 +439,21 @@ pipex_link_session(struct pipex_session 
>>              pipex_timer_start();
>>      session->state = PIPEX_STATE_OPENED;
>> 
>> -    return (0);
>> +out:
>> +    mtx_leave(&pipex_list_mtx);
>> +
>> +    return error;
>> }
>> 
>> void
>> -pipex_unlink_session(struct pipex_session *session)
>> +pipex_unlink_session_locked(struct pipex_session *session)
>> {
>>      struct radix_node *rn;
>> 
>> +    MUTEX_ASSERT_LOCKED(&pipex_list_mtx);
>> +
>>      session->ifindex = 0;
>> 
>> -    NET_ASSERT_LOCKED();
>>      if (session->state == PIPEX_STATE_CLOSED)
>>              return;
>>      if (session->is_pppx == 0 &&
>> @@ -466,10 +483,19 @@ pipex_unlink_session(struct pipex_sessio
>>              pipex_timer_stop();
>> }
>> 
>> +void
>> +pipex_unlink_session(struct pipex_session *session)
>> +{
>> +    mtx_enter(&pipex_list_mtx);
>> +    pipex_unlink_session_locked(session);
>> +    mtx_leave(&pipex_list_mtx);
>> +}
>> +
>> int
>> pipex_notify_close_session(struct pipex_session *session)
>> {
>> -    NET_ASSERT_LOCKED();
>> +    MUTEX_ASSERT_LOCKED(&pipex_list_mtx);
>> +
>>      session->state = PIPEX_STATE_CLOSE_WAIT;
>>      session->idle_time = 0;
>>      LIST_INSERT_HEAD(&pipex_close_wait_list, session, state_list);
>> @@ -499,34 +525,54 @@ Static int
>> pipex_config_session(struct pipex_session_config_req *req, void *ownersc)
>> {
>>      struct pipex_session *session;
>> +    int error = 0;
>> 
>>      NET_ASSERT_LOCKED();
>> +    mtx_enter(&pipex_list_mtx);
>> +
>>      session = pipex_lookup_by_session_id(req->pcr_protocol,
>>          req->pcr_session_id);
>> -    if (session == NULL)
>> -            return (EINVAL);
>> -    if (session->ownersc != ownersc)
>> -            return (EINVAL);
>> +    if (session == NULL) {
>> +            error = EINVAL;
>> +            goto out;
>> +    }
>> +    if (session->ownersc != ownersc) {
>> +            error = EINVAL;
>> +            goto out;
>> +    }
>>      session->ip_forward = req->pcr_ip_forward;
>> 
>> -    return (0);
>> +out:
>> +    mtx_leave(&pipex_list_mtx);
>> +
>> +    return error;
>> }
>> 
>> Static int
>> pipex_get_stat(struct pipex_session_stat_req *req, void *ownersc)
>> {
>>      struct pipex_session *session;
>> +    int error = 0;
>> 
>>      NET_ASSERT_LOCKED();
>> +    mtx_enter(&pipex_list_mtx);
>> +
>>      session = pipex_lookup_by_session_id(req->psr_protocol,
>>          req->psr_session_id);
>> -    if (session == NULL)
>> -            return (EINVAL);
>> -    if (session->ownersc != ownersc)
>> -            return (EINVAL);
>> +    if (session == NULL) {
>> +            error = EINVAL;
>> +            goto out;
>> +    }
>> +    if (session->ownersc != ownersc) {
>> +            error = EINVAL;
>> +            goto out;
>> +    }
>>      pipex_export_session_stats(session, &req->psr_stat);
>> 
>> -    return (0);
>> +out:
>> +    mtx_leave(&pipex_list_mtx);
>> +
>> +    return error;
>> }
>> 
>> Static int
>> @@ -534,8 +580,10 @@ pipex_get_closed(struct pipex_session_li
>> {
>>      struct pipex_session *session, *session_tmp;
>> 
>> -    NET_ASSERT_LOCKED();
>>      bzero(req, sizeof(*req));
>> +
>> +    mtx_enter(&pipex_list_mtx);
>> +
>>      LIST_FOREACH_SAFE(session, &pipex_close_wait_list, state_list,
>>          session_tmp) {
>>              if (session->ownersc != ownersc)
>> @@ -550,6 +598,8 @@ pipex_get_closed(struct pipex_session_li
>>              }
>>      }
>> 
>> +    mtx_leave(&pipex_list_mtx);
>> +
>>      return (0);
>> }
>> 
>> @@ -559,6 +609,8 @@ pipex_lookup_by_ip_address(struct in_add
>>      struct pipex_session *session;
>>      struct sockaddr_in pipex_in4, pipex_in4mask;
>> 
>> +    MUTEX_ASSERT_LOCKED(&pipex_list_mtx);
>> +
>>      if (pipex_rd_head4 == NULL)
>>              return (NULL);
>>      bzero(&pipex_in4, sizeof(pipex_in4));
>> @@ -592,7 +644,8 @@ pipex_lookup_by_session_id(int protocol,
>>      struct pipex_hash_head *list;
>>      struct pipex_session *session;
>> 
>> -    NET_ASSERT_LOCKED();
>> +    MUTEX_ASSERT_LOCKED(&pipex_list_mtx);
>> +
>>      list = PIPEX_ID_HASHTABLE(session_id);
>>      LIST_FOREACH(session, list, id_chain) {
>>              if (session->protocol == protocol &&
>> @@ -633,7 +686,7 @@ pipex_timer(void *ignored_arg)
>> 
>>      timeout_add_sec(&pipex_timer_ch, pipex_prune);
>> 
>> -    NET_LOCK();
>> +    mtx_enter(&pipex_list_mtx);
>>      /* walk through */
>>      LIST_FOREACH_SAFE(session, &pipex_session_list, session_list,
>>          session_tmp) {
>> @@ -656,7 +709,7 @@ pipex_timer(void *ignored_arg)
>>                      if (session->idle_time < PIPEX_CLOSE_TIMEOUT)
>>                              continue;
>>                      /* Release the sessions when timeout */
>> -                    pipex_unlink_session(session);
>> +                    pipex_unlink_session_locked(session);
>>                      KASSERTMSG(session->is_pppx == 0,
>>                          "FIXME session must not be released when pppx");
>>                      pipex_rele_session(session);
>> @@ -667,7 +720,7 @@ pipex_timer(void *ignored_arg)
>>              }
>>      }
>> 
>> -    NET_UNLOCK();
>> +    mtx_leave(&pipex_list_mtx);
>> }
>> 
>> /***********************************************************************
>> @@ -1113,6 +1166,8 @@ pipex_pppoe_lookup_session(struct mbuf *
>>      struct pipex_session *session;
>>      struct pipex_pppoe_header pppoe;
>> 
>> +    MUTEX_ASSERT_LOCKED(&pipex_list_mtx);
>> +
>>      /* short packet */
>>      if (m0->m_pkthdr.len < (sizeof(struct ether_header) + sizeof(pppoe)))
>>              return (NULL);
>> @@ -1139,7 +1194,8 @@ pipex_pppoe_input(struct mbuf *m0, struc
>>      int hlen;
>>      struct pipex_pppoe_header pppoe;
>> 
>> -    NET_ASSERT_LOCKED();
>> +    MUTEX_ASSERT_LOCKED(&pipex_list_mtx);
>> +
>>      /* already checked at pipex_pppoe_lookup_session */
>>      KASSERT(m0->m_pkthdr.len >= (sizeof(struct ether_header) +
>>          sizeof(pppoe)));
>> @@ -1302,6 +1358,8 @@ pipex_pptp_lookup_session(struct mbuf *m
>>      uint16_t id;
>>      int hlen;
>> 
>> +    MUTEX_ASSERT_LOCKED(&pipex_list_mtx);
>> +
>>      if (m0->m_pkthdr.len < PIPEX_IPGRE_HDRLEN) {
>>              PIPEX_DBG((NULL, LOG_DEBUG,
>>                  "<%s> packet length is too short", __func__));
>> @@ -1372,6 +1430,8 @@ pipex_pptp_input(struct mbuf *m0, struct
>>      int rewind = 0;
>> 
>>      NET_ASSERT_LOCKED();
>> +    MUTEX_ASSERT_LOCKED(&pipex_list_mtx);
>> +
>>      KASSERT(m0->m_pkthdr.len >= PIPEX_IPGRE_HDRLEN);
>>      pptp_session = &session->proto.pptp;
>> 
>> @@ -1535,6 +1595,8 @@ pipex_pptp_userland_lookup_session(struc
>>      struct pipex_session *session;
>>      uint16_t id, flags;
>> 
>> +    MUTEX_ASSERT_LOCKED(&pipex_list_mtx);
>> +
>>      /* pullup */
>>      if (m0->m_pkthdr.len < sizeof(gre)) {
>>              PIPEX_DBG((NULL, LOG_DEBUG,
>> @@ -1592,6 +1654,8 @@ pipex_pptp_userland_output(struct mbuf *
>>      u_char *cp, *cp0;
>>      uint32_t val32;
>> 
>> +    MUTEX_ASSERT_LOCKED(&pipex_list_mtx);
>> +
>>      len = sizeof(struct pipex_gre_header);
>>      m_copydata(m0, 0, len, &gre0);
>>      gre = &gre0;
>> @@ -1776,6 +1840,8 @@ pipex_l2tp_lookup_session(struct mbuf *m
>>      uint16_t flags, session_id, ver;
>>      u_char *cp, buf[PIPEX_L2TP_MINLEN];
>> 
>> +    MUTEX_ASSERT_LOCKED(&pipex_list_mtx);
>> +
>>      if (m0->m_pkthdr.len < off + PIPEX_L2TP_MINLEN) {
>>              PIPEX_DBG((NULL, LOG_DEBUG,
>>                  "<%s> packet length is too short", __func__));
>> @@ -1829,6 +1895,8 @@ pipex_l2tp_input(struct mbuf *m0, int of
>>      int rewind = 0;
>> 
>>      NET_ASSERT_LOCKED();
>> +    MUTEX_ASSERT_LOCKED(&pipex_list_mtx);
>> +
>>      length = offset = ns = nr = 0;
>>      l2tp_session = &session->proto.l2tp;
>>      l2tp_session->ipsecflowinfo = ipsecflowinfo;
>> @@ -1975,6 +2043,8 @@ pipex_l2tp_userland_lookup_session(struc
>>      struct pipex_session *session;
>>      uint16_t session_id, tunnel_id, flags;
>> 
>> +    MUTEX_ASSERT_LOCKED(&pipex_list_mtx);
>> +
>>      if (sa->sa_family != AF_INET && sa->sa_family != AF_INET6)
>>              return (NULL);
>> 
>> @@ -2031,6 +2101,8 @@ pipex_l2tp_userland_output(struct mbuf *
>>      struct pipex_l2tp_header *l2tp;
>>      struct pipex_l2tp_seq_header *seq;
>>      uint16_t ns, nr;
>> +
>> +    MUTEX_ASSERT_LOCKED(&pipex_list_mtx);
>> 
>>      /* check length */
>>      PIPEX_PULLUP(m0, sizeof(struct pipex_l2tp_header) +
>> Index: sys/net/pipex.h
>> ===================================================================
>> RCS file: /cvs/src/sys/net/pipex.h,v
>> retrieving revision 1.31
>> diff -u -p -r1.31 pipex.h
>> --- sys/net/pipex.h  2 Jan 2022 22:36:04 -0000       1.31
>> +++ sys/net/pipex.h  20 Jun 2022 14:44:03 -0000
>> @@ -29,6 +29,8 @@
>> #ifndef NET_PIPEX_H
>> #define NET_PIPEX_H 1
>> 
>> +#include <sys/mutex.h>
>> +
>> /*
>>  * Names for pipex sysctl objects
>>  */
>> @@ -174,6 +176,7 @@ struct pipex_session_descr_req {
>> 
>> #ifdef _KERNEL
>> extern int   pipex_enable;
>> +extern struct mutex pipex_list_mtx;
>> 
>> struct pipex_session;
>> 
>> Index: sys/net/pipex_local.h
>> ===================================================================
>> RCS file: /cvs/src/sys/net/pipex_local.h,v
>> retrieving revision 1.45
>> diff -u -p -r1.45 pipex_local.h
>> --- sys/net/pipex_local.h    15 Feb 2022 03:31:17 -0000      1.45
>> +++ sys/net/pipex_local.h    20 Jun 2022 14:44:03 -0000
>> @@ -26,6 +26,8 @@
>>  * SUCH DAMAGE.
>>  */
>> 
>> +#include <sys/mutex.h>
>> +
>> #define      PIPEX_PPTP      1
>> #define      PIPEX_L2TP      1
>> #define      PIPEX_PPPOE     1
>> @@ -53,10 +55,13 @@
>>  * Locks used to protect struct members:
>>  *      I       immutable after creation
>>  *      N       net lock
>> + *      L       pipex_list_mtx
>>  *      s       this pipex_session' `pxs_mtx'
>>  *      m       this pipex_mppe' `pxm_mtx'
>>  */
>> 
>> +extern struct mutex pipex_list_mtx;
>> +
>> #ifdef PIPEX_MPPE
>> /* mppe rc4 key */
>> struct pipex_mppe {
>> @@ -152,24 +157,24 @@ struct cpumem;
>> /* pppac ip-extension session table */
>> struct pipex_session {
>>      struct radix_node       ps4_rn[2];
>> -                                    /* [N] tree glue, and other values */
>> +                                    /* [L] tree glue, and other values */
>>      struct radix_node       ps6_rn[2];
>> -                                    /* [N] tree glue, and other values */
>> +                                    /* [L] 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 */
>> +    LIST_ENTRY(pipex_session) session_list; /* [L] all session chain */
>> +    LIST_ENTRY(pipex_session) state_list;   /* [L] state list chain */
>> +    LIST_ENTRY(pipex_session) id_chain;     /* [L] id hash chain */
>>      LIST_ENTRY(pipex_session) peer_addr_chain;
>> -                                    /* [N] peer's address hash chain */
>> -    uint16_t        state;          /* [N] pipex session state */
>> +                                    /* [L] peer's address hash chain */
>> +    uint16_t        state;          /* [L] pipex session state */
>> #define PIPEX_STATE_INITIAL          0x0000
>> #define PIPEX_STATE_OPENED           0x0001
>> #define PIPEX_STATE_CLOSE_WAIT               0x0002
>> #define PIPEX_STATE_CLOSE_WAIT2              0x0003
>> #define PIPEX_STATE_CLOSED           0x0004
>> 
>> -    uint32_t        idle_time;      /* [N] idle time in seconds */
>> +    uint32_t        idle_time;      /* [L] idle time in seconds */
>>      uint16_t        ip_forward:1,   /* [N] {en|dis}ableIP forwarding */
>>                      ip6_forward:1,  /* [I] {en|dis}able IPv6 forwarding */
>>                      is_multicast:1, /* [I] virtual entry for multicast */
>> @@ -400,6 +405,7 @@ void                  pipex_rele_session
>> int                   pipex_link_session(struct pipex_session *,
>>                           struct ifnet *, void *);
>> void                  pipex_unlink_session(struct pipex_session *);
>> +void                  pipex_unlink_session_locked(struct pipex_session *);
>> void                  pipex_export_session_stats(struct pipex_session *,
>>                           struct pipex_statistics *);
>> int                   pipex_config_session (struct pipex_session_config_req 
>> *,
>> Index: sys/netinet/ip_gre.c
>> ===================================================================
>> RCS file: /cvs/src/sys/netinet/ip_gre.c,v
>> retrieving revision 1.73
>> diff -u -p -r1.73 ip_gre.c
>> --- sys/netinet/ip_gre.c     25 Feb 2022 23:51:03 -0000      1.73
>> +++ sys/netinet/ip_gre.c     20 Jun 2022 14:44:03 -0000
>> @@ -70,7 +70,6 @@ gre_usrreq(struct socket *so, int req, s
>>      if (inp != NULL && inp->inp_pipex && req == PRU_SEND) {
>>              struct sockaddr_in *sin4;
>>              struct in_addr *ina_dst;
>> -            struct pipex_session *session;
>> 
>>              ina_dst = NULL;
>>              if ((so->so_state & SS_ISCONNECTED) != 0) {
>> @@ -81,10 +80,18 @@ gre_usrreq(struct socket *so, int req, s
>>                      if (in_nam2sin(nam, &sin4) == 0)
>>                              ina_dst = &sin4->sin_addr;
>>              }
>> -            if (ina_dst != NULL &&
>> -                (session = pipex_pptp_userland_lookup_session_ipv4(m,
>> -                        *ina_dst)))
>> -                    m = pipex_pptp_userland_output(m, session);
>> +            if (ina_dst != NULL) {
>> +                    struct pipex_session *session;
>> +
>> +                    mtx_enter(&pipex_list_mtx);
>> +                    session = pipex_pptp_userland_lookup_session_ipv4(m,
>> +                        *ina_dst);
>> +
>> +                    if(session != NULL)
>> +                            m = pipex_pptp_userland_output(m, session);
>> +
>> +                    mtx_leave(&pipex_list_mtx);
>> +            }
>> 
>>              if (m == NULL)
>>                      return (ENOMEM);
>> Index: sys/netinet/udp_usrreq.c
>> ===================================================================
>> RCS file: /cvs/src/sys/netinet/udp_usrreq.c,v
>> retrieving revision 1.278
>> diff -u -p -r1.278 udp_usrreq.c
>> --- sys/netinet/udp_usrreq.c 15 May 2022 09:12:20 -0000      1.278
>> +++ sys/netinet/udp_usrreq.c 20 Jun 2022 14:44:03 -0000
>> @@ -565,13 +565,17 @@ udp_input(struct mbuf **mp, int *offp, i
>>      if (pipex_enable && inp->inp_pipex) {
>>              struct pipex_session *session;
>>              int off = iphlen + sizeof(struct udphdr);
>> +
>> +            mtx_enter(&pipex_list_mtx);
>>              if ((session = pipex_l2tp_lookup_session(m, off)) != NULL) {
>>                      if ((m = *mp = pipex_l2tp_input(m, off, session,
>>                          ipsecflowinfo)) == NULL) {
>>                              /* the packet is handled by PIPEX */
>> +                            mtx_leave(&pipex_list_mtx);
>>                              return IPPROTO_DONE;
>>                      }
>>              }
>> +            mtx_leave(&pipex_list_mtx);
>>      }
>> #endif
>> 
>> @@ -1135,6 +1139,8 @@ udp_usrreq(struct socket *so, int req, s
>>              if (inp->inp_pipex) {
>>                      struct pipex_session *session;
>> 
>> +                    mtx_enter(&pipex_list_mtx);
>> +
>>                      if (addr != NULL)
>>                              session =
>>                                  pipex_l2tp_userland_lookup_session(m,
>> @@ -1153,9 +1159,12 @@ udp_usrreq(struct socket *so, int req, s
>>                      if (session != NULL)
>>                              if ((m = pipex_l2tp_userland_output(
>>                                  m, session)) == NULL) {
>> +                                    mtx_leave(&pipex_list_mtx);
>>                                      error = ENOMEM;
>>                                      goto release;
>>                              }
>> +
>> +                    mtx_leave(&pipex_list_mtx);
>>              }
>> #endif
>> 
>> 
> 

Reply via email to