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:	Wed, 16 Feb 2011 15:56:27 +0100
From:	Jan Kara <jack@...e.cz>
To:	Toshiyuki Okajima <toshi.okajima@...fujitsu.com>
Cc:	Jan Kara <jack@...e.cz>, Ted Ts'o <tytso@....edu>,
	Masayoshi MIZUMA <m.mizuma@...fujitsu.com>,
	Andreas Dilger <adilger.kernel@...ger.ca>,
	linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On Wed 16-02-11 08:17:46, Toshiyuki Okajima wrote:
> On Tue, 15 Feb 2011 18:29:54 +0100
> Jan Kara <jack@...e.cz> wrote:
> > On Tue 15-02-11 12:03:52, Ted Ts'o wrote:
> > > On Tue, Feb 15, 2011 at 05:06:30PM +0100, Jan Kara wrote:
> > > > Thanks for detailed analysis. Indeed this is a bug. Whenever we do IO
> > > > under s_umount semaphore, we are prone to deadlock like the one you
> > > > describe above.
> > > 
> > > One of the fundamental problems here is that the freeze and thaw
> > > routines are using down_write(&sb->s_umount) for two purposes.  The
> > > first is to prevent the resume/thaw from racing with a umount (which
> > > it could do just as well by taking a read lock), but the second is to
> > > prevent the resume/thaw code from racing with itself.  That's the core
> > > fundamental problem here.
> > > 
> > > So I think we can solve this by introduce a new mutex, s_freeze, and
> > > having the the resume/thaw first take the s_freeze mutex and then
> > > second take a read lock on the s_umount.
> >   Sadly this does not quite work because even down_read(&sb->s_umount)
> > in thaw_super() can block if there is another process that tries to acquire
> > s_umount for writing - a situation like:
> >   TASK 1 (e.g. flusher)		TASK 2	(e.g. remount)		TASK 3 (unfreeze)
> > down_read(&sb->s_umount)
> >   block on s_frozen
> > 				down_write(&sb->s_umount)
> > 				  -blocked
> > 								down_read(&sb->s_umount)
> > 								  -blocked
> > behind the write access...
> > 
> > The only working solution I see is to check for frozen filesystem before
> > taking s_umount semaphore which seems rather ugly (but might be bearable if
> > we did so in some well described wrapper).
> I created the patch that you imagine yesterday.
>  
> I got a reproducer from Mizuma-san yesterday, and then I executed it on the kernel
> without a fixed patch. After an hour, I confirmed that this deadlock happened.
> 
> However, on the kernel with a fixed patch, this deadlock doesn't still happen 
> after 12 hours passed.
> 
> The patch for linux-2.6.38-rc4 is as follows:
> ---
>  fs/fs-writeback.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 59c6e49..1c9a05e 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -456,7 +456,7 @@ static bool pin_sb_for_writeback(struct super_block *sb)
>         spin_unlock(&sb_lock);
> 
>         if (down_read_trylock(&sb->s_umount)) {
> -               if (sb->s_root)
> +               if (sb->s_frozen == SB_UNFROZEN && sb->s_root)
>                         return true;
>                 up_read(&sb->s_umount);
  So this is something along the lines I thought but it actually won't work
for example if sync(1) is run while the filesystem is frozen (that takes
s_umount semaphore in a different place). And generally, I'm not convinced
there are not other places that try to do IO while holding s_umount
semaphore...

									Honza
-- 
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ