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]
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-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists