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

2015-11-19 Thread Tejun Heo
Hello, On Thu, Nov 19, 2015 at 10:56:43PM +0100, Ilya Dryomov wrote: > Detaching the inode earlier is what I suggested in the first email, but > I didn't know if this kind of special casing was OK. I'll try it out. Yeah, I was confused. Sorry about that. On the surface, it looks like a

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

2015-11-18 Thread Tejun Heo
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

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

2015-11-18 Thread Tejun Heo
Hello, Ilya. On Wed, Nov 18, 2015 at 04:48:06PM +0100, Ilya Dryomov wrote: > Just to be clear, the bdi/wb vs inode lifetime rules are that inodes > should always be within bdi/wb? There's been a lot of churn in this Yes, that's where *I* think we should be headed. Stuff in lower layers should

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

2015-11-17 Thread Tejun Heo
Hello, Ilya. On Mon, Nov 16, 2015 at 09:59:18PM +0100, Ilya Dryomov wrote: ... > Looking at __blkdev_put(), the issue becomes clear: we are taking > precautions to flush before calling out to ->release() because, at > least according to the comment, ->release() can free queue; we are > recording

request_queue use-after-free - inode_detach_wb()

2015-11-16 Thread Ilya Dryomov
Hello, Last week, while running an rbd test which does a lot of maps and unmaps (read losetup / losetup -d) with slab debugging enabled, I hit the attached splat. That 6a byte corresponds to the atomic_long_t count of the percpu_ref refcnt in request_queue::backing_dev_info::wb, pointing to a