On Sat, Apr 21, 2018 at 03:03:09PM +0200, Jan Kara wrote:
> On Wed 18-04-18 14:08:26, Dave Chinner wrote:
> > From: Dave Chinner <[email protected]>
> >
> > Currently iomap_dio_rw() only handles (data)sync write completions
> > for AIO. This means we can't optimised non-AIO IO to minimise device
> > flushes as we can't tell the caller whether a flush is required or
> > not.
> >
> > To solve this problem and enable further optimisations, make
> > iomap_dio_rw responsible for data sync behaviour for all IO, not
> > just AIO.
> >
> > In doing so, the sync operation is now accounted as part of the DIO
> > IO by inode_dio_end(), hence post-IO data stability updates will no
> > long race against operations that serialise via inode_dio_wait()
> > such as truncate or hole punch.
> >
> > Signed-Off-By: Dave Chinner <[email protected]>
> > Reviewed-by: Christoph Hellwig <[email protected]>
>
> Looks good to me. You can add:
>
> Reviewed-by: Jan Kara <[email protected]>
It looks good, but it's broken in a subtle, nasty way. :/
> > @@ -768,14 +776,8 @@ static ssize_t iomap_dio_complete(struct iomap_dio
> > *dio)
> > static void iomap_dio_complete_work(struct work_struct *work)
> > {
> > struct iomap_dio *dio = container_of(work, struct iomap_dio, aio.work);
> > - struct kiocb *iocb = dio->iocb;
> > - bool is_write = (dio->flags & IOMAP_DIO_WRITE);
> > - ssize_t ret;
> >
> > - ret = iomap_dio_complete(dio);
> > - if (is_write && ret > 0)
> > - ret = generic_write_sync(iocb, ret);
> > - iocb->ki_complete(iocb, ret, 0);
> > + dio->iocb->ki_complete(dio->iocb, iomap_dio_complete(dio), 0);
This generates a use after free from KASAN from generic/016. it
appears the compiler orders the code so that it dereferences
dio->iocb after iomap_dio_complete() has freed the dio structure
(yay!).
I'll post a new version of the patchset now that I've got changes to
2 of the 3 remaining patches in it....
Cheers,
Dave.
--
Dave Chinner
[email protected]
