linux-ext4 - Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

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: <169849F1-1830-4771-9D38-6A39C8B46C90@dilger.ca>
Date:	Wed, 16 Feb 2011 22:13:53 -0700
From:	Andreas Dilger <adilger.kernel@...ger.ca>
To:	toshi.okajima@...fujitsu.com
Cc:	Jan Kara <jack@...e.cz>, Ted Ts'o <tytso@....edu>,
	Masayoshi MIZUMA <m.mizuma@...fujitsu.com>,
	linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [BUG] ext4: cannot unfreeze a filesystem due to a deadlock

On 2011-02-16, at 20:50, Toshiyuki Okajima wrote:
> (2011/02/16 23:56), Jan Kara wrote:
>> 
>>> 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);

This seems like a very low-risk fix.

>>   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...
> 
> OK. I understand.
> 
> This code only fixes the case for the following path:
> writeback_inodes_wb
> -> ext4_da_writepages
>   -> ext4_journal_start_sb
>      -> vfs_check_frozen
> But, the code doesn't fix the other cases.
> 
> We must modify the local filesystem part in order to fix all cases...?

It seems worthwhile to implement the low-risk fix that covers the common case, and if/when someone hits the rare 3-process case and/or submits a patch for it then that one will be fixed also.

Cheers, Andreas





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