• Coly Li's avatar
    bcache: use llist_for_each_entry_safe() in __closure_wake_up() · a5f3d8a5
    Coly Li authored
    Commit 09b3efec ("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: 09b3efec ("bcache: Don't reinvent the wheel but use existing llist API")
    Signed-off-by: default avatarColy Li <colyli@suse.de>
    Reported-by: default avatarMichael Lyle <mlyle@lyle.org>
    Reviewed-by: default avatarByungchul Park <byungchul.park@lge.com>
    Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
    a5f3d8a5
closure.c 4.7 KB