From: Jan Kara Subject: Re: [PATCH 1/2] ext3: fix a bug accessing freed memory in ext3_abort Date: Mon, 27 Oct 2008 09:11:08 +0100 Message-ID: <20081027081108.GB23804@atrey.karlin.mff.cuni.cz> References: <19f34abd0810250422t17990222x78fcf0fad2e6f04b@mail.gmail.com> <49056D29.2010700@hitachi.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Vegard Nossum , Theodore Tso , Jan Kara , Stephen Rothwell , Al Viro , linux-ext4@vger.kernel.org, Andrew Morton , Linus Torvalds , "Rafael J. Wysocki" , Ingo Molnar , Pekka Enberg , linux-kernel@vger.kernel.org To: Hidehiro Kawai Return-path: Received: from atrey.karlin.mff.cuni.cz ([195.113.26.193]:33737 "EHLO atrey.karlin.mff.cuni.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751739AbYJ0ILK (ORCPT ); Mon, 27 Oct 2008 04:11:10 -0400 Content-Disposition: inline In-Reply-To: <49056D29.2010700@hitachi.com> Sender: linux-ext4-owner@vger.kernel.org List-ID: > Hi Vegard, > > Vegard Nossum wrote: > > > commit 2d7c820e56ce83b23daee9eb5343730fb309418e > > Author: Hidehiro Kawai > > Date: Wed Oct 22 14:15:01 2008 -0700 > > > > ext3: add checks for errors from jbd > > > > introduces a regression which was discovered by kmemcheck: > > Thank you for reporting this regression. It's my fault. > I send patches to fix. > > Regards, > > > Subject: [PATCH 1/2] ext3: fix a bug accessing freed memory in ext3_abort > > Vegard Nossum reported a bug which accesses freed memory. When > journal has been aborted, ext3_put_super() calls ext3_abort() after > freeing the journal_t object, and then ext3_abort() accesses it. > This patch fix it. > > Signed-off-by: Hidehiro Kawai The patch looks fine. Acked-by: Jan Kara Honza > --- > fs/ext3/super.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > Index: linux-2.6.28-rc2/fs/ext3/super.c > =================================================================== > --- linux-2.6.28-rc2.orig/fs/ext3/super.c > +++ linux-2.6.28-rc2/fs/ext3/super.c > @@ -281,7 +281,8 @@ void ext3_abort (struct super_block * sb > EXT3_SB(sb)->s_mount_state |= EXT3_ERROR_FS; > sb->s_flags |= MS_RDONLY; > EXT3_SB(sb)->s_mount_opt |= EXT3_MOUNT_ABORT; > - journal_abort(EXT3_SB(sb)->s_journal, -EIO); > + if (EXT3_SB(sb)->s_journal) > + journal_abort(EXT3_SB(sb)->s_journal, -EIO); > } > > void ext3_warning (struct super_block * sb, const char * function, > @@ -390,11 +391,14 @@ static void ext3_put_super (struct super > { > struct ext3_sb_info *sbi = EXT3_SB(sb); > struct ext3_super_block *es = sbi->s_es; > - int i; > + int i, err; > > ext3_xattr_put_super(sb); > - if (journal_destroy(sbi->s_journal) < 0) > + err = journal_destroy(sbi->s_journal); > + sbi->s_journal = NULL; > + if (err < 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); > -- Jan Kara SuSE CR Labs