Each pipex_session has timeout_sec field and if it is not 0,
pipex_timer() can destroy pipex_session. Each pppx_if contents
pipex_session. If userland creates pppx_if and pipex_session has
timeout (for example, npppd.conf has idle-timeout option), pipex_timer()
can destroy this pipex_session and break it's pppx_if. Diff below is
quick and dirty fix for this case. At first look, creation of
pipex_timer()-like pppx_timer() is more better, but I am little confused
with pipexintr(). Looks like pipexoutq and pipexinq can contain mbufs
with holds pointer to already destructed pipex_session, because
pipex_destroy_session() and pppx_if_destroy() just frees pipex_session
and potential pppx_timer() is only anoter point to crash kernel.

I suggest, pipex_session and pppx_if should be refactored for good fix,
at least:
1. add reference counter for pipex_session
2. pppx_if should hold a reference to pipex_session
3. pipex_session should have handler to inform it's holder about
destruction

Default npppd.conf has no idle-timeout option, so npppd will not be very
confused with this diff.

Index: sys/net/if_pppx.c
===================================================================
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.77
diff -u -p -r1.77 if_pppx.c
--- sys/net/if_pppx.c   26 Mar 2020 16:50:46 -0000      1.77
+++ sys/net/if_pppx.c   27 Mar 2020 08:20:50 -0000
@@ -665,6 +665,10 @@ pppx_add_session(struct pppx_dev *pxd, s
        struct ifnet *over_ifp = NULL;
 #endif
 
+       /* XXX: prevent pxi destruction by pipex_timer() */
+       if (req->pr_timeout_sec != 0)
+               return (EINVAL);
+
        switch (req->pr_protocol) {
 #ifdef PIPEX_PPPOE
        case PIPEX_PROTO_PPPOE:

Reply via email to