While `mbuf' enqueued to `pipexinq' or `pipexoutq' it has the reference
to corresponding pipex(4) session as `ph_cookie'. `ph_cookie' is
accessed by pipexintr() and it's always defferent context from context
where we destroy session. `ph_cookie' is protected only while we destroy
session by pipex_timer() but this protection is not effective. While we
destroy session related to pppx(4) `ph_cookie' is not potected. While we
destroy session related to pppac(4) by `PIPEXSMODE' ioctl() or by
closing pppac(4) device node `ph_cookie' is also not protected.

I'am going to use reference counters to protect `ph_cookie' but some
additional steps required to be done.

We have `pipex_iface' which holds the reference to external `ifnet'. The
pipex(4) session also has reference to this `ifnet'. With reference
counters pipex(4) session can still be in `pipexinq' or `pipexoutq' but
corresponding `pppx_if' or `pppac_softc' is already destroyed. I want to
be shure while we do if_detach() no one has reference to this `ifnet'.

Diff below grabs reference to `ifnet' each time it passed to pipex(4).
Also while we release this session we release the reference.

We attach `ifnet' before we have linked sessions and we detach `ifnet'
after we release all sessions. Now it's safe.

if_ref() was declared in sys/if.c so I moved if_ref() declaration to
sys/if.h.

Index: sys/net/if.c
===================================================================
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.610
diff -u -p -r1.610 if.c
--- sys/net/if.c        22 Jun 2020 09:45:13 -0000      1.610
+++ sys/net/if.c        24 Jun 2020 13:43:33 -0000
@@ -192,7 +192,6 @@ void        if_qstart_compat(struct ifqueue *);
 
 void if_ifp_dtor(void *, void *);
 void if_map_dtor(void *, void *);
-struct ifnet *if_ref(struct ifnet *);
 
 /*
  * struct if_map
Index: sys/net/if.h
===================================================================
RCS file: /cvs/src/sys/net/if.h,v
retrieving revision 1.203
diff -u -p -r1.203 if.h
--- sys/net/if.h        25 Jul 2019 15:23:39 -0000      1.203
+++ sys/net/if.h        24 Jun 2020 13:43:33 -0000
@@ -548,6 +548,7 @@ int if_delgroup(struct ifnet *, const ch
 void   if_group_routechange(struct sockaddr *, struct sockaddr *);
 struct ifnet *ifunit(const char *);
 struct ifnet *if_get(unsigned int);
+struct ifnet *if_ref(struct ifnet *);
 void   if_put(struct ifnet *);
 void   ifnewlladdr(struct ifnet *);
 void   if_congestion(void);
Index: sys/net/if_pppx.c
===================================================================
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.90
diff -u -p -r1.90 if_pppx.c
--- sys/net/if_pppx.c   24 Jun 2020 08:52:53 -0000      1.90
+++ sys/net/if_pppx.c   24 Jun 2020 13:43:33 -0000
@@ -714,15 +714,15 @@ pppx_add_session(struct pppx_dev *pxd, s
        ifp->if_softc = pxi;
        /* ifp->if_rdomain = req->pr_rdomain; */
 
-       error = pipex_link_session(session, &pxi->pxi_ifcontext);
-       if (error)
-               goto remove;
-
        /* XXXSMP breaks atomicity */
        NET_UNLOCK();
        if_attach(ifp);
        NET_LOCK();
 
+       error = pipex_link_session(session, &pxi->pxi_ifcontext);
+       if (error)
+               goto detach;
+
        if_addgroup(ifp, "pppx");
        if_alloc_sadl(ifp);
 
@@ -771,7 +771,12 @@ pppx_add_session(struct pppx_dev *pxd, s
 
        return (error);
 
-remove:
+detach:
+       /* XXXSMP breaks atomicity */
+       NET_UNLOCK();
+       if_detach(ifp);
+       NET_LOCK();
+
        if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
                panic("%s: inconsistent RB tree", __func__);
        LIST_REMOVE(pxi, pxi_list);
@@ -860,13 +865,13 @@ pppx_if_destroy(struct pppx_dev *pxd, st
        CLR(ifp->if_flags, IFF_RUNNING);
 
        pipex_unlink_session(session);
+       pipex_rele_session(session);
 
        /* XXXSMP breaks atomicity */
        NET_UNLOCK();
        if_detach(ifp);
        NET_LOCK();
 
-       pipex_rele_session(session);
        if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
                panic("%s: inconsistent RB tree", __func__);
        LIST_REMOVE(pxi, pxi_list);
Index: sys/net/pipex.c
===================================================================
RCS file: /cvs/src/sys/net/pipex.c,v
retrieving revision 1.116
diff -u -p -r1.116 pipex.c
--- sys/net/pipex.c     22 Jun 2020 09:38:15 -0000      1.116
+++ sys/net/pipex.c     24 Jun 2020 13:43:34 -0000
@@ -147,6 +147,7 @@ pipex_iface_init(struct pipex_iface_cont
 {
        struct pipex_session *session;
 
+       if_ref(ifp);
        pipex_iface->pipexmode = 0;
        pipex_iface->ifnet_this = ifp;
 
@@ -164,6 +165,7 @@ pipex_iface_init(struct pipex_iface_cont
        /* virtual pipex_session entry for multicast */
        session = pool_get(&pipex_session_pool, PR_WAITOK | PR_ZERO);
        session->is_multicast = 1;
+       if_ref(pipex_iface->ifnet_this);
        session->pipex_iface = pipex_iface;
        pipex_iface->multicast_session = session;
 }
@@ -194,10 +196,11 @@ pipex_iface_stop(struct pipex_iface_cont
 void
 pipex_iface_fini(struct pipex_iface_context *pipex_iface)
 {
-       pool_put(&pipex_session_pool, pipex_iface->multicast_session);
        NET_LOCK();
        pipex_iface_stop(pipex_iface);
        NET_UNLOCK();
+       pipex_rele_session(pipex_iface->multicast_session);
+       if_put(pipex_iface->ifnet_this);
 }
 
 int
@@ -421,6 +424,9 @@ pipex_init_session(struct pipex_session 
 void
 pipex_rele_session(struct pipex_session *session)
 {
+       if (session->pipex_iface) {
+               if_put(session->pipex_iface->ifnet_this);
+       }
        if (session->mppe_recv.old_session_keys)
                pool_put(&mppe_key_pool, session->mppe_recv.old_session_keys);
        pool_put(&pipex_session_pool, session);
@@ -438,6 +444,7 @@ pipex_link_session(struct pipex_session 
            session->session_id))
                return (EEXIST);
 
+       if_ref(iface->ifnet_this);
        session->pipex_iface = iface;
 
        LIST_INSERT_HEAD(&pipex_session_list, session, session_list);
@@ -655,7 +662,7 @@ pipex_destroy_session(struct pipex_sessi
        }
 
        pipex_unlink_session(session);
-       pool_put(&pipex_session_pool, session);
+       pipex_rele_session(session);
 
        return (0);
 }

Reply via email to