Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 11 Mar 2021 10:43:15 -0500
From:   "Theodore Ts'o" <tytso@mit.edu>
To:     "zhangyi (F)" <yi.zhang@huawei.com>
Cc:     linux-ext4@vger.kernel.org, jack@suse.cz, yangerkun@huawei.com
Subject: Re: [PATCH v1 1/2] ext4: find old entry again if failed to rename
 whiteout
Message-ID: <YEo6k8kg3zF7avId@mit.edu>
References: <20210303131703.330415-1-yi.zhang@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210303131703.330415-1-yi.zhang@huawei.com>
Precedence: bulk

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?

							- Ted