linux-ext4 - Re: [BUG] infinite loop when unmounting ext3/4

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: <20150720072356.GC18016@dhcp-13-216.nay.redhat.com>
Date:	Mon, 20 Jul 2015 15:23:56 +0800
From:	Eryu Guan <guaneryu@...il.com>
To:	Joseph Qi <joseph.qi@...wei.com>
Cc:	linux-ext4@...r.kernel.org, Theodore Ts'o <tytso@....edu>
Subject: Re: [BUG] infinite loop when unmounting ext3/4

On Thu, Jul 16, 2015 at 02:18:16PM +0800, Joseph Qi wrote:
> I got the problem.
> I am not sure why old code uses "result <= 0" even if
> it won't return negative value. Could we use "result == 0" instead of
> "result <= 0"?

I thought about this too, but I'm not sure if it has other side effects.
Someone else familiar with this code could comment on this?

Thanks,
Eryu
> 
> On 2015/7/15 22:30, Eryu Guan wrote:
> > Hi all,
> > 
> > I found an infinite loop when unmounting a known bad ext3 image (using
> > ext4 driver) with 4.2-rc1 kernel.
> > 
> > The fs image can be found here
> > https://bugzilla.kernel.org/show_bug.cgi?id=10882#c1
> > 
> > Reproduce steps:
> >   mount -o loop ext3.img /mnt/ext3
> >   rm -rf /mnt/ext3/{dev,proc,sys}
> >   umount /mnt/ext3	# never return
> > 
> > And this issue was introduced by
> > 6f6a6fd jbd2: fix ocfs2 corrupt when updating journal superblock fails
> > 
> > It's looping in
> > fs/jbd2/journal.c:jbd2_journal_destroy()
> > ...
> > 1693         while (journal->j_checkpoint_transactions != NULL) {                                                                             
> > 1694                 spin_unlock(&journal->j_list_lock);                                                                                      
> > 1695                 mutex_lock(&journal->j_checkpoint_mutex);                                                                                
> > 1696                 jbd2_log_do_checkpoint(journal);                                                                                         
> > 1697                 mutex_unlock(&journal->j_checkpoint_mutex);                                                                              
> > 1698                 spin_lock(&journal->j_list_lock);                                                                                        
> > 1699         }
> > ...
> > 
> > Because jbd2_cleanup_journal_tail() is returning -EIO on aborted journal
> > now instead of 1, and jbd2_log_do_checkpoint() won't cleanup
> > journal->j_checkpoint_transactions in this while loop.
> > 
> > A quick hack would be
> > 
> > diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
> > index 4227dc4..1b2ea47 100644
> > --- a/fs/jbd2/checkpoint.c
> > +++ b/fs/jbd2/checkpoint.c
> > @@ -220,11 +220,13 @@ int jbd2_log_do_checkpoint(journal_t *journal)
> >          * don't need checkpointing, just eliminate them from the
> >          * journal straight away.
> >          */
> > -       result = jbd2_cleanup_journal_tail(journal);
> > -       trace_jbd2_checkpoint(journal, result);
> > -       jbd_debug(1, "cleanup_journal_tail returned %d\n", result);
> > -       if (result <= 0)
> > -               return result;
> > +       if (!is_journal_aborted(journal)) {
> > +               result = jbd2_cleanup_journal_tail(journal);
> > +               trace_jbd2_checkpoint(journal, result);
> > +               jbd_debug(1, "cleanup_journal_tail returned %d\n", result);
> > +               if (result <= 0)
> > +                       return result;
> > +       }
> >  
> >         /*
> >          * OK, we need to start writing disk blocks.  Take one transaction
> > 
> > to restore the old behavior (continue on aborted journal). Maybe someone
> > has a better fix.
> > 
> > Thanks,
> > Eryu
> > 
> > .
> > 
> 
> 
--
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