linux-ext4 - [PATCH v2] ext4: fix data integrity sync in ordered mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:	Wed, 30 Apr 2014 19:04:41 +0900
From:	Namjae Jeon <namjae.jeon@...sung.com>
To:	Theodore Ts'o <tytso@....edu>
Cc:	linux-ext4 <linux-ext4@...r.kernel.org>,
	Ashish Sangwan <a.sangwan@...sung.com>, Jan kara <jack@...e.de>
Subject: [PATCH v2] ext4: fix data integrity sync in ordered mode

When we perform a data integrity sync we tag all the dirty pages with
PAGECACHE_TAG_TOWRITE at start of ext4_da_writepages.
Later we check for this tag in write_cache_pages_da and creates a
struct mpage_da_data containing contiguously indexed pages tagged with this
tag and sync these pages with a call to mpage_da_map_and_submit.
This process is done in while loop until all the PAGECACHE_TAG_TOWRITE pages
are synced. We also do journal start and stop in each iteration.
journal_stop could initiate journal commit which would call ext4_writepage
which in turn will call ext4_bio_write_page even for delayed OR unwritten
buffers. When ext4_bio_write_page is called for such buffers, even though it
does not sync them but it clears the PAGECACHE_TAG_TOWRITE of the corresponding
page and hence these pages are also not synced by the currently running data
integrity sync. We will end up with dirty pages although sync is completed.

This could cause a potential data loss when the sync call is followed by a
truncate_pagecache call, which is exactly the case in collapse_range.
(It will cause generic/127 failure in xfstests)

Cc: stable@...r.kernel.org
Cc: Jan kara <jack@...e.de>
Signed-off-by: Namjae Jeon <namjae.jeon@...sung.com>
Signed-off-by: Ashish Sangwan <a.sangwan@...sung.com>
---
 fs/ext4/inode.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index b1dc334..bd85712 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1865,12 +1865,19 @@ static int ext4_writepage(struct page *page,
 	if (ext4_walk_page_buffers(NULL, page_bufs, 0, len, NULL,
 				   ext4_bh_delay_or_unwritten)) {
 		redirty_page_for_writepage(wbc, page);
-		if (current->flags & PF_MEMALLOC) {
+		if ((current->flags & PF_MEMALLOC) ||
+		     radix_tree_tag_get(&page->mapping->page_tree,
+					page->index, PAGECACHE_TAG_TOWRITE)) {
 			/*
 			 * For memory cleaning there's no point in writing only
 			 * some buffers. So just bail out. Warn if we came here
 			 * from direct reclaim.
-			 */
+			 * We should also bail out when a journal commit happen
+			 * during an integrity sync operation because calling
+			 * ext4_bio_write_page in this case will clear
+			 * PAGECACHE_TAG_TOWRITE and we could end up with
+			 * dirty pages even after completion of a sync call.
+			 */
 			WARN_ON_ONCE((current->flags & (PF_MEMALLOC|PF_KSWAPD))
 							== PF_MEMALLOC);
 			unlock_page(page);
-- 
1.7.11-rc0

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html