From: Jan Kara <jack@suse.cz>
To: <linux-fsdevel@vger.kernel.org>
Cc: Matthew Wilcox <willy@infradead.org>,
<linux-ext4@vger.kernel.org>, Jan Kara <jack@suse.cz>,
stable@vger.kernel.org, Amir Goldstein <amir73il@gmail.com>
Subject: [PATCH 3/3] ext4: Fix stale data exposure when read races with hole punch
Date: Wed, 20 Jan 2021 17:06:11 +0100 [thread overview]
Message-ID: <20210120160611.26853-4-jack@suse.cz> (raw)
In-Reply-To: <20210120160611.26853-1-jack@suse.cz>
Hole puching currently evicts pages from page cache and then goes on to
remove blocks from the inode. This happens under both i_mmap_sem and
i_rwsem held exclusively which provides appropriate serialization with
racing page faults. However there is currently nothing that prevents
ordinary read(2) from racing with the hole punch and instantiating page
cache page after hole punching has evicted page cache but before it has
removed blocks from the inode. This page cache page will be mapping soon
to be freed block and that can lead to returning stale data to userspace
or even filesystem corruption.
Fix the problem by protecting reads as well as readahead requests with
i_mmap_sem.
CC: stable@vger.kernel.org
Reported-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/ext4/file.c | 16 ++++++++++++++++
fs/ext4/inode.c | 21 +++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 349b27f0dda0..d66f7c08b123 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -30,6 +30,7 @@
#include <linux/uio.h>
#include <linux/mman.h>
#include <linux/backing-dev.h>
+#include <linux/fadvise.h>
#include "ext4.h"
#include "ext4_jbd2.h"
#include "xattr.h"
@@ -131,6 +132,20 @@ static ssize_t ext4_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
return generic_file_read_iter(iocb, to);
}
+static int ext4_fadvise(struct file *filp, loff_t start, loff_t end, int advice)
+{
+ struct inode *inode = file_inode(filp);
+ int ret;
+
+ /* Readahead needs protection from hole punching and similar ops */
+ if (advice == POSIX_FADV_WILLNEED)
+ down_read(&EXT4_I(inode)->i_mmap_sem);
+ ret = generic_fadvise(filp, start, end, advice);
+ if (advice == POSIX_FADV_WILLNEED)
+ up_read(&EXT4_I(inode)->i_mmap_sem);
+ return ret;
+}
+
/*
* Called when an inode is released. Note that this is different
* from ext4_file_open: open gets called at every open, but release
@@ -911,6 +926,7 @@ const struct file_operations ext4_file_operations = {
.splice_read = generic_file_splice_read,
.splice_write = iter_file_splice_write,
.fallocate = ext4_fallocate,
+ .fadvise = ext4_fadvise,
};
const struct inode_operations ext4_file_inode_operations = {
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index c173c8405856..a01569f2fa49 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3646,9 +3646,28 @@ static int ext4_iomap_swap_activate(struct swap_info_struct *sis,
&ext4_iomap_report_ops);
}
+static int ext4_fill_pages(struct kiocb *iocb, size_t len, bool partial_page,
+ struct page **pagep, unsigned int nr_pages)
+{
+ struct ext4_inode_info *ei = EXT4_I(iocb->ki_filp->f_mapping->host);
+ int ret;
+
+ /*
+ * Protect adding of pages into page cache against hole punching and
+ * other cache manipulation functions so that we don't expose
+ * potentially stale user data.
+ */
+ down_read(&ei->i_mmap_sem);
+ ret = generic_file_buffered_read_get_pages(iocb, len, partial_page,
+ pagep, nr_pages);
+ up_read(&ei->i_mmap_sem);
+ return ret;
+}
+
static const struct address_space_operations ext4_aops = {
.readpage = ext4_readpage,
.readahead = ext4_readahead,
+ .fill_pages = ext4_fill_pages,
.writepage = ext4_writepage,
.writepages = ext4_writepages,
.write_begin = ext4_write_begin,
@@ -3667,6 +3686,7 @@ static const struct address_space_operations ext4_aops = {
static const struct address_space_operations ext4_journalled_aops = {
.readpage = ext4_readpage,
.readahead = ext4_readahead,
+ .fill_pages = ext4_fill_pages,
.writepage = ext4_writepage,
.writepages = ext4_writepages,
.write_begin = ext4_write_begin,
@@ -3684,6 +3704,7 @@ static const struct address_space_operations ext4_journalled_aops = {
static const struct address_space_operations ext4_da_aops = {
.readpage = ext4_readpage,
.readahead = ext4_readahead,
+ .fill_pages = ext4_fill_pages,
.writepage = ext4_writepage,
.writepages = ext4_writepages,
.write_begin = ext4_da_write_begin,
--
2.26.2
next prev parent reply other threads:[~2021-01-20 16:08 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-20 16:06 [PATCH 0/3 RFC] fs: Hole punch vs page cache filling races Jan Kara
2021-01-20 16:06 ` [PATCH 1/3] mm: Do not pass iter into generic_file_buffered_read_get_pages() Jan Kara
2021-01-20 16:18 ` Christoph Hellwig
2021-01-20 16:06 ` [PATCH 2/3] mm: Provide address_space operation for filling pages for read Jan Kara
2021-01-20 16:20 ` Christoph Hellwig
2021-01-20 17:27 ` Jan Kara
2021-01-20 17:28 ` Christoph Hellwig
2021-01-20 17:56 ` Matthew Wilcox
2021-04-02 21:17 ` Kent Overstreet
2021-04-06 12:21 ` Jan Kara
2021-01-20 16:06 ` Jan Kara [this message]
2021-01-21 19:27 ` [PATCH 0/3 RFC] fs: Hole punch vs page cache filling races Matthew Wilcox
2021-01-22 14:32 ` Jan Kara
2021-04-02 19:34 ` Theodore Ts'o
2021-04-06 12:17 ` Jan Kara
2021-04-06 16:45 ` Theodore Ts'o
2021-04-06 16:50 ` Theodore Ts'o
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210120160611.26853-4-jack@suse.cz \
--to=jack@suse.cz \
--cc=amir73il@gmail.com \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).