Skip to content

Commit a5f3d8a

Browse files
Coly Liaxboe
authored andcommitted
bcache: use llist_for_each_entry_safe() in __closure_wake_up()
Commit 09b3efe ("bcache: Don't reinvent the wheel but use existing llist API") replaces the following while loop by llist_for_each_entry(), - - while (reverse) { - cl = container_of(reverse, struct closure, list); - reverse = llist_next(reverse); - + llist_for_each_entry(cl, reverse, list) { closure_set_waiting(cl, 0); closure_sub(cl, CLOSURE_WAITING + 1); } This modification introduces a potential race by iterating a corrupted list. Here is how it happens. In the above modification, closure_sub() may wake up a process which is waiting on reverse list. If this process decides to wait again by calling closure_wait(), its cl->list will be added to another wait list. Then when llist_for_each_entry() continues to iterate next node, it will travel on another new wait list which is added in closure_wait(), not the original reverse list in __closure_wake_up(). It is more probably to happen on UP machine because the waked up process may preempt the process which wakes up it. Use llist_for_each_entry_safe() will fix the issue, the safe version fetch next node before waking up a process. Then the copy of next node will make sure list iteration stays on original reverse list. Fixes: 09b3efe ("bcache: Don't reinvent the wheel but use existing llist API") Signed-off-by: Coly Li <[email protected]> Reported-by: Michael Lyle <[email protected]> Reviewed-by: Byungchul Park <[email protected]> Signed-off-by: Jens Axboe <[email protected]>
1 parent dc972a6 commit a5f3d8a

File tree

1 file changed

+2
-2
lines changed

1 file changed

+2
-2
lines changed

drivers/md/bcache/closure.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ EXPORT_SYMBOL(closure_put);
6464
void __closure_wake_up(struct closure_waitlist *wait_list)
6565
{
6666
struct llist_node *list;
67-
struct closure *cl;
67+
struct closure *cl, *t;
6868
struct llist_node *reverse = NULL;
6969

7070
list = llist_del_all(&wait_list->list);
@@ -73,7 +73,7 @@ void __closure_wake_up(struct closure_waitlist *wait_list)
7373
reverse = llist_reverse_order(list);
7474

7575
/* Then do the wakeups */
76-
llist_for_each_entry(cl, reverse, list) {
76+
llist_for_each_entry_safe(cl, t, reverse, list) {
7777
closure_set_waiting(cl, 0);
7878
closure_sub(cl, CLOSURE_WAITING + 1);
7979
}

0 commit comments

Comments
 (0)