--- a/sys/kern/kern_descrip.c +++ b/sys/kern/kern_descrip.c @@ -1,19 +1,37 @@ void funsetown(struct sigio **sigiop) { + struct sigio *sigio; + + /* + * DF-2682 FIX: the pointer is loaded and cleared under + * sigio_token. Raw-pointer readers (the pgsigio() call sites) + * hold sigio_token shared around their load+use; + * funsetown_free() re-acquires it exclusively for the kfree() + * so the free is ordered after the last in-flight reader. + */ + lwkt_gettoken(&sigio_token); + if ((sigio = *sigiop) == NULL) { + lwkt_reltoken(&sigio_token); + return; + } + KKASSERT(sigiop == sigio->sio_myref); + *sigiop = NULL; + lwkt_reltoken(&sigio_token); + + funsetown_free(sigio); +} + +/* + * Tear down and free a sigio whose *sio_myref has already been cleared. + * No sigio_token is held across the owner-list removals (exit1() holds + * p_token across funsetownlst(), so nesting the tokens can livelock). + */ +static void +funsetown_free(struct sigio *sigio) +{ struct pgrp *pgrp; struct proc *p; - struct sigio *sigio; - - if ((sigio = *sigiop) != NULL) { - lwkt_gettoken(&sigio_token); /* protect sigio */ - KKASSERT(sigiop == sigio->sio_myref); - sigio = *sigiop; - *sigiop = NULL; - lwkt_reltoken(&sigio_token); - } - if (sigio == NULL) - return; if (sigio->sio_pgid < 0) { pgrp = sigio->sio_pgrp; @@ -22,7 +40,7 @@ SLIST_REMOVE(&pgrp->pg_sigiolst, sigio, sigio, sio_pgsigio); lwkt_reltoken(&pgrp->pg_token); pgrel(pgrp); - } else /* if ((*sigiop)->sio_pgid > 0) */ { + } else /* if (sigio->sio_pgid > 0) */ { p = sigio->sio_proc; sigio->sio_proc = NULL; PHOLD(p); @@ -33,20 +51,46 @@ } crfree(sigio->sio_ucred); sigio->sio_ucred = NULL; + + /* + * DF-2682 FIX: free only after every in-flight reader is done. + */ + lwkt_gettoken(&sigio_token); kfree(sigio, M_SIGIO); + lwkt_reltoken(&sigio_token); } /* * Free a list of sigio structures. Caller is responsible for ensuring * that the list is MPSAFE. + * + * DF-2683 FIX (v4): read the list head UNDER sigio_token (a head read + * outside the token raced free+slab-reuse into an identity confusion + * and a wrong-list SLIST_REMOVE). Publication-in-flight entries are + * retried after a 1-tick sleep; sleeping releases our lwkt tokens so + * the publisher/remover can always run. */ void funsetownlst(struct sigiolst *sigiolst) { struct sigio *sigio; - while ((sigio = SLIST_FIRST(sigiolst)) != NULL) - funsetown(sigio->sio_myref); + for (;;) { + lwkt_gettoken(&sigio_token); + sigio = SLIST_FIRST(sigiolst); + if (sigio == NULL) { + lwkt_reltoken(&sigio_token); + break; + } + if (*sigio->sio_myref != sigio) { + lwkt_reltoken(&sigio_token); + tsleep(sigio, 0, "sigbrth", 1); + continue; + } + *sigio->sio_myref = NULL; + lwkt_reltoken(&sigio_token); + funsetown_free(sigio); + } } /* @@ -108,25 +152,33 @@ } } sigio = kmalloc(sizeof(struct sigio), M_SIGIO, M_WAITOK | M_ZERO); - if (pgid > 0) { - KKASSERT(pgrp == NULL); - lwkt_gettoken(&proc->p_token); - SLIST_INSERT_HEAD(&proc->p_sigiolst, sigio, sio_pgsigio); - sigio->sio_proc = proc; - lwkt_reltoken(&proc->p_token); - } else { - KKASSERT(proc == NULL); - lwkt_gettoken(&pgrp->pg_token); - SLIST_INSERT_HEAD(&pgrp->pg_sigiolst, sigio, sio_pgsigio); - sigio->sio_pgrp = pgrp; - lwkt_reltoken(&pgrp->pg_token); - pgrp = NULL; - } + + /* + * DF-2683 FIX: fully initialize the sigio BEFORE linking it into + * the owner's list (old code initialized sio_pgid/sio_ucred/ + * sio_ruid/sio_myref only after the insert, letting a teardown + * walker consume a half-born sigio -> funsetown(NULL) NULL fault + * or wrong-branch SLIST_REMOVE walk-off). + */ sigio->sio_pgid = pgid; sigio->sio_ucred = crhold(curthread->td_ucred); /* It would be convenient if p_ruid was in ucred. */ sigio->sio_ruid = sigio->sio_ucred->cr_ruid; sigio->sio_myref = sigiop; + if (pgid > 0) { + KKASSERT(pgrp == NULL); + sigio->sio_proc = proc; + lwkt_gettoken(&proc->p_token); + SLIST_INSERT_HEAD(&proc->p_sigiolst, sigio, sio_pgsigio); + lwkt_reltoken(&proc->p_token); + } else { + KKASSERT(proc == NULL); + sigio->sio_pgrp = pgrp; + lwkt_gettoken(&pgrp->pg_token); + SLIST_INSERT_HEAD(&pgrp->pg_sigiolst, sigio, sio_pgsigio); + lwkt_reltoken(&pgrp->pg_token); + pgrp = NULL; + } lwkt_gettoken(&sigio_token); while (*sigiop) --- a/sys/kern/kern_descrip.c +++ b/sys/kern/kern_descrip.c @@ -1,23 +1,31 @@ sigio = kmalloc(sizeof(struct sigio), M_SIGIO, M_WAITOK | M_ZERO); - if (pgid > 0) { - KKASSERT(pgrp == NULL); - lwkt_gettoken(&proc->p_token); - SLIST_INSERT_HEAD(&proc->p_sigiolst, sigio, sio_pgsigio); - sigio->sio_proc = proc; - lwkt_reltoken(&proc->p_token); - } else { - KKASSERT(proc == NULL); - lwkt_gettoken(&pgrp->pg_token); - SLIST_INSERT_HEAD(&pgrp->pg_sigiolst, sigio, sio_pgsigio); - sigio->sio_pgrp = pgrp; - lwkt_reltoken(&pgrp->pg_token); - pgrp = NULL; - } + + /* + * DF-2683 FIX: fully initialize the sigio BEFORE linking it into + * the owner's list (old code initialized sio_pgid/sio_ucred/ + * sio_ruid/sio_myref only after the insert, letting a teardown + * walker consume a half-born sigio -> funsetown(NULL) NULL fault + * or wrong-branch SLIST_REMOVE walk-off). + */ sigio->sio_pgid = pgid; sigio->sio_ucred = crhold(curthread->td_ucred); /* It would be convenient if p_ruid was in ucred. */ sigio->sio_ruid = sigio->sio_ucred->cr_ruid; sigio->sio_myref = sigiop; + if (pgid > 0) { + KKASSERT(pgrp == NULL); + sigio->sio_proc = proc; + lwkt_gettoken(&proc->p_token); + SLIST_INSERT_HEAD(&proc->p_sigiolst, sigio, sio_pgsigio); + lwkt_reltoken(&proc->p_token); + } else { + KKASSERT(proc == NULL); + sigio->sio_pgrp = pgrp; + lwkt_gettoken(&pgrp->pg_token); + SLIST_INSERT_HEAD(&pgrp->pg_sigiolst, sigio, sio_pgsigio); + lwkt_reltoken(&pgrp->pg_token); + pgrp = NULL; + } lwkt_gettoken(&sigio_token); while (*sigiop)