d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

select: make sure there are no wakeup attempts after selfdfree returns

Browse Source 
Prior to the patch returning selfdfree could still be racing against doselwakeup
which set sf_si = NULL and now locks stp to wake up the other thread.

A sufficiently unlucky pair can end up going all the way down to freeing
select-related structures before the lock/wakeup/unlock finishes.

This started manifesting itself as crashes since select data started getting
freed in r367714.
This commit is contained in:
Mateusz Guzik 2020-12-02 00:48:15 +00:00
parent 013a1ae66e
commit 10e64782ed
1 changed files with 17 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
sys/kern/sys_generic.c
View File

@ -1820,14 +1820,17 @@ doselwakeup(struct selinfo *sip, int pri)
*/
TAILQ_REMOVE(&sip->si_tdlist, sfp, sf_threads);
stp = sfp->sf_td;
/*
* Paired with selfdfree.
*/
atomic_store_rel_ptr((uintptr_t *)&sfp->sf_si, (uintptr_t)NULL);
mtx_lock(&stp->st_mtx);
stp->st_flags |= SELTD_PENDING;
cv_broadcastpri(&stp->st_wait, pri);
mtx_unlock(&stp->st_mtx);
/*
* Paired with selfdfree.
*
* Storing this only after the wakeup provides an invariant that
* stp is not used after selfdfree returns.
*/
atomic_store_rel_ptr((uintptr_t *)&sfp->sf_si, (uintptr_t)NULL);
}
mtx_unlock(sip->si_mtx);
}
@ -1837,14 +1840,18 @@ seltdinit(struct thread *td)
{
struct seltd *stp;
if ((stp = td->td_sel) != NULL)
goto out;
td->td_sel = stp = malloc(sizeof(*stp), M_SELECT, M_WAITOK|M_ZERO);
stp = td->td_sel;
if (stp != NULL) {
MPASS(stp->st_flags == 0);
MPASS(STAILQ_EMPTY(&stp->st_selq));
return;
}
stp = malloc(sizeof(*stp), M_SELECT, M_WAITOK|M_ZERO);
mtx_init(&stp->st_mtx, "sellck", NULL, MTX_DEF);
cv_init(&stp->st_wait, "select");
out:
stp->st_flags = 0;
STAILQ_INIT(&stp->st_selq);
td->td_sel = stp;
}
static int
@ -1887,6 +1894,8 @@ seltdfini(struct thread *td)
stp = td->td_sel;
if (stp == NULL)
return;
MPASS(stp->st_flags == 0);
MPASS(STAILQ_EMPTY(&stp->st_selq));
if (stp->st_free1)
free(stp->st_free1, M_SELFD);
if (stp->st_free2)