Re: request_queue use-after-free - inode_detach_wb()

From: Tejun Heo
Date: Wed Nov 18 2015 - 10:30:48 EST


Hello, Ilya.

On Wed, Nov 18, 2015 at 04:12:07PM +0100, Ilya Dryomov wrote:
> > It's stinky that the bdi is going away while the inode is still there.
> > Yeah, blkdev inodes are special and created early but I think it makes
> > sense to keep the underlying structures (queue and bdi) around while
> > bdev is associated with it. Would simply moving put_disk() after
> > bdput() work?
>
> I'd think so. struct block_device is essentially a "block device"
> pseudo-filesystem inode, and as such, may not be around during the
> entire lifetime of gendisk / queue. It may be kicked out of the inode
> cache as soon as the device is closed, so it makes sense to put it
> before putting gendisk / queue, which will outlive it.
>
> However, I'm confused by this comment
>
> /*
> * ->release can cause the queue to disappear, so flush all
> * dirty data before.
> */
> bdev_write_inode(bdev);
>
> It's not true, at least since your 523e1d399ce0 ("block: make gendisk
> hold a reference to its queue"), right? (It used to say "->release can
> cause the old bdi to disappear, so must switch it out first" and was
> changed by Christoph in the middle of his backing_dev_info series.)

Right, it started with each layer going away separately, which tends
to get tricky with hotunplug, and we've been gradually moving towards
a model where the entire stack stays till the last ref is gone, so
yeah the comment isn't true anymore.

Thanks.

--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/