3 Fixes -
a) There was a case where a ICMP message could cause us to return leaving a stuck lock on an stcb. b) The iterator needed some tweaks to fix its lock ordering. c) The ITERATOR_LOCK is no longer needed in the freeing of a stcb. Now that the timer based one is gone we don't have a multiple resume situation. Add to that that there was somewhere a path out of the freeing of an assoc that did NOT release the iterator_lock.. it was time to clean this old code up and in the process fix the lock bug. MFC after: 1 week
This commit is contained in:
parent
f635c047c5
commit
ec4c19fcf0
@ -3075,6 +3075,11 @@ sctp_iterator_inp_being_freed(struct sctp_inpcb *inp)
|
||||
} else {
|
||||
it->inp = LIST_NEXT(it->inp, sctp_list);
|
||||
}
|
||||
/*
|
||||
* When its put in the refcnt is incremented so decr
|
||||
* it
|
||||
*/
|
||||
SCTP_INP_DECR_REF(inp);
|
||||
}
|
||||
it = nit;
|
||||
}
|
||||
@ -4428,38 +4433,6 @@ sctp_add_vtag_to_timewait(uint32_t tag, uint32_t time, uint16_t lport, uint16_t
|
||||
}
|
||||
|
||||
|
||||
static void
|
||||
sctp_iterator_asoc_being_freed(struct sctp_inpcb *inp, struct sctp_tcb *stcb)
|
||||
{
|
||||
struct sctp_iterator *it;
|
||||
|
||||
/*
|
||||
* Unlock the tcb lock we do this so we avoid a dead lock scenario
|
||||
* where the iterator is waiting on the TCB lock and the TCB lock is
|
||||
* waiting on the iterator lock.
|
||||
*/
|
||||
it = stcb->asoc.stcb_starting_point_for_iterator;
|
||||
if (it == NULL) {
|
||||
return;
|
||||
}
|
||||
if (it->inp != stcb->sctp_ep) {
|
||||
/* hmm, focused on the wrong one? */
|
||||
return;
|
||||
}
|
||||
if (it->stcb != stcb) {
|
||||
return;
|
||||
}
|
||||
it->stcb = LIST_NEXT(stcb, sctp_tcblist);
|
||||
if (it->stcb == NULL) {
|
||||
/* done with all asoc's in this assoc */
|
||||
if (it->iterator_flags & SCTP_ITERATOR_DO_SINGLE_INP) {
|
||||
it->inp = NULL;
|
||||
} else {
|
||||
it->inp = LIST_NEXT(inp, sctp_list);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
/*-
|
||||
* Free the association after un-hashing the remote port. This
|
||||
@ -4665,7 +4638,6 @@ sctp_free_assoc(struct sctp_inpcb *inp, struct sctp_tcb *stcb, int from_inpcbfre
|
||||
|
||||
SCTP_TCB_UNLOCK(stcb);
|
||||
|
||||
SCTP_ITERATOR_LOCK();
|
||||
SCTP_INP_INFO_WLOCK();
|
||||
SCTP_INP_WLOCK(inp);
|
||||
SCTP_TCB_LOCK(stcb);
|
||||
@ -4704,7 +4676,6 @@ sctp_free_assoc(struct sctp_inpcb *inp, struct sctp_tcb *stcb, int from_inpcbfre
|
||||
* Make it invalid too, that way if its about to run it will abort
|
||||
* and return.
|
||||
*/
|
||||
sctp_iterator_asoc_being_freed(inp, stcb);
|
||||
/* re-increment the lock */
|
||||
if (from_inpcbfree == SCTP_NORMAL_PROC) {
|
||||
atomic_add_int(&stcb->asoc.refcnt, -1);
|
||||
@ -4721,7 +4692,6 @@ sctp_free_assoc(struct sctp_inpcb *inp, struct sctp_tcb *stcb, int from_inpcbfre
|
||||
if (from_inpcbfree == SCTP_NORMAL_PROC) {
|
||||
SCTP_INP_INCR_REF(inp);
|
||||
SCTP_INP_WUNLOCK(inp);
|
||||
SCTP_ITERATOR_UNLOCK();
|
||||
}
|
||||
/* pull from vtag hash */
|
||||
LIST_REMOVE(stcb, sctp_asocs);
|
||||
@ -6694,20 +6664,22 @@ sctp_initiate_iterator(inp_func inpf,
|
||||
it->no_chunk_output = chunk_output_off;
|
||||
it->vn = curvnet;
|
||||
if (s_inp) {
|
||||
/* Assume lock is held here */
|
||||
it->inp = s_inp;
|
||||
SCTP_INP_INCR_REF(it->inp);
|
||||
it->iterator_flags = SCTP_ITERATOR_DO_SINGLE_INP;
|
||||
} else {
|
||||
SCTP_INP_INFO_RLOCK();
|
||||
it->inp = LIST_FIRST(&SCTP_BASE_INFO(listhead));
|
||||
|
||||
if (it->inp) {
|
||||
SCTP_INP_INCR_REF(it->inp);
|
||||
}
|
||||
SCTP_INP_INFO_RUNLOCK();
|
||||
it->iterator_flags = SCTP_ITERATOR_DO_ALL_INP;
|
||||
|
||||
}
|
||||
SCTP_IPI_ITERATOR_WQ_LOCK();
|
||||
if (it->inp) {
|
||||
SCTP_INP_INCR_REF(it->inp);
|
||||
}
|
||||
|
||||
TAILQ_INSERT_TAIL(&sctp_it_ctl.iteratorhead, it, sctp_nxt_itr);
|
||||
if (sctp_it_ctl.iterator_running == 0) {
|
||||
sctp_wakeup_iterator();
|
||||
|
@ -402,6 +402,9 @@ sctp_ctlinput(cmd, sa, vip)
|
||||
SCTP_INP_DECR_REF(inp);
|
||||
SCTP_INP_WUNLOCK(inp);
|
||||
}
|
||||
if (stcb) {
|
||||
SCTP_TCB_UNLOCK(stcb);
|
||||
}
|
||||
}
|
||||
}
|
||||
return;
|
||||
|
@ -1255,15 +1255,20 @@ sctp_iterator_work(struct sctp_iterator *it)
|
||||
{
|
||||
int iteration_count = 0;
|
||||
int inp_skip = 0;
|
||||
int first_in = 1;
|
||||
struct sctp_inpcb *tinp;
|
||||
|
||||
SCTP_INP_INFO_RLOCK();
|
||||
SCTP_ITERATOR_LOCK();
|
||||
if (it->inp) {
|
||||
SCTP_INP_RLOCK(it->inp);
|
||||
SCTP_INP_DECR_REF(it->inp);
|
||||
}
|
||||
if (it->inp == NULL) {
|
||||
/* iterator is complete */
|
||||
done_with_iterator:
|
||||
SCTP_ITERATOR_UNLOCK();
|
||||
SCTP_INP_INFO_RUNLOCK();
|
||||
if (it->function_atend != NULL) {
|
||||
(*it->function_atend) (it->pointer, it->val);
|
||||
}
|
||||
@ -1271,7 +1276,11 @@ sctp_iterator_work(struct sctp_iterator *it)
|
||||
return;
|
||||
}
|
||||
select_a_new_ep:
|
||||
SCTP_INP_RLOCK(it->inp);
|
||||
if (first_in) {
|
||||
first_in = 0;
|
||||
} else {
|
||||
SCTP_INP_RLOCK(it->inp);
|
||||
}
|
||||
while (((it->pcb_flags) &&
|
||||
((it->inp->sctp_flags & it->pcb_flags) != it->pcb_flags)) ||
|
||||
((it->pcb_features) &&
|
||||
@ -1281,8 +1290,9 @@ sctp_iterator_work(struct sctp_iterator *it)
|
||||
SCTP_INP_RUNLOCK(it->inp);
|
||||
goto done_with_iterator;
|
||||
}
|
||||
SCTP_INP_RUNLOCK(it->inp);
|
||||
tinp = it->inp;
|
||||
it->inp = LIST_NEXT(it->inp, sctp_list);
|
||||
SCTP_INP_RUNLOCK(tinp);
|
||||
if (it->inp == NULL) {
|
||||
goto done_with_iterator;
|
||||
}
|
||||
@ -1323,6 +1333,8 @@ sctp_iterator_work(struct sctp_iterator *it)
|
||||
SCTP_INP_INCR_REF(it->inp);
|
||||
SCTP_INP_RUNLOCK(it->inp);
|
||||
SCTP_ITERATOR_UNLOCK();
|
||||
SCTP_INP_INFO_RUNLOCK();
|
||||
SCTP_INP_INFO_RLOCK();
|
||||
SCTP_ITERATOR_LOCK();
|
||||
if (sctp_it_ctl.iterator_flags) {
|
||||
/* We won't be staying here */
|
||||
@ -1382,9 +1394,7 @@ sctp_iterator_work(struct sctp_iterator *it)
|
||||
if (it->iterator_flags & SCTP_ITERATOR_DO_SINGLE_INP) {
|
||||
it->inp = NULL;
|
||||
} else {
|
||||
SCTP_INP_INFO_RLOCK();
|
||||
it->inp = LIST_NEXT(it->inp, sctp_list);
|
||||
SCTP_INP_INFO_RUNLOCK();
|
||||
}
|
||||
if (it->inp == NULL) {
|
||||
goto done_with_iterator;
|
||||
|
Loading…
Reference in New Issue
Block a user