linux-ext4 - Re: [PATCH v1 1/2] ext4: find old entry again if failed to rename whiteout

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:   Fri, 12 Mar 2021 10:01:50 +0800
From:   "zhangyi (F)" <yi.zhang@...wei.com>
To:     Theodore Ts'o <tytso@....edu>
CC:     <linux-ext4@...r.kernel.org>, <jack@...e.cz>,
        <yangerkun@...wei.com>
Subject: Re: [PATCH v1 1/2] ext4: find old entry again if failed to rename
 whiteout

On 2021/3/11 23:43, Theodore Ts'o wrote:
> On Wed, Mar 03, 2021 at 09:17:02PM +0800, zhangyi (F) wrote:
>> If we failed to add new entry on rename whiteout, we cannot reset the
>> old->de entry directly, because the old->de could have moved from under
>> us during make indexed dir. So find the old entry again before reset is
>> needed, otherwise it may corrupt the filesystem as below.
>>
>>   /dev/sda: Entry '00000001' in ??? (12) has deleted/unused inode 15. CLEARED.
>>   /dev/sda: Unattached inode 75
>>   /dev/sda: UNEXPECTED INCONSISTENCY; RUN fsck MANUALLY.
>>
>>   ....
>>
>> +	/*
>> +	 * old->de could have moved from under us during make indexed dir,
>> +	 * so the old->de may no longer valid and need to find it again
>> +	 * before reset old inode info.
>> +	 */
>> +	old.bh = ext4_find_entry(old.dir, &old.dentry->d_name, &old.de, NULL);
>> +	if (IS_ERR(old.bh))
>> +		retval = PTR_ERR(old.bh);
>> +	if (!old.bh)
>> +		retval = -ENOENT;
>> +	if (retval) {
>> +		ext4_std_error(old.dir->i_sb, retval);
> 
> 
> So if the directory entry may have been deleted out from under us, an
> ENOENT failure might happen under normal circumstances, shouldn't it?
> > In that case, ext4_std_error() will declare that the file system is
> inconsistent, potentially resulting in the file system to be remounted
> read-only, or causing the system to panic.  So calling
> ext4_std_error() needs to be done carefully.
> 
> Are we sure that calling ext4_std_error() is the right thing to do in
> the case where ext4_find_entry() returns ENOENT?
> 

Hi, Ted

In this error path of whiteout rename, we want to restore the old inode
number and old name back to the old entry, it's just a rollback operation.
The old entry will stay where it was in common cases, but it can be moved
from the first block to the leaf block during make indexed dir for one
special case, but it cannot be deleted in theory. So if we cannot find it
again, there must some bad thing happen and the filesystem may probably
inconsistency. So I calling ext4_std_error() here，or am I missing something？

Thanks,
Yi.