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); }