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
| ||
|
Date: Fri, 16 May 2008 19:28:13 +0900 From: Hidehiro Kawai <hidehiro.kawai.ez@...achi.com> To: Josef Bacik <jbacik@...hat.com> Cc: Jan Kara <jack@...e.cz>, Andrew Morton <akpm@...ux-foundation.org>, sct@...hat.com, adilger@...sterfs.com, linux-kernel@...r.kernel.org, linux-ext4@...r.kernel.org, Mingming Cao <cmm@...ibm.com>, Satoshi OSHIMA <satoshi.oshima.fk@...achi.com>, sugita <yumiko.sugita.yf@...achi.com> Subject: Re: [PATCH 4/4] jbd: fix error handling for checkpoint io (rebased) Hi, Thank you for review. Josef Bacik wrote: > On Wed, May 14, 2008 at 04:44:10PM +0200, Jan Kara wrote: > >>>> >>>>Index: linux-2.6.26-rc2/fs/ext3/super.c >>>>=================================================================== >>>>--- linux-2.6.26-rc2.orig/fs/ext3/super.c >>>>+++ linux-2.6.26-rc2/fs/ext3/super.c >>>>@@ -395,7 +395,10 @@ static void ext3_put_super (struct super >>>> ext3_xattr_put_super(sb); >>>> journal_destroy(sbi->s_journal); >>>> if (!(sb->s_flags & MS_RDONLY)) { >>>>- EXT3_CLEAR_INCOMPAT_FEATURE(sb, EXT3_FEATURE_INCOMPAT_RECOVER); >>>>+ if (!is_checkpoint_aborted(sbi->s_journal)) { >>>>+ EXT3_CLEAR_INCOMPAT_FEATURE(sb, >>>>+ EXT3_FEATURE_INCOMPAT_RECOVER); >>>>+ } >>>> es->s_state = cpu_to_le16(sbi->s_mount_state); >>>> BUFFER_TRACE(sbi->s_sbh, "marking dirty"); >>>> mark_buffer_dirty(sbi->s_sbh); >>> >>>Is this bit here really needed? If we abort the journal the fs will be mounted >>>read only and we should never get in here. Is there a case where we could abort >>>the journal and not be flipped to being read-only? If there is such a case I >>>would think that we should fix that by making the fs read-only, and not have >>>this check. >> >> Actually, journal_abort() (which could be called from journal_destroy()) >>does nothing to the filesystem as such. So at this moment, ext3 can still >>happily think everything is OK. We only detect aborted journal in >>ext3_journal_start_sb() and call ext3_abort() in that case, which does all >>that is needed... Yes, that is why I added this check. > Hmm you're right, I was thinking we did some other stuff before put_super which > would have caught a journal abort but it looks like thats not the case. Still > shouldn't do is_checkpoint_aborted(sbi->s_journal) since journal_destroy() > kfree's the journal. Should probably move the is_journal_aborted() check above > that or something. Thanks, Good catch, I will fix it. Thanks! -- Hidehiro Kawai Hitachi, Systems Development Laboratory Linux Technology Center -- 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