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 Jul 2008 11:44:25 +0900
From:	Hidehiro Kawai <hidehiro.kawai.ez@...achi.com>
To:	akpm@...ux-foundation.org
Cc:	mm-commits@...r.kernel.org, jack@...e.cz,
	linux-ext4@...r.kernel.org, sct@...hat.com, adilger@...sterfs.com,
	linux-kernel@...r.kernel.org, jbacik@...hat.com, cmm@...ibm.com,
	tytso@....edu, snitzer@...il.com, tglx@...utronix.de,
	yumiko.sugita.yf@...achi.com, satoshi.oshima.fk@...achi.com
Subject: Re: + ext3-abort-ext3-if-the-journal-has-aborted.patch added to -mm
    tree

Hi Andrew,

akpm@...ux-foundation.org wrote:

> The patch titled
>      ext3: abort ext3 if the journal has aborted
> has been added to the -mm tree.  Its filename is
>      ext3-abort-ext3-if-the-journal-has-aborted.patch

A problem was found in this patch:

http://lkml.org/lkml/2008/7/28/15
> 4. when checkpointing fails, notify this error to the ext3 layer so
>>   that ext3 don't clear the needs_recovery flag, otherwise the
>>   journaled contents are ignored and cleaned in the recovery phase
>
> Mike Snitzer noticed that ext3_mark_recovery_complete() doesn't
> check checkpointing failure and it clears needs_recovery flag 
> (thanks, Mike!).  I need an additional fix.
>
> I also found ext3_quota_on() forces checkpointing by journal_flush()
> but it doesn't check the error.  This will appear in 2.6.27-rc1,
> so I'll send the revised patch set for 2.6.27-rc1 (it may be
> a separate patch).

I attached the revised version below.  Could you replace the patch
titled `ext3: abort ext3 if the journal has aborted' with this
new one?  Please note that the patch's title also changed.


Subject: [PATCH 3/4] ext3: add checks for errors from jbd

If the journal has aborted due to a checkpointing failure, we
have to keep the contents of the journal space.  Otherwise, the
filesystem will lose uncheckpointed metadata completely and
become inconsistent.  To avoid this, we need to keep needs_recovery
flag if checkpoint has failed.

With this patch, ext3_put_super() detects a checkpointing failure
from the return value of journal_destroy(), then it invokes
ext3_abort() to make the filesystem read only and keep
needs_recovery flag.  Errors from journal_flush() are also handled
by this patch in some places.

Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@...achi.com>
---
 fs/ext3/ioctl.c |   12 ++++++++----
 fs/ext3/super.c |   23 +++++++++++++++++++----
 2 files changed, 27 insertions(+), 8 deletions(-)

Index: linux-2.6.27-rc1/fs/ext3/ioctl.c
===================================================================
--- linux-2.6.27-rc1.orig/fs/ext3/ioctl.c
+++ linux-2.6.27-rc1/fs/ext3/ioctl.c
@@ -239,7 +239,7 @@ setrsvsz_out:
 	case EXT3_IOC_GROUP_EXTEND: {
 		ext3_fsblk_t n_blocks_count;
 		struct super_block *sb = inode->i_sb;
-		int err;
+		int err, err2;
 
 		if (!capable(CAP_SYS_RESOURCE))
 			return -EPERM;
@@ -254,8 +254,10 @@ setrsvsz_out:
 		}
 		err = ext3_group_extend(sb, EXT3_SB(sb)->s_es, n_blocks_count);
 		journal_lock_updates(EXT3_SB(sb)->s_journal);
-		journal_flush(EXT3_SB(sb)->s_journal);
+		err2 = journal_flush(EXT3_SB(sb)->s_journal);
 		journal_unlock_updates(EXT3_SB(sb)->s_journal);
+		if (err == 0)
+			err = err2;
 group_extend_out:
 		mnt_drop_write(filp->f_path.mnt);
 		return err;
@@ -263,7 +265,7 @@ group_extend_out:
 	case EXT3_IOC_GROUP_ADD: {
 		struct ext3_new_group_data input;
 		struct super_block *sb = inode->i_sb;
-		int err;
+		int err, err2;
 
 		if (!capable(CAP_SYS_RESOURCE))
 			return -EPERM;
@@ -280,8 +282,10 @@ group_extend_out:
 
 		err = ext3_group_add(sb, &input);
 		journal_lock_updates(EXT3_SB(sb)->s_journal);
-		journal_flush(EXT3_SB(sb)->s_journal);
+		err2 = journal_flush(EXT3_SB(sb)->s_journal);
 		journal_unlock_updates(EXT3_SB(sb)->s_journal);
+		if (err == 0)
+			err = err2;
 group_add_out:
 		mnt_drop_write(filp->f_path.mnt);
 		return err;
Index: linux-2.6.27-rc1/fs/ext3/super.c
===================================================================
--- linux-2.6.27-rc1.orig/fs/ext3/super.c
+++ linux-2.6.27-rc1/fs/ext3/super.c
@@ -393,7 +393,8 @@ static void ext3_put_super (struct super
 	int i;
 
 	ext3_xattr_put_super(sb);
-	journal_destroy(sbi->s_journal);
+	if (journal_destroy(sbi->s_journal) < 0)
+		ext3_abort(sb, __func__, "Couldn't clean up the journal");
 	if (!(sb->s_flags & MS_RDONLY)) {
 		EXT3_CLEAR_INCOMPAT_FEATURE(sb, EXT3_FEATURE_INCOMPAT_RECOVER);
 		es->s_state = cpu_to_le16(sbi->s_mount_state);
@@ -2280,7 +2281,9 @@ static void ext3_mark_recovery_complete(
 	journal_t *journal = EXT3_SB(sb)->s_journal;
 
 	journal_lock_updates(journal);
-	journal_flush(journal);
+	if (journal_flush(journal) < 0)
+		goto out;
+
 	lock_super(sb);
 	if (EXT3_HAS_INCOMPAT_FEATURE(sb, EXT3_FEATURE_INCOMPAT_RECOVER) &&
 	    sb->s_flags & MS_RDONLY) {
@@ -2289,6 +2292,8 @@ static void ext3_mark_recovery_complete(
 		ext3_commit_super(sb, es, 1);
 	}
 	unlock_super(sb);
+
+out:
 	journal_unlock_updates(journal);
 }
 
@@ -2388,7 +2393,13 @@ static void ext3_write_super_lockfs(stru
 
 		/* Now we set up the journal barrier. */
 		journal_lock_updates(journal);
-		journal_flush(journal);
+
+		/*
+		 * We don't want to clear needs_recovery flag when we failed
+		 * to flush the journal.
+		 */
+		if (journal_flush(journal) < 0)
+			return;
 
 		/* Journal blocked and flushed, clear needs_recovery flag. */
 		EXT3_CLEAR_INCOMPAT_FEATURE(sb, EXT3_FEATURE_INCOMPAT_RECOVER);
@@ -2806,8 +2817,12 @@ static int ext3_quota_on(struct super_bl
 		 * otherwise be livelocked...
 		 */
 		journal_lock_updates(EXT3_SB(sb)->s_journal);
-		journal_flush(EXT3_SB(sb)->s_journal);
+		err = journal_flush(EXT3_SB(sb)->s_journal);
 		journal_unlock_updates(EXT3_SB(sb)->s_journal);
+		if (err) {
+			path_put(&nd.path);
+			return err;
+		}
 	}
 
 	path_put(&nd.path);




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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ