Re: pppx(4): prevent concurency with pppx_if_destroy()

[Vitaliy Makkoveev]

Forgot to release lock in pppx_del_session() error case...

Index: sys/net/if_pppx.c
===================================================================
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.81
diff -u -p -r1.81 if_pppx.c
--- sys/net/if_pppx.c   7 Apr 2020 07:11:22 -0000       1.81
+++ sys/net/if_pppx.c   7 Apr 2020 14:39:28 -0000
@@ -170,7 +170,8 @@ RBT_HEAD(pppx_ifs, pppx_if) pppx_ifs = R
 RBT_PROTOTYPE(pppx_ifs, pppx_if, pxi_entry, pppx_if_cmp);
 
 int            pppx_if_next_unit(void);
-struct pppx_if *pppx_if_find(struct pppx_dev *, int, int);
+struct pppx_if *pppx_if_find_locked(struct pppx_dev *, int, int);
+static inline struct pppx_if *pppx_if_find(struct pppx_dev *, int, int);
 int            pppx_add_session(struct pppx_dev *,
                    struct pipex_session_req *);
 int            pppx_del_session(struct pppx_dev *,
@@ -594,8 +595,19 @@ pppxclose(dev_t dev, int flags, int mode
 
        /* XXX */
        NET_LOCK();
-       while ((pxi = LIST_FIRST(&pxd->pxd_pxis)))
+       rw_enter_write(&pppx_ifs_lk);
+       while ((pxi = LIST_FIRST(&pxd->pxd_pxis))) {
+               if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
+                       panic("%s: pppx_ifs modified while lock was held",
+                               __func__);
+               LIST_REMOVE(pxi, pxi_list);
+               rw_exit_write(&pppx_ifs_lk);
+               
                pppx_if_destroy(pxd, pxi);
+               
+               rw_enter_write(&pppx_ifs_lk);
+       }
+       rw_exit_write(&pppx_ifs_lk);
        NET_UNLOCK();
 
        LIST_REMOVE(pxd, pxd_entry);
@@ -641,24 +653,37 @@ pppx_if_next_unit(void)
 }
 
 struct pppx_if *
-pppx_if_find(struct pppx_dev *pxd, int session_id, int protocol)
+pppx_if_find_locked(struct pppx_dev *pxd, int session_id, int protocol)
 {
        struct pppx_if *s, *p;
+
+       rw_assert_anylock(&pppx_ifs_lk);
+
        s = malloc(sizeof(*s), M_DEVBUF, M_WAITOK | M_ZERO);
 
        s->pxi_key.pxik_session_id = session_id;
        s->pxi_key.pxik_protocol = protocol;
 
-       rw_enter_read(&pppx_ifs_lk);
        p = RBT_FIND(pppx_ifs, &pppx_ifs, s);
        if (p && p->pxi_ready == 0)
                p = NULL;
-       rw_exit_read(&pppx_ifs_lk);
 
        free(s, M_DEVBUF, sizeof(*s));
        return (p);
 }
 
+static inline struct pppx_if *
+pppx_if_find(struct pppx_dev *pxd, int session_id, int protocol)
+{
+       struct pppx_if *pxi;
+
+       rw_enter_read(&pppx_ifs_lk);
+       pxi = pppx_if_find_locked(pxd, session_id, protocol);
+       rw_exit_read(&pppx_ifs_lk);
+
+       return pxi;
+}
+
 int
 pppx_add_session(struct pppx_dev *pxd, struct pipex_session_req *req)
 {
@@ -948,14 +973,24 @@ pppx_del_session(struct pppx_dev *pxd, s
 {
        struct pppx_if *pxi;
 
-       pxi = pppx_if_find(pxd, req->pcr_session_id, req->pcr_protocol);
-       if (pxi == NULL)
+       rw_enter_write(&pppx_ifs_lk);
+       pxi = pppx_if_find_locked(pxd, req->pcr_session_id,
+           req->pcr_protocol);
+       if (pxi == NULL) {
+               rw_exit_write(&pppx_ifs_lk);
                return (EINVAL);
+       }
+
+       if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
+               panic("%s: pppx_ifs modified while lock was held", __func__);
+       LIST_REMOVE(pxi, pxi_list);
+       rw_exit_write(&pppx_ifs_lk);
 
        req->pcr_stat = pxi->pxi_session.stat;
 
        pppx_if_destroy(pxd, pxi);
-       return (0);
+
+       return 0;
 }
 
 int
@@ -1037,12 +1072,6 @@ pppx_if_destroy(struct pppx_dev *pxd, st
        NET_UNLOCK();
        if_detach(ifp);
        NET_LOCK();
-
-       rw_enter_write(&pppx_ifs_lk);
-       if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
-               panic("%s: pppx_ifs modified while lock was held", __func__);
-       LIST_REMOVE(pxi, pxi_list);
-       rw_exit_write(&pppx_ifs_lk);
 
        pool_put(pppx_if_pl, pxi);
 }