linux-kernel - [PATCH] exofs: simple fsync race fix

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4CEE3FD8.6080202@panasas.com>
Date:	Thu, 25 Nov 2010 12:52:08 +0200
From:	Boaz Harrosh <bharrosh@...asas.com>
To:	Nick Piggin <npiggin@...nel.dk>
CC:	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-ext4@...r.kernel.org, Roman Zippel <zippel@...ux-m68k.org>,
	"Tigran A. Aivazian" <tigran@...azian.fsnet.co.uk>,
	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>,
	Dave Kleikamp <shaggy@...ux.vnet.ibm.com>,
	Bob Copeland <me@...copeland.com>,
	reiserfs-devel@...r.kernel.org,
	Christoph Hellwig <hch@...radead.org>,
	Evgeniy Dushistov <dushistov@...l.ru>, Jan Kara <jack@...e.cz>
Subject: [PATCH] exofs: simple fsync race fix

From: Nick Piggin <npiggin@...nel.dk>

It is incorrect to test inode dirty bits without participating in the inode
writeback protocol. Inode writeback sets I_SYNC and clears I_DIRTY_?, then
writes out the particular bits, then clears I_SYNC when it is done. BTW. it
may not completely write all pages out, so I_DIRTY_PAGES would get set
again.

This is a standard pattern used throughout the kernel's writeback caches
(I_SYNC ~= I_WRITEBACK, if that makes it clearer).

And so it is not possible to determine an inode's dirty status just by
checking I_DIRTY bits. Especially not for the purpose of data integrity
syncs.

Missing the check for these bits means that fsync can complete while
writeback to the inode is underway. Inode writeback functions get this
right, so call into them rather than try to shortcut things by testing
dirty state improperly.

Signed-off-by: Nick Piggin <npiggin@...nel.dk>
Signed-off-by: Boaz Harrosh <bharrosh@...asas.com>
---
 fs/exofs/file.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/fs/exofs/file.c b/fs/exofs/file.c
index b905c79..4c0d6ba 100644
--- a/fs/exofs/file.c
+++ b/fs/exofs/file.c
@@ -48,11 +48,6 @@ static int exofs_file_fsync(struct file *filp, int datasync)
 	struct inode *inode = filp->f_mapping->host;
 	struct super_block *sb;

-	if (!(inode->i_state & I_DIRTY))
-		return 0;
-	if (datasync && !(inode->i_state & I_DIRTY_DATASYNC))
-		return 0;
-
 	ret = sync_inode_metadata(inode, 1);

 	/* This is a good place to write the sb */
-- 
1.7.2.3

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