From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:59552 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756739AbcJQUFo (ORCPT ); Mon, 17 Oct 2016 16:05:44 -0400 From: Jeff Moyer To: Christoph Hellwig Cc: viro@zeniv.linux.org.uk, 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) References: <1476597082-15317-1-git-send-email-hch@lst.de> Date: Mon, 17 Oct 2016 16:05:41 -0400 In-Reply-To: <1476597082-15317-1-git-send-email-hch@lst.de> (Christoph Hellwig's message of "Sun, 16 Oct 2016 07:51:22 +0200") Message-ID: MIME-Version: 1.0 Content-Type: text/plain Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Christoph Hellwig writes: > 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 Reviewed-by: Jeff Moyer