Al,
any chance to send this user triggerable use after free on to Linus?
On Sun, Oct 16, 2016 at 07:51:22AM +0200, Christoph Hellwig wrote:
> From: Jan Kara <j...@suse.cz>
>
> Currently we dropped freeze protection of aio writes just after IO was
> submitted. Thus aio write could be in flight while the filesystem was
> frozen and that could result in unexpected situation like aio completion
> wanting to convert extent type on frozen filesystem. Testcase from
> Dmitry triggering this is like:
>
> for ((i=0;i<60;i++));do fsfreeze -f /mnt ;sleep 1;fsfreeze -u /mnt;done &
> fio --bs=4k --ioengine=libaio --iodepth=128 --size=1g --direct=1 \
> --runtime=60 --filename=/mnt/file --name=rand-write --rw=randwrite
>
> Fix the problem by dropping freeze protection only once IO is completed
> in aio_complete().
>
> [hch: The above was the changelog of the original patch from Jan.
> It turns out that it fixes something even more important - a use
> after free of the file structucture given that the direct I/O
> code calls fput and potentially drops the last reference to it in
> aio_complete. Together with two racing threads and a zero sized
> I/O this seems easily exploitable]
>
> Reported-by: Dmitry Monakhov <dmonak...@openvz.org>
> Signed-off-by: Jan Kara <j...@suse.cz>
> [hch: switch to use __sb_writers_acquired and file_inode(file),
> updated changelog]
> Signed-off-by: Christoph Hellwig <h...@lst.de>
> ---
> fs/aio.c | 28 +++++++++++++++++++++++++---
> include/linux/fs.h | 1 +
> 2 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 1157e13..bf315cd 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1078,6 +1078,17 @@ static void aio_complete(struct kiocb *kiocb, long
> res, long res2)
> unsigned tail, pos, head;
> unsigned long flags;
>
> + if (kiocb->ki_flags & IOCB_WRITE) {
> + struct file *file = kiocb->ki_filp;
> +
> + /*
> + * Tell lockdep we inherited freeze protection from submission
> + * thread.
> + */
> + __sb_writers_acquired(file_inode(file)->i_sb, SB_FREEZE_WRITE);
> + file_end_write(file);
> + }
> +
> /*
> * Special case handling for sync iocbs:
> * - events go directly into the iocb for fast handling
> @@ -1460,13 +1471,24 @@ static ssize_t aio_run_iocb(struct kiocb *req,
> unsigned opcode,
> return ret;
> }
>
> - if (rw == WRITE)
> + if (rw == WRITE) {
> file_start_write(file);
> + req->ki_flags |= IOCB_WRITE;
> + }
> +
> + if (rw == WRITE) {
> + /*
> + * We release freeze protection in aio_complete(). Fool
> + * lockdep by telling it the lock got released so that
> + * it doesn't complain about held lock when we return
> + * to userspace.
> + */
> + __sb_writers_release(file_inode(file)->i_sb,
> + SB_FREEZE_WRITE);
> + }
>
> ret = iter_op(req, &iter);
>
> - if (rw == WRITE)
> - file_end_write(file);
> kfree(iovec);
> break;
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 16d2b6e..db600e9 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -321,6 +321,7 @@ struct writeback_control;
> #define IOCB_HIPRI (1 << 3)
> #define IOCB_DSYNC (1 << 4)
> #define IOCB_SYNC (1 << 5)
> +#define IOCB_WRITE (1 << 6)
>
> struct kiocb {
> struct file *ki_filp;
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
---end quoted text---
