From 66e7619099a5810908fdc09124b4f254c54c3563 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Mon, 8 Nov 2010 03:06:14 +0000 Subject: [PATCH] [retry] Process at most one timer's expiry in each call to retry_step() Calling a timer's expiry method may cause arbitrary consequences, including arbitrary modifications of the list of retry timers. list_for_each_entry_safe() guards against only deletion of the current list entry; it provides no protection against other list modifications. In particular, if a timer's expiry method causes the subsequent timer in the list to be deleted, then the next loop iteration will access a timer that may no longer exist. This is a particularly nasty bug, since absolutely none of the list-manipulation or reference-counting assertion checks will be triggered. (The first assertion failure happens on the next iteration through list_for_each_entry(), showing that the list has become corrupted but providing no clue as to when this happened.) Fix by stopping traversal of the list of retry timers as soon as we hit an expired timer. Signed-off-by: Michael Brown --- src/net/retry.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/net/retry.c b/src/net/retry.c index b65bb57e..082be39b 100644 --- a/src/net/retry.c +++ b/src/net/retry.c @@ -180,14 +180,20 @@ static void timer_expired ( struct retry_timer *timer ) { */ static void retry_step ( struct process *process __unused ) { struct retry_timer *timer; - struct retry_timer *tmp; unsigned long now = currticks(); unsigned long used; - list_for_each_entry_safe ( timer, tmp, &timers, list ) { + /* Process at most one timer expiry. We cannot process + * multiple expiries in one pass, because one timer expiring + * may end up triggering another timer's deletion from the + * list. + */ + list_for_each_entry ( timer, &timers, list ) { used = ( now - timer->start ); - if ( used >= timer->timeout ) + if ( used >= timer->timeout ) { timer_expired ( timer ); + break; + } } }