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

Try to catch a couple of SCTP teardown race conditions.

Browse Source 
Saw all the printfs already.

Note: not sure the atomics are needed but without them, the condition
would never trigger, and we'd still see panics (which could have been
due to the insert race).  Will work my way backwards in case this stays
stable.

Sponsored by:	The FreeBSD Foundation
This commit is contained in:
Bjoern A. Zeeb 2016-01-23 11:05:13 +00:00
parent eef5775f02
commit 27a01c6c0c
5 changed files with 36 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
sys/netinet/sctp_asconf.c
View File

@ -3248,6 +3248,7 @@ sctp_addr_mgmt_ep_sa(struct sctp_inpcb *inp, struct sockaddr *sa,
} else {
struct sctp_asconf_iterator *asc;
struct sctp_laddr *wi;
int ret;
SCTP_MALLOC(asc, struct sctp_asconf_iterator *,
sizeof(struct sctp_asconf_iterator),
@ -3269,7 +3270,7 @@ sctp_addr_mgmt_ep_sa(struct sctp_inpcb *inp, struct sockaddr *sa,
wi->action = type;
atomic_add_int(&ifa->refcount, 1);
LIST_INSERT_HEAD(&asc->list_of_work, wi, sctp_nxt_addr);
(void)sctp_initiate_iterator(sctp_asconf_iterator_ep,
ret = sctp_initiate_iterator(sctp_asconf_iterator_ep,
sctp_asconf_iterator_stcb,
sctp_asconf_iterator_ep_end,
SCTP_PCB_ANY_FLAGS,
@ -3277,6 +3278,15 @@ sctp_addr_mgmt_ep_sa(struct sctp_inpcb *inp, struct sockaddr *sa,
SCTP_ASOC_ANY_STATE,
(void *)asc, 0,
sctp_asconf_iterator_end, inp, 0);
if (ret) {
SCTP_PRINTF("Failed to initiate iterator for addr_mgmt_ep_sa\n");
SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP_ASCONF, EFAULT);
atomic_add_int(&ifa->refcount, -1);
SCTP_DECR_LADDR_COUNT();
SCTP_ZONE_FREE(SCTP_BASE_INFO(ipi_zone_laddr), wi);
SCTP_FREE(asc, SCTP_M_ASC_IT);
return (EFAULT);
}
}
return (0);
} else {

4
sys/netinet/sctp_bsd_addr.c
View File

@ -77,7 +77,7 @@ struct iterator_control sctp_it_ctl;
void
sctp_wakeup_iterator(void)
{
wakeup(&sctp_it_ctl.iterator_running);
wakeup(&sctp_it_ctl.iterator_flags);
}
static void
@ -86,7 +86,7 @@ sctp_iterator_thread(void *v SCTP_UNUSED)
SCTP_IPI_ITERATOR_WQ_LOCK();
/* In FreeBSD this thread never terminates. */
for (;;) {
msleep(&sctp_it_ctl.iterator_running,
msleep(&sctp_it_ctl.iterator_flags,
&sctp_it_ctl.ipi_iterator_wq_mtx,
0, "waiting_for_work", 0);
sctp_iterator_worker();

26
sys/netinet/sctp_pcb.c
View File

@ -5929,6 +5929,7 @@ sctp_pcb_finish(void)
struct sctp_tagblock *twait_block, *prev_twait_block;
struct sctp_laddr *wi, *nwi;
int i;
unsigned int r;
struct sctp_iterator *it, *nit;
if (SCTP_BASE_VAR(sctp_pcb_initialized) == 0) {
@ -5943,8 +5944,6 @@ sctp_pcb_finish(void)
* still add the ifdef to make it compile on old versions.
*/
retry:
while (sctp_it_ctl.iterator_running != 0)
DELAY(1);
SCTP_IPI_ITERATOR_WQ_LOCK();
/*
* sctp_iterator_worker() might be working on an it entry without
@ -5953,10 +5952,13 @@ retry:
* avoid the race condition as sctp_iterator_worker() will have to
* wait to re-aquire the lock.
*/
if (sctp_it_ctl.cur_it != NULL || sctp_it_ctl.iterator_running != 0) {
r = atomic_fetchadd_int(&sctp_it_ctl.iterator_running, 0);
if (r != 0 || sctp_it_ctl.cur_it != NULL) {
SCTP_IPI_ITERATOR_WQ_UNLOCK();
printf("%s: Iterator running while we held the lock. Retry.\n",
__func__);
/* XXX-BZ make this a statistics variable. */
printf("%s: Iterator running while we held the lock. Retry. "
"r=%u cur_it=%p\n", __func__, r, sctp_it_ctl.cur_it);
DELAY(10);
goto retry;
}
TAILQ_FOREACH_SAFE(it, &sctp_it_ctl.iteratorhead, sctp_nxt_itr, nit) {
@ -7022,6 +7024,11 @@ sctp_initiate_iterator(inp_func inpf,
if (af == NULL) {
return (-1);
}
if (SCTP_BASE_VAR(sctp_pcb_initialized) == 0) {
printf("%s: abort on initialize being %d\n", __func__,
SCTP_BASE_VAR(sctp_pcb_initialized));
return (-1);
}
SCTP_MALLOC(it, struct sctp_iterator *, sizeof(struct sctp_iterator),
SCTP_M_ITER);
if (it == NULL) {
@ -7060,9 +7067,16 @@ sctp_initiate_iterator(inp_func inpf,
}
SCTP_IPI_ITERATOR_WQ_LOCK();
if (SCTP_BASE_VAR(sctp_pcb_initialized) == 0) {
SCTP_IPI_ITERATOR_WQ_UNLOCK();
printf("%s: rollback on initialize being %d it=%p\n", __func__,
SCTP_BASE_VAR(sctp_pcb_initialized), it);
SCTP_FREE(it, SCTP_M_ITER);
return (-1);
}
TAILQ_INSERT_TAIL(&sctp_it_ctl.iteratorhead, it, sctp_nxt_itr);
if (sctp_it_ctl.iterator_running == 0) {
if (atomic_fetchadd_int(&sctp_it_ctl.iterator_running, 0) == 0) {
sctp_wakeup_iterator();
}
SCTP_IPI_ITERATOR_WQ_UNLOCK();

2
sys/netinet/sctp_structs.h
View File

@ -180,7 +180,7 @@ struct iterator_control {
SCTP_PROCESS_STRUCT thread_proc;
struct sctpiterators iteratorhead;
struct sctp_iterator *cur_it;
uint32_t iterator_running;
volatile uint32_t iterator_running;
uint32_t iterator_flags;
};

4
sys/netinet/sctputil.c
View File

@ -1421,7 +1421,7 @@ sctp_iterator_worker(void)
/* This function is called with the WQ lock in place */
sctp_it_ctl.iterator_running = 1;
atomic_store_rel_int(&sctp_it_ctl.iterator_running, 1);
TAILQ_FOREACH_SAFE(it, &sctp_it_ctl.iteratorhead, sctp_nxt_itr, nit) {
sctp_it_ctl.cur_it = it;
/* now lets work on this one */
@ -1434,7 +1434,7 @@ sctp_iterator_worker(void)
SCTP_IPI_ITERATOR_WQ_LOCK();
/* sa_ignore FREED_MEMORY */
}
sctp_it_ctl.iterator_running = 0;
atomic_store_rel_int(&sctp_it_ctl.iterator_running, 0);
return;
}