Browse Source

[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 <mcb30@ipxe.org>
tags/v1.20.1
Michael Brown 14 years ago
parent
commit
66e7619099
1 changed files with 9 additions and 3 deletions
  1. 9
    3
      src/net/retry.c

+ 9
- 3
src/net/retry.c View File

180
  */
180
  */
181
 static void retry_step ( struct process *process __unused ) {
181
 static void retry_step ( struct process *process __unused ) {
182
 	struct retry_timer *timer;
182
 	struct retry_timer *timer;
183
-	struct retry_timer *tmp;
184
 	unsigned long now = currticks();
183
 	unsigned long now = currticks();
185
 	unsigned long used;
184
 	unsigned long used;
186
 
185
 
187
-	list_for_each_entry_safe ( timer, tmp, &timers, list ) {
186
+	/* Process at most one timer expiry.  We cannot process
187
+	 * multiple expiries in one pass, because one timer expiring
188
+	 * may end up triggering another timer's deletion from the
189
+	 * list.
190
+	 */
191
+	list_for_each_entry ( timer, &timers, list ) {
188
 		used = ( now - timer->start );
192
 		used = ( now - timer->start );
189
-		if ( used >= timer->timeout )
193
+		if ( used >= timer->timeout ) {
190
 			timer_expired ( timer );
194
 			timer_expired ( timer );
195
+			break;
196
+		}
191
 	}
197
 	}
192
 }
198
 }
193
 
199
 

Loading…
Cancel
Save