Re: [bug] 3.9-rc7+next: NULL deref in btrfs_next_old_leaf/btrfs_search_slot

[David Sterba <dsterba@suse.cz>]

On Sat, Apr 27, 2013 at 10:50:43AM +0800, Liu Bo wrote:
> > @@ -3486,14 +3486,14 @@ int close_ctree(struct btrfs_root *root)
> >                        percpu_counter_sum(&fs_info->delalloc_bytes));
> >         }
> > 
> > -       free_root_pointers(fs_info, 1);
> > -
> >         btrfs_free_block_groups(fs_info);
> > 
> >         del_fs_roots(fs_info);
> > 
> >         iput(fs_info->btree_inode);
> > 
> > +       free_root_pointers(fs_info, 1);
> > +
> >         btrfs_stop_workers(&fs_info->generic_worker);
> >         btrfs_stop_workers(&fs_info->fixup_workers);
> >         btrfs_stop_workers(&fs_info->delalloc_workers);
> > ---
> 
> Thanks for tracking it Dave, I'm trying to reproduce it here.

Adding sleeps between free_root_pointers and stopping workers should
leave enough space for the threads to work and touch the freed data.

The proposed fix is not entirely correct, it just reduces the race
window. Freeing roots must come after stopping the workers, same holds
for any of the other cleanup functions that may access released
resources. But for example the free block groups need the caching thread
alive so it cannot be trivially fixed by moving everything after
stop_workers and must be decided case by case.

There is another instance of a similar sequence that is mis-ordered and
has to be fixed as well.


david