Skip to content

Commit

Permalink
kernel: Fix double-list-removal corruption case in timeout handling
Browse files Browse the repository at this point in the history
This fixes #8669, and is distressingly subtle for a one-line patch:

The list iteration code in _handle_expired_timeouts() would remove the
timeout from our (temporary -- the dlist header is on the stack of our
calling function) list of expired timeouts before invoking the
handler.  But sys_dlist_remove() only fixes up the containing list
pointers, leaving garbage in the node.  If the action of that handler
is to re-add the timeout (which is very common!) then that will then
try to remove it AGAIN from the same list.

Even then, the common case is that the expired list contains only one
item, so the result is a perfectly valid empty list that affects
nothing.  But if you have more than one, you get a corrupt cycle in
the iteration list and things get weird.

As it happens, there's no value in trying to remove this timeout from
the temporary list at all.  Just iterate over it naturally.

Really, this design is fragile: we shouldn't be reusing the list nodes
in struct _timeout for this purpose and should figure out some other
mechanism.  But this fix should be good for now.

Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
  • Loading branch information
Andy Ross committed Aug 24, 2018
1 parent e6b45fe commit 2376a77
Showing 1 changed file with 2 additions and 3 deletions.
5 changes: 2 additions & 3 deletions kernel/include/timeout_q.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,9 @@ static inline void _handle_one_expired_timeout(struct _timeout *timeout)

static inline void _handle_expired_timeouts(sys_dlist_t *expired)
{
struct _timeout *timeout, *next;
struct _timeout *timeout;

SYS_DLIST_FOR_EACH_CONTAINER_SAFE(expired, timeout, next, node) {
sys_dlist_remove(&timeout->node);
SYS_DLIST_FOR_EACH_CONTAINER(expired, timeout, node) {
_handle_one_expired_timeout(timeout);
}
}
Expand Down

0 comments on commit 2376a77

Please sign in to comment.