From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757217AbcJXGgl (ORCPT ); Mon, 24 Oct 2016 02:36:41 -0400 Received: from verein.lst.de ([213.95.11.211]:35720 "EHLO newverein.lst.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757085AbcJXGgj (ORCPT ); Mon, 24 Oct 2016 02:36:39 -0400 Date: Mon, 24 Oct 2016 08:36:37 +0200 From: Christoph Hellwig To: viro@zeniv.linux.org.uk Cc: jack@suse.cz, dmonakhov@openvz.org, linux-fsdevel@vger.kernel.org, linux-aio@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] aio: fix a use after free (and fix freeze protection of aio writes) Message-ID: <20161024063637.GA26837@lst.de> References: <1476597082-15317-1-git-send-email-hch@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1476597082-15317-1-git-send-email-hch@lst.de> User-Agent: Mutt/1.5.17 (2007-11-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > 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 > Signed-off-by: Jan Kara > [hch: switch to use __sb_writers_acquired and file_inode(file), > updated changelog] > Signed-off-by: Christoph Hellwig > --- > 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ---end quoted text---