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