Re: [PATCH] aio: fix a user triggered use after free (and fix freeze protection of aio writes)

[Al Viro <viro@ZenIV.linux.org.uk>]

On Sat, Oct 29, 2016 at 05:12:30PM +0100, Al Viro wrote:

> NAK, with apologies for not having looked at that earlier.  The bug is real,
> all right, but this is not a solution - both incomplete and far too brittle.
> 
> Why do we play that kind of insane games, anyway?  Why not e.g. refcount
> the (async) iocb and have kiocb_free() drop the reference, with io_submit_one()
> holding one across the call of aio_run_iocb()?  Cacheline bouncing issues?
> Anything more subtle?

PS: I'm not saying that refcounting kiocb is the best solution - grabbing an
extra reference to struct file might be better (we have just dirtied that
cacheline, so the fact that struct file is shared more than kiocb shouldn't
matter much), but I really think that "file and everything attached to it
might disappear as soon as you get async IO started" is insanely brittle -
e.g. xfs_rw_iunlock(ip, iolock) in xfs_file_dio_aio_write() is also unsafe
<checks -next - yup, still there>

If struct file might be gone, so might struct inode and everything behind
it.  Which means that we either are not allowed to hold any locks across
__blockdev_direct_IO(), or need have end_io() callback taking care of
dropping those (and adjust the callers accordingly).  It might be not
impossible, but... *ouch*