From patchwork Thu Nov 21 16:15:34 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kara X-Patchwork-Id: 11256505 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 0E7E313A4 for ; Thu, 21 Nov 2019 16:15:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E318720672 for ; Thu, 21 Nov 2019 16:15:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726676AbfKUQPl (ORCPT ); Thu, 21 Nov 2019 11:15:41 -0500 Received: from mx2.suse.de ([195.135.220.15]:55646 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726293AbfKUQPl (ORCPT ); Thu, 21 Nov 2019 11:15:41 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 6574CB391; Thu, 21 Nov 2019 16:15:39 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id D8F961E4360; Thu, 21 Nov 2019 17:15:38 +0100 (CET) From: Jan Kara To: "Darrick J. Wong" Cc: , Christoph Hellwig , Matthew Bobrowski , Eric Biggers , Jan Kara , stable@vger.kernel.org Subject: [PATCH 1/2] iomap: Fix pipe page leakage during splicing Date: Thu, 21 Nov 2019 17:15:34 +0100 Message-Id: <20191121161538.18445-1-jack@suse.cz> X-Mailer: git-send-email 2.16.4 In-Reply-To: <20191121161144.30802-1-jack@suse.cz> References: <20191121161144.30802-1-jack@suse.cz> Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org When splicing using iomap_dio_rw() to a pipe, we may leak pipe pages because bio_iov_iter_get_pages() records that the pipe will have full extent worth of data however if file size is not block size aligned iomap_dio_rw() returns less than what bio_iov_iter_get_pages() set up and splice code gets confused leaking a pipe page with the file tail. Handle the situation similarly to the old direct IO implementation and revert iter to actually returned read amount which makes iter consistent with value returned from iomap_dio_rw() and thus the splice code is happy. Fixes: ff6a9292e6f6 ("iomap: implement direct I/O") CC: stable@vger.kernel.org Reported-by: syzbot+991400e8eba7e00a26e1@syzkaller.appspotmail.com Signed-off-by: Jan Kara Reviewed-by: Darrick J. Wong Reviewed-by: Christoph Hellwig --- fs/iomap/direct-io.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c index 1fc28c2da279..30189652c560 100644 --- a/fs/iomap/direct-io.c +++ b/fs/iomap/direct-io.c @@ -497,8 +497,15 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, } pos += ret; - if (iov_iter_rw(iter) == READ && pos >= dio->i_size) + if (iov_iter_rw(iter) == READ && pos >= dio->i_size) { + /* + * We will report we've read data only upto i_size. + * Revert iter to a state corresponding to that as + * some callers (such as splice code) rely on it. + */ + iov_iter_revert(iter, pos - dio->i_size); break; + } } while ((count = iov_iter_count(iter)) > 0); blk_finish_plug(&plug); From patchwork Thu Nov 21 16:15:35 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kara X-Patchwork-Id: 11256503 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id BE200138C for ; Thu, 21 Nov 2019 16:15:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A8C1F20643 for ; Thu, 21 Nov 2019 16:15:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726690AbfKUQPl (ORCPT ); Thu, 21 Nov 2019 11:15:41 -0500 Received: from mx2.suse.de ([195.135.220.15]:55648 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726379AbfKUQPl (ORCPT ); Thu, 21 Nov 2019 11:15:41 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 73681B396; Thu, 21 Nov 2019 16:15:39 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id DC84A1E4AFB; Thu, 21 Nov 2019 17:15:38 +0100 (CET) From: Jan Kara To: "Darrick J. Wong" Cc: , Christoph Hellwig , Matthew Bobrowski , Eric Biggers , Jan Kara Subject: [PATCH 2/2] iomap: Do not create fake iter in iomap_dio_bio_actor() Date: Thu, 21 Nov 2019 17:15:35 +0100 Message-Id: <20191121161538.18445-2-jack@suse.cz> X-Mailer: git-send-email 2.16.4 In-Reply-To: <20191121161144.30802-1-jack@suse.cz> References: <20191121161144.30802-1-jack@suse.cz> Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org iomap_dio_bio_actor() copies iter to a local variable and then limits it to a file extent we have mapped. When IO is submitted, iomap_dio_bio_actor() advances the original iter while the copied iter is advanced inside bio_iov_iter_get_pages(). This logic is non-obvious especially because both iters still point to same shared structures (such as pipe info) so if iov_iter_advance() changes anything in the shared structure, this scheme breaks. Let's just truncate and reexpand the original iter as needed instead of playing games with copying iters and keeping them in sync. Signed-off-by: Jan Kara Reviewed-by: Darrick J. Wong --- fs/iomap/direct-io.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c index 30189652c560..01a4264bce37 100644 --- a/fs/iomap/direct-io.c +++ b/fs/iomap/direct-io.c @@ -201,12 +201,12 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length, unsigned int blkbits = blksize_bits(bdev_logical_block_size(iomap->bdev)); unsigned int fs_block_size = i_blocksize(inode), pad; unsigned int align = iov_iter_alignment(dio->submit.iter); - struct iov_iter iter; struct bio *bio; bool need_zeroout = false; bool use_fua = false; int nr_pages, ret = 0; size_t copied = 0; + size_t orig_count = iov_iter_count(dio->submit.iter); if ((pos | length | align) & ((1 << blkbits) - 1)) return -EINVAL; @@ -235,16 +235,14 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length, use_fua = true; } - /* - * Operate on a partial iter trimmed to the extent we were called for. - * We'll update the iter in the dio once we're done with this extent. - */ - iter = *dio->submit.iter; - iov_iter_truncate(&iter, length); + /* Operate on a partial iter trimmed to the extent we were called for */ + iov_iter_truncate(dio->submit.iter, length); - nr_pages = iov_iter_npages(&iter, BIO_MAX_PAGES); - if (nr_pages <= 0) + nr_pages = iov_iter_npages(dio->submit.iter, BIO_MAX_PAGES); + if (nr_pages <= 0) { + iov_iter_reexpand(dio->submit.iter, orig_count); return nr_pages; + } if (need_zeroout) { /* zero out from the start of the block to the write offset */ @@ -257,6 +255,7 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length, size_t n; if (dio->error) { iov_iter_revert(dio->submit.iter, copied); + iov_iter_reexpand(dio->submit.iter, orig_count); return 0; } @@ -268,7 +267,7 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length, bio->bi_private = dio; bio->bi_end_io = iomap_dio_bio_end_io; - ret = bio_iov_iter_get_pages(bio, &iter); + ret = bio_iov_iter_get_pages(bio, dio->submit.iter); if (unlikely(ret)) { /* * We have to stop part way through an IO. We must fall @@ -294,13 +293,11 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length, bio_set_pages_dirty(bio); } - iov_iter_advance(dio->submit.iter, n); - dio->size += n; pos += n; copied += n; - nr_pages = iov_iter_npages(&iter, BIO_MAX_PAGES); + nr_pages = iov_iter_npages(dio->submit.iter, BIO_MAX_PAGES); iomap_dio_submit_bio(dio, iomap, bio); } while (nr_pages); @@ -318,6 +315,8 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length, if (pad) iomap_dio_zero(dio, iomap, pos, fs_block_size - pad); } + /* Undo iter limitation to current extent */ + iov_iter_reexpand(dio->submit.iter, orig_count - copied); return copied ? copied : ret; }