Now pipex(4) session can't be grabbed while going to be destroyed.
So `pppx_ifs' tree can't be broken by NET_LOCK() dances in
pppx_if_destroy() and I guess it's time to remove panic()'s from
pppx(4).

claudio@ pointed in [1] we can avoid two lookups while inserting `pxi'
into `pppx_ifs' so I removed RBT_FIND() from pppx_add_session(). Also
there is no context switch between RBT_INSERT() and RBT_REMOVE() in
error path so additional checks for RBT_REMOVE() are unnecessary.

I replaced panic() by KASSERT() in pppx_if_destroy(). It's also
unnecessary but, if I remember correct, claudio@ privately said he like
to keep assertion here.

1. https://marc.info/?l=openbsd-tech&m=158625333217729&w=2


Index: sys/net/if_pppx.c
===================================================================
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.88
diff -u -p -r1.88 if_pppx.c
--- sys/net/if_pppx.c   18 Jun 2020 14:20:12 -0000      1.88
+++ sys/net/if_pppx.c   18 Jun 2020 16:40:18 -0000
@@ -696,14 +696,10 @@ pppx_add_session(struct pppx_dev *pxd, s
        pxi->pxi_key.pxik_protocol = req->pr_protocol;
        pxi->pxi_dev = pxd;
 
-       /* this is safe without splnet since we're not modifying it */
-       if (RBT_FIND(pppx_ifs, &pppx_ifs, pxi) != NULL) {
+       if (RBT_INSERT(pppx_ifs, &pppx_ifs, pxi) != NULL) {
                error = EADDRINUSE;
                goto out;
        }
-
-       if (RBT_INSERT(pppx_ifs, &pppx_ifs, pxi) != NULL)
-               panic("%s: pppx_ifs modified while lock was held", __func__);
        LIST_INSERT_HEAD(&pxd->pxd_pxis, pxi, pxi_list);
 
        snprintf(ifp->if_xname, sizeof(ifp->if_xname), "%s%d", "pppx", unit);
@@ -776,8 +772,7 @@ pppx_add_session(struct pppx_dev *pxd, s
        return (error);
 
 remove:
-       if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
-               panic("%s: pppx_ifs modified while lock was held", __func__);
+       RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi);
        LIST_REMOVE(pxi, pxi_list);
 out:
        pool_put(pppx_if_pl, pxi);
@@ -870,8 +865,7 @@ pppx_if_destroy(struct pppx_dev *pxd, st
        NET_LOCK();
 
        pipex_rele_session(session);
-       if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
-               panic("%s: pppx_ifs modified while lock was held", __func__);
+       KASSERT(RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) != NULL);
        LIST_REMOVE(pxi, pxi_list);
 
        pool_put(pppx_if_pl, pxi);

Reply via email to