lkml.org 
[lkml]   [2021]   [Nov]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] bcache: fix NULL pointer reference in cached_dev_detach_finish
From
Hi Coly,

Since previous patch(faulted) has been merged into upstream, this issue
has to be fix ASAP, else kernel will crash once detach runs, I'm so sorry
for my mistake!

Thanks,
LinFeng

On 11/9/21 12:13, Lin Feng wrote:
> Commit 0259d4498ba484("bcache: move calc_cached_dev_sectors to proper
> place on backing device detach") tries to fix calc_cached_dev_sectors
> when bcache device detaches, but now we have:
>
> cached_dev_detach_finish
> ...
> bcache_device_detach(&dc->disk);
> ...
> closure_put(&d->c->caching);
> d->c = NULL; [*explicitly set dc->disk.c to NULL*]
> list_move(&dc->list, &uncached_devices);
> calc_cached_dev_sectors(dc->disk.c); [*passing a NULL pointer*]
> ...
>
> Upper codeflows shows how bug happens, this patch fix the problem by
> caching dc->disk.c beforehand, and cache_set won't be freed under us
> because c->caching closure at least holds a reference count and closure
> callback __cache_set_unregister only being called by bch_cache_set_stop
> which using closure_queue(&c->caching), that means c->caching closure
> callback for destroying cache_set won't be trigger by previous
> closure_put(&d->c->caching).
> So at this stage(while cached_dev_detach_finish is calling) it's safe to
> access cache_set dc->disk.c.
>
> Fixes: 0259d4498ba484("bcache: move calc_cached_dev_sectors to proper place on backing device detach")
> Signed-off-by: Lin Feng <linf@wangsu.com>
> ---
> drivers/md/bcache/super.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index 4a9a65dff95e..3d9bc7cd27f8 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -1139,6 +1139,7 @@ static void cancel_writeback_rate_update_dwork(struct cached_dev *dc)
> static void cached_dev_detach_finish(struct work_struct *w)
> {
> struct cached_dev *dc = container_of(w, struct cached_dev, detach);
> + struct cache_set *c = dc->disk.c;
>
> BUG_ON(!test_bit(BCACHE_DEV_DETACHING, &dc->disk.flags));
> BUG_ON(refcount_read(&dc->count));
> @@ -1156,7 +1157,7 @@ static void cached_dev_detach_finish(struct work_struct *w)
>
> bcache_device_detach(&dc->disk);
> list_move(&dc->list, &uncached_devices);
> - calc_cached_dev_sectors(dc->disk.c);
> + calc_cached_dev_sectors(c);
>
> clear_bit(BCACHE_DEV_DETACHING, &dc->disk.flags);
> clear_bit(BCACHE_DEV_UNLINK_DONE, &dc->disk.flags);
>

\
 
 \ /
  Last update: 2021-11-10 07:16    [W:0.411 / U:0.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site