linux-ext4 - Re: [PATCH] ext4: Fix checks for orphan inodes

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: <0ddc8f96-04f5-4d9b-b51e-d6d2666b5a57@huaweicloud.com>
Date: Fri, 26 Sep 2025 09:55:01 +0800
From: Zhang Yi <yi.zhang@...weicloud.com>
To: Jan Kara <jack@...e.cz>
Cc: linux-ext4@...r.kernel.org, Ted Tso <tytso@....edu>
Subject: Re: [PATCH] ext4: Fix checks for orphan inodes

On 9/25/2025 8:30 PM, Jan Kara wrote:
> When orphan file feature is enabled, inode can be tracked as orphan
> either in the standard orphan list or in the orphan file. The first can
> be tested by checking ei->i_orphan list head, the second is recorded by
> EXT4_STATE_ORPHAN_FILE inode state flag. There are several places where
> we want to check whether inode is tracked as orphan and only some of
> them properly check for both possibilities. Luckily the consequences are
> mostly minor, the worst that can happen is that we track an inode as
> orphan although we don't need to and e2fsck then complains (resulting in
> occasional ext4/307 xfstest failures). Fix the problem by introducing a
> helper for checking whether an inode is tracked as orphan and use it in
> appropriate places.
> 
> Fixes: 4a79a98c7b19 ("ext4: Improve scalability of ext4 orphan file handling")
> Signed-off-by: Jan Kara <jack@...e.cz>

Ha, this is a good catch!

Reviewed-by: Zhang Yi <yi.zhang@...wei.com>

> ---
>  fs/ext4/ext4.h   | 10 ++++++++++
>  fs/ext4/file.c   |  2 +-
>  fs/ext4/inode.c  |  2 +-
>  fs/ext4/orphan.c |  6 +-----
>  fs/ext4/super.c  |  4 ++--
>  5 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 01a6e2de7fc3..72e02df72c4c 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1981,6 +1981,16 @@ static inline bool ext4_verity_in_progress(struct inode *inode)
>  
>  #define NEXT_ORPHAN(inode) EXT4_I(inode)->i_dtime
>  
> +/*
> + * Check whether the inode is tracked as orphan (either in orphan file or
> + * orphan list).
> + */
> +static inline bool ext4_inode_orphan_tracked(struct inode *inode)
> +{
> +	return ext4_test_inode_state(inode, EXT4_STATE_ORPHAN_FILE) ||
> +		!list_empty(&EXT4_I(inode)->i_orphan);
> +}
> +
>  /*
>   * Codes for operating systems
>   */
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 93240e35ee36..7a8b30932189 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -354,7 +354,7 @@ static void ext4_inode_extension_cleanup(struct inode *inode, bool need_trunc)
>  	 * to cleanup the orphan list in ext4_handle_inode_extension(). Do it
>  	 * now.
>  	 */
> -	if (!list_empty(&EXT4_I(inode)->i_orphan) && inode->i_nlink) {
> +	if (ext4_inode_orphan_tracked(inode) && inode->i_nlink) {
>  		handle_t *handle = ext4_journal_start(inode, EXT4_HT_INODE, 2);
>  
>  		if (IS_ERR(handle)) {
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 5b7a15db4953..5230452e29dd 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4748,7 +4748,7 @@ static int ext4_fill_raw_inode(struct inode *inode, struct ext4_inode *raw_inode
>  		 * old inodes get re-used with the upper 16 bits of the
>  		 * uid/gid intact.
>  		 */
> -		if (ei->i_dtime && list_empty(&ei->i_orphan)) {
> +		if (ei->i_dtime && !ext4_inode_orphan_tracked(inode)) {
>  			raw_inode->i_uid_high = 0;
>  			raw_inode->i_gid_high = 0;
>  		} else {
> diff --git a/fs/ext4/orphan.c b/fs/ext4/orphan.c
> index 524d4658fa40..0fbcce67ffd4 100644
> --- a/fs/ext4/orphan.c
> +++ b/fs/ext4/orphan.c
> @@ -109,11 +109,7 @@ int ext4_orphan_add(handle_t *handle, struct inode *inode)
>  
>  	WARN_ON_ONCE(!(inode->i_state & (I_NEW | I_FREEING)) &&
>  		     !inode_is_locked(inode));
> -	/*
> -	 * Inode orphaned in orphan file or in orphan list?
> -	 */
> -	if (ext4_test_inode_state(inode, EXT4_STATE_ORPHAN_FILE) ||
> -	    !list_empty(&EXT4_I(inode)->i_orphan))
> +	if (ext4_inode_orphan_tracked(inode))
>  		return 0;
>  
>  	/*
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 699c15db28a8..ba497387b9c8 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1438,9 +1438,9 @@ static void ext4_free_in_core_inode(struct inode *inode)
>  
>  static void ext4_destroy_inode(struct inode *inode)
>  {
> -	if (!list_empty(&(EXT4_I(inode)->i_orphan))) {
> +	if (ext4_inode_orphan_tracked(inode)) {
>  		ext4_msg(inode->i_sb, KERN_ERR,
> -			 "Inode %lu (%p): orphan list check failed!",
> +			 "Inode %lu (%p): inode tracked as orphan!",
>  			 inode->i_ino, EXT4_I(inode));
>  		print_hex_dump(KERN_INFO, "", DUMP_PREFIX_ADDRESS, 16, 4,
>  				EXT4_I(inode), sizeof(struct ext4_inode_info),