Re: [PATCH] mm: Fix a NULL dereference crash while accessing bdev->bd_disk

[Jan Kara <jack@suse.cz>]

On Tue 29-11-16 11:43:50, Tejun Heo wrote:
> Hello, Jan.
> 
> On Tue, Nov 29, 2016 at 10:30:35AM +0100, Jan Kara wrote:
> > > It's kinda weird that sync() is ends up accessing bdev's without any
> > > synchronization.  Can't we just make iterate_bdevs() grab bd_mutex and
> > > verify bd_disk isn't NULL before calling into the callback?
> > 
> > This reminded me I've already seen something like this and indeed I've
> > already had a very similar discussion in March -
> > https://patchwork.kernel.org/patch/8556941/
> 
> lol
> 
> > Holding bd_mutex in iterate_devs() works but still nothing protects from
> > flusher thread just walking across the block device inode and trying to
> > write it which would result in the very same oops.
> 
> Ah, right.  We aren't implementing either sever or refcnted draining
> semantics properly.  I wonder whether we'd be able to retire the inode
> synchronously during blkdev_put.

Yeah, I've realized flusher thread is mostly taken care of by the fact that
__blkdev_put() does bdev_write_inode() which waits for I_SYNC to get
cleared and then the inode is clean so writeback code mostly ignores it. It
is fragile but likely it works. So for now I've decided to just push the
patch mentioned above to get at least obvious breakage fixed as playing
with bdev lifetime rules definitely won't be a stable material anyway.

I was also thinking about completely tearing down bdev inode in
__blkdev_put() and it could be doable although hunting all inodes
referencing bdev inode through i_bdev will be tricky. Probably we'll need
i_devices things for block devices back...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>