lkml.org 
[lkml]   [2018]   [Jun]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Subject[PATCH] bdi: Fix another oops in wb_workfn()
    From
    Date
    From 014c4149f2e24cd26b278b32d5dfda056eecf093 Mon Sep 17 00:00:00 2001
    From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
    Date: Sat, 9 Jun 2018 22:47:52 +0900
    Subject: [PATCH] bdi: Fix another oops in wb_workfn()

    syzbot is reporting NULL pointer dereference at wb_workfn() [1] due to
    wb->bdi->dev being NULL. And Dmitry confirmed that wb->state was
    WB_shutting_down after wb->bdi->dev became NULL. This indicates that
    unregister_bdi() failed to call wb_shutdown() on one of wb objects.

    Since cgwb_bdi_unregister() from bdi_unregister() cannot call wb_shutdown()
    on wb objects which have already passed list_del_rcu() in wb_shutdown(),
    cgwb_bdi_unregister() from bdi_unregister() can return and set wb->bdi->dev
    to NULL before such wb objects enter final round of wb_workfn() via
    mod_delayed_work()/flush_delayed_work().

    Since WB_registered is already cleared by wb_shutdown(), only wb_shutdown()
    can schedule for final round of wb_workfn(). Since concurrent calls to
    wb_shutdown() on the same wb object is safe because of WB_shutting_down
    state, I think that wb_shutdown() can safely keep a wb object in the
    bdi->wb_list until that wb object leaves final round of wb_workfn().
    Thus, make wb_shutdown() call list_del_rcu() after flush_delayed_work().

    [1] https://syzkaller.appspot.com/bug?id=e0818ccb7e46190b3f1038b0c794299208ed4206

    Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
    Reported-by: syzbot <syzbot+4a7438e774b21ddd8eca@syzkaller.appspotmail.com>
    Cc: Dmitry Vyukov <dvyukov@google.com>
    Cc: Tejun Heo <tj@kernel.org>
    Cc: Jan Kara <jack@suse.cz>
    Cc: Jens Axboe <axboe@fb.com>
    ---
    mm/backing-dev.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

    diff --git a/mm/backing-dev.c b/mm/backing-dev.c
    index 347cc83..bef4b25 100644
    --- a/mm/backing-dev.c
    +++ b/mm/backing-dev.c
    @@ -370,7 +370,6 @@ static void wb_shutdown(struct bdi_writeback *wb)
    set_bit(WB_shutting_down, &wb->state);
    spin_unlock_bh(&wb->work_lock);

    - cgwb_remove_from_bdi_list(wb);
    /*
    * Drain work list and shutdown the delayed_work. !WB_registered
    * tells wb_workfn() that @wb is dying and its work_list needs to
    @@ -379,6 +378,7 @@ static void wb_shutdown(struct bdi_writeback *wb)
    mod_delayed_work(bdi_wq, &wb->dwork, 0);
    flush_delayed_work(&wb->dwork);
    WARN_ON(!list_empty(&wb->work_list));
    + cgwb_remove_from_bdi_list(wb);
    /*
    * Make sure bit gets cleared after shutdown is finished. Matches with
    * the barrier provided by test_and_clear_bit() above.
    --
    1.8.3.1

    \
     
     \ /
      Last update: 2018-06-09 16:01    [W:2.999 / U:0.116 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site