linux-kernel - Re: [PATCH 1/5] jbd: strictly check for write errors on data buffers

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: <484D0155.4040701@hitachi.com>
Date:	Mon, 09 Jun 2008 19:09:25 +0900
From:	Hidehiro Kawai <hidehiro.kawai.ez@...achi.com>
To:	Andreas Dilger <adilger@....com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Theodore Tso <tytso@....edu>, Jan Kara <jack@...e.cz>,
	sct@...hat.com, linux-kernel@...r.kernel.org,
	linux-ext4@...r.kernel.org, jbacik@...hat.com, cmm@...ibm.com,
	yumiko.sugita.yf@...achi.com, satoshi.oshima.fk@...achi.com
Subject: Re: [PATCH 1/5] jbd: strictly check for write errors on data buffers

Andreas Dilger wrote:
> On Jun 05, 2008  09:20 -0700, Andrew Morton wrote:

>>I guess we need to undo this.  And yes, propagating errors into AS_EIO
>>is the way.  I guess that's safe without holding lock_page(), as long
>>as the bh is pinned.
> 
> Something like the following instead if -EIO and journal abort:
> 
> 		if (!buffer_uptodate(bh)) {
> 			set_bit(AS_EIO, &bh->b_page->mapping->flags);
> 			SetPageError(bh->b_page);
> 		}
> 
> It seems end_buffer_async_write() does this already, but
> journal_do_submit_data() uses end_buffer_write_sync() and it does not
> do either of those operations.

Thank you for your suggestion.  I wrote an additional patch to do
that below.  Please apply it as the 6th patch of this patch series.

BTW, I'm developing a patch which makes "abort the journal if a file
data buffer has an error" tunable.  I'll send it in another thread
because it's not a bug fix patch.

Regards,
-- 
Hidehiro Kawai
Hitachi, Systems Development Laboratory
Linux Technology Center


Subject: JBD: don't abort if flushing file data failed

In ordered mode, it is not appropriate behavior to abort the journal
when we failed to write file data.  This patch calls printk()
instead of aborting the journal.  Additionally, set AS_EIO into
the address_space object of the buffer which is written out by
journal_do_submit_data() and failed so that fsync() can get -EIO.

Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@...achi.com>
---
 fs/jbd/commit.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Index: linux-2.6.26-rc4/fs/jbd/commit.c
===================================================================
--- linux-2.6.26-rc4.orig/fs/jbd/commit.c
+++ linux-2.6.26-rc4/fs/jbd/commit.c
@@ -432,8 +432,11 @@ void journal_commit_transaction(journal_
 			wait_on_buffer(bh);
 			spin_lock(&journal->j_list_lock);
 		}
-		if (unlikely(!buffer_uptodate(bh)))
+		if (unlikely(!buffer_uptodate(bh))) {
+			set_bit(AS_EIO, &bh->b_page->mapping->flags);
+			SetPageError(bh->b_page);
 			err = -EIO;
+		}
 		if (!inverted_lock(journal, bh)) {
 			put_bh(bh);
 			spin_lock(&journal->j_list_lock);
@@ -452,8 +455,11 @@ void journal_commit_transaction(journal_
 	}
 	spin_unlock(&journal->j_list_lock);
 
-	if (err)
-		journal_abort(journal, err);
+	if (err) {
+		printk(KERN_WARNING
+		       "JBD: Detected IO errors during flushing file data\n");
+		err = 0;
+	}
 
 	journal_write_revoke_records(journal, commit_transaction);
 



--
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/