system/freebsd-src
system/freebsd-src
1
0
Fork 0
mirror of https://github.com/freebsd/freebsd-src synced 2024-10-16 21:34:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix locking order reversal in the TCP ratelimit code by moving

Browse source 
destructors outside the rsmtx mutex.

Witness message:
lock order reversal: (sleepable after non-sleepable)
   1st tcp_rs_mtx (rsmtx) @ sys/netinet/tcp_ratelimit.c:242
   2nd sysctl lock (sysctl lock) @ sys/kern/kern_sysctl.c:607

Backtrace:
witness_debugger
witness_checkorder
_rm_wlock_debug
sysctl_ctx_free
rs_destroy
epoch_call_task
gtaskqueue_run_locked
gtaskqueue_thread_loop

Discussed with:	rrs@
Sponsored by:	Mellanox Technologies
This commit is contained in:
Hans Petter Selasky 2019-10-09 16:48:48 +00:00
parent caeeeaa7c5
commit 24be13533b
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=353353
1 changed files with 23 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

43
sys/netinet/tcp_ratelimit.c
View file

@ -237,34 +237,37 @@ static void
rs_destroy(epoch_context_t ctx)
{
struct tcp_rate_set *rs;
bool do_free_rs;
rs = __containerof(ctx, struct tcp_rate_set, rs_epoch_ctx);
mtx_lock(&rs_mtx);
rs->rs_flags &= ~RS_FUNERAL_SCHD;
if (rs->rs_flows_using == 0) {
/*
* In theory its possible (but unlikely)
* that while the delete was occuring
* and we were applying the DEAD flag
* someone slipped in and found the
* interface in a lookup. While we
* decided rs_flows_using were 0 and
* scheduling the epoch_call, the other
* thread incremented rs_flow_using. This
* is because users have a pointer and
* we only use the rs_flows_using in an
* atomic fashion, i.e. the other entities
* are not protected. To assure this did
* not occur, we check rs_flows_using here
* before deleteing.
*/
/*
* In theory its possible (but unlikely)
* that while the delete was occuring
* and we were applying the DEAD flag
* someone slipped in and found the
* interface in a lookup. While we
* decided rs_flows_using were 0 and
* scheduling the epoch_call, the other
* thread incremented rs_flow_using. This
* is because users have a pointer and
* we only use the rs_flows_using in an
* atomic fashion, i.e. the other entities
* are not protected. To assure this did
* not occur, we check rs_flows_using here
* before deleting.
*/
do_free_rs = (rs->rs_flows_using == 0);
rs_number_dead--;
mtx_unlock(&rs_mtx);
if (do_free_rs) {
sysctl_ctx_free(&rs->sysctl_ctx);
free(rs->rs_rlt, M_TCPPACE);
free(rs, M_TCPPACE);
rs_number_dead--;
}
mtx_unlock(&rs_mtx);
}
#ifdef INET