public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ext4: Fix data corruption for mmap writes
@ 2017-05-25 11:46 Jan Kara
  2017-05-26 23:02 ` Theodore Ts'o
  0 siblings, 1 reply; 2+ messages in thread
From: Jan Kara @ 2017-05-25 11:46 UTC (permalink / raw)
  To: Ted Tso; +Cc: linux-ext4, Michael Zimmer, Jan Kara, stable

mpage_submit_page() can race with another process growing i_size and
writing data via mmap to the written-back page. As mpage_submit_page()
samples i_size too early, it may happen that ext4_bio_write_page()
zeroes out too large tail of the page and thus corrupts user data.

Fix the problem by sampling i_size only after the page has been
write-protected in page tables by clear_page_dirty_for_io() call.

Reported-by: Michael Zimmer <michael@swarm64.com>
CC: stable@vger.kernel.org
Fixes: cb20d5188366f04d96d2e07b1240cc92170ade40
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/inode.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 1bd0bfa547f6..14539ce38034 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2124,15 +2124,29 @@ static int ext4_writepage(struct page *page,
 static int mpage_submit_page(struct mpage_da_data *mpd, struct page *page)
 {
 	int len;
-	loff_t size = i_size_read(mpd->inode);
+	loff_t size;
 	int err;
 
 	BUG_ON(page->index != mpd->first_page);
+	clear_page_dirty_for_io(page);
+	/*
+	 * We have to be very careful here!  Nothing protects writeback path
+	 * against i_size changes and the page can be writeably mapped into
+	 * page tables. So an application can be growing i_size and writing
+	 * data through mmap while writeback runs. clear_page_dirty_for_io()
+	 * write-protects our page in page tables and the page cannot get
+	 * written to again until we release page lock. So only after
+	 * clear_page_dirty_for_io() we are safe to sample i_size for
+	 * ext4_bio_write_page() to zero-out tail of the written page. We rely
+	 * on the barrier provided by TestClearPageDirty in
+	 * clear_page_dirty_for_io() to make sure i_size is really sampled only
+	 * after page tables are updated.
+	 */
+	size = i_size_read(mpd->inode);
 	if (page->index == size >> PAGE_SHIFT)
 		len = size & ~PAGE_MASK;
 	else
 		len = PAGE_SIZE;
-	clear_page_dirty_for_io(page);
 	err = ext4_bio_write_page(&mpd->io_submit, page, len, mpd->wbc, false);
 	if (!err)
 		mpd->wbc->nr_to_write--;
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] ext4: Fix data corruption for mmap writes
  2017-05-25 11:46 [PATCH] ext4: Fix data corruption for mmap writes Jan Kara
@ 2017-05-26 23:02 ` Theodore Ts'o
  0 siblings, 0 replies; 2+ messages in thread
From: Theodore Ts'o @ 2017-05-26 23:02 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-ext4, Michael Zimmer, stable

On Thu, May 25, 2017 at 01:46:22PM +0200, Jan Kara wrote:
> mpage_submit_page() can race with another process growing i_size and
> writing data via mmap to the written-back page. As mpage_submit_page()
> samples i_size too early, it may happen that ext4_bio_write_page()
> zeroes out too large tail of the page and thus corrupts user data.
> 
> Fix the problem by sampling i_size only after the page has been
> write-protected in page tables by clear_page_dirty_for_io() call.
> 
> Reported-by: Michael Zimmer <michael@swarm64.com>
> CC: stable@vger.kernel.org
> Fixes: cb20d5188366f04d96d2e07b1240cc92170ade40
> Signed-off-by: Jan Kara <jack@suse.cz>

Thanks, applied.

					- Ted

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-05-26 23:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-25 11:46 [PATCH] ext4: Fix data corruption for mmap writes Jan Kara
2017-05-26 23:02 ` Theodore Ts'o

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox