On 26/05/20(Tue) 16:12, Vitaliy Makkoveev wrote:
> `pppx_if' has `pxi_ready' field used to prevent access to incomplete
> `pxi'. But we don't prevent access to `pxi' which we destroy.
> pppx_if_destroy() can sleep so we can grab `pxi' which we already
> destroying by concurrent thread and cause use-after-free issue. I guess
> to use `pxi_ready' to prevent access to `pxi' at destroy stage too.
What about setting this field as first step in pppx_if_destroy()?
> Index: sys/net/if_pppx.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_pppx.c,v
> retrieving revision 1.84
> diff -u -p -r1.84 if_pppx.c
> --- sys/net/if_pppx.c 18 Apr 2020 04:03:56 -0000 1.84
> +++ sys/net/if_pppx.c 26 May 2020 12:59:56 -0000
> @@ -594,8 +594,10 @@ pppxclose(dev_t dev, int flags, int mode
>
> /* XXX */
> NET_LOCK();
> - while ((pxi = LIST_FIRST(&pxd->pxd_pxis)))
> + while ((pxi = LIST_FIRST(&pxd->pxd_pxis))) {
> + pxi->pxi_ready = 0;
> pppx_if_destroy(pxd, pxi);
> + }
> NET_UNLOCK();
>
> LIST_REMOVE(pxd, pxd_entry);
> @@ -951,6 +953,7 @@ pppx_del_session(struct pppx_dev *pxd, s
> pxi = pppx_if_find(pxd, req->pcr_session_id, req->pcr_protocol);
> if (pxi == NULL)
> return (EINVAL);
> + pxi->pxi_ready = 0;
>
> req->pcr_stat = pxi->pxi_session.stat;
>
>
