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

On Sun, Oct 30, 2016 at 11:52:31AM +0100, Jan Kara wrote: > Hum, the additional refcount patch oopses on me when running generic/323, > I'll have to board to my flight to US shortly so I won't be able to send it > soon - maybe when I'm transferring in Denver ;). I've got a version that works and

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

On Sun, Oct 30, 2016 at 11:52:31AM +0100, Jan Kara wrote: > Hum, the additional refcount patch oopses on me when running generic/323, > I'll have to board to my flight to US shortly so I won't be able to send it > soon - maybe when I'm transferring in Denver ;). I've got a version that works and

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

On Sun 30-10-16 10:44:37, Jan Kara wrote: > On Sat 29-10-16 13:09:25, Linus Torvalds wrote: > > On Sat, Oct 29, 2016 at 12:17 PM, Al Viro wrote: > > > > > > AFAICS, the possibility of dropping the last reference to struct file > > > before ->write_iter() has returned is

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

On Sun 30-10-16 10:44:37, Jan Kara wrote: > On Sat 29-10-16 13:09:25, Linus Torvalds wrote: > > On Sat, Oct 29, 2016 at 12:17 PM, Al Viro wrote: > > > > > > AFAICS, the possibility of dropping the last reference to struct file > > > before ->write_iter() has returned is fundamentally broken. I

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

On Sat 29-10-16 13:09:25, Linus Torvalds wrote: > On Sat, Oct 29, 2016 at 12:17 PM, Al Viro wrote: > > > > AFAICS, the possibility of dropping the last reference to struct file > > before ->write_iter() has returned is fundamentally broken. I might be > > missing

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

On Sat 29-10-16 13:09:25, Linus Torvalds wrote: > On Sat, Oct 29, 2016 at 12:17 PM, Al Viro wrote: > > > > AFAICS, the possibility of dropping the last reference to struct file > > before ->write_iter() has returned is fundamentally broken. I might be > > missing something subtle here, but... >

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

On Sat, Oct 29, 2016 at 05:12:30PM +0100, Al Viro wrote: > Eww... IOW, as soon as we'd submitted an async iocb, nobody can so much as > look at struct file *or* iocb, right? Or underlying inode, or any fs-private > data structures attached to it... Yeah. > I certainly agree that it's a bug,

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

On Sat, Oct 29, 2016 at 05:12:30PM +0100, Al Viro wrote: > Eww... IOW, as soon as we'd submitted an async iocb, nobody can so much as > look at struct file *or* iocb, right? Or underlying inode, or any fs-private > data structures attached to it... Yeah. > I certainly agree that it's a bug,

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

On Sat, Oct 29, 2016 at 10:47:58AM -0700, Linus Torvalds wrote: > On Sat, Oct 29, 2016 at 8:20 AM, Christoph Hellwig wrote: > > > > We can't as that would not fix the use after free (at least for the lockdep > > case - otherwise the call is a no-op). Once iter_op returns

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

On Sat, Oct 29, 2016 at 10:47:58AM -0700, Linus Torvalds wrote: > On Sat, Oct 29, 2016 at 8:20 AM, Christoph Hellwig wrote: > > > > We can't as that would not fix the use after free (at least for the lockdep > > case - otherwise the call is a no-op). Once iter_op returns aio_complete > > might

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

On Sat, Oct 29, 2016 at 12:17 PM, Al Viro wrote: > > AFAICS, the possibility of dropping the last reference to struct file > before ->write_iter() has returned is fundamentally broken. I might be > missing something subtle here, but... Ok, let's add a get_file();

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

On Sat, Oct 29, 2016 at 12:17 PM, Al Viro wrote: > > AFAICS, the possibility of dropping the last reference to struct file > before ->write_iter() has returned is fundamentally broken. I might be > missing something subtle here, but... Ok, let's add a get_file(); fput(); around that whole iter

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

On Sat, Oct 29, 2016 at 12:07:07PM -0700, Linus Torvalds wrote: > On Sat, Oct 29, 2016 at 11:52 AM, Al Viro wrote: > > > > And that call can happen as soon as we return from __blockdev_direct_IO() > > (even earlier, actually). As soon as that happens, the reference to >

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

On Sat, Oct 29, 2016 at 12:07:07PM -0700, Linus Torvalds wrote: > On Sat, Oct 29, 2016 at 11:52 AM, Al Viro wrote: > > > > And that call can happen as soon as we return from __blockdev_direct_IO() > > (even earlier, actually). As soon as that happens, the reference to > > struct file we'd

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

On Sat, Oct 29, 2016 at 11:52 AM, Al Viro wrote: > > And that call can happen as soon as we return from __blockdev_direct_IO() > (even earlier, actually). As soon as that happens, the reference to > struct file we'd acquired in io_submit_one() is dropped. If descriptor

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

On Sat, Oct 29, 2016 at 11:52 AM, Al Viro wrote: > > And that call can happen as soon as we return from __blockdev_direct_IO() > (even earlier, actually). As soon as that happens, the reference to > struct file we'd acquired in io_submit_one() is dropped. If descriptor > table had been shared,

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

On Sat, Oct 29, 2016 at 10:47:58AM -0700, Linus Torvalds wrote: > Also, honestly, make it use a helper: "aio_file_start_write()" and > "aio_file_end_write()" that has the comments and the lockdep games. > > Because that patch is just too effing ugly. > > Does something like the attached work

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

On Sat, Oct 29, 2016 at 10:47:58AM -0700, Linus Torvalds wrote: > Also, honestly, make it use a helper: "aio_file_start_write()" and > "aio_file_end_write()" that has the comments and the lockdep games. > > Because that patch is just too effing ugly. > > Does something like the attached work

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

On Sat, Oct 29, 2016 at 8:20 AM, Christoph Hellwig wrote: > > We can't as that would not fix the use after free (at least for the lockdep > case - otherwise the call is a no-op). Once iter_op returns aio_complete > might have dropped our reference to the file, and another thread

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

On Sat, Oct 29, 2016 at 8:20 AM, Christoph Hellwig wrote: > > We can't as that would not fix the use after free (at least for the lockdep > case - otherwise the call is a no-op). Once iter_op returns aio_complete > might have dropped our reference to the file, and another thread might > have

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

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 >

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

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 >

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

On Sat, Oct 29, 2016 at 05:20:17PM +0200, Christoph Hellwig wrote: > On Sat, Oct 29, 2016 at 01:24:51PM +0100, Al Viro wrote: > > How about taking this chunk (i.e. telling lockdep that we are not holding > > this > > thing) past the iter_op() call, where file_end_write() used to be? > > We can't

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

On Sat, Oct 29, 2016 at 05:20:17PM +0200, Christoph Hellwig wrote: > On Sat, Oct 29, 2016 at 01:24:51PM +0100, Al Viro wrote: > > How about taking this chunk (i.e. telling lockdep that we are not holding > > this > > thing) past the iter_op() call, where file_end_write() used to be? > > We can't

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

On Sat, Oct 29, 2016 at 01:24:51PM +0100, Al Viro wrote: > How about taking this chunk (i.e. telling lockdep that we are not holding this > thing) past the iter_op() call, where file_end_write() used to be? We can't as that would not fix the use after free (at least for the lockdep case -

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

On Sat, Oct 29, 2016 at 01:24:51PM +0100, Al Viro wrote: > How about taking this chunk (i.e. telling lockdep that we are not holding this > thing) past the iter_op() call, where file_end_write() used to be? We can't as that would not fix the use after free (at least for the lockdep case -

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

On Sat, Oct 29, 2016 at 09:44:29AM +0200, Christoph Hellwig wrote: > - if (rw == WRITE) > + if (rw == WRITE) { > file_start_write(file); > + req->ki_flags |= IOCB_WRITE; > + } > + if (rw == WRITE) { > +

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

On Sat, Oct 29, 2016 at 09:44:29AM +0200, Christoph Hellwig wrote: > - if (rw == WRITE) > + if (rw == WRITE) { > file_start_write(file); > + req->ki_flags |= IOCB_WRITE; > + } > + if (rw == WRITE) { > +