linux-kernel - Re: [PATCH] ext4: do not check fast symlink during orphan recovery

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] [day] [month] [year] [list]

Message-ID: <d56ffd21-c835-47cd-8e70-85c0ab320ff8@huaweicloud.com>
Date: Thu, 29 Jan 2026 09:56:48 +0800
From: Zhang Yi <yi.zhang@...weicloud.com>
To: Jan Kara <jack@...e.cz>
Cc: linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org,
 linux-kernel@...r.kernel.org, tytso@....edu, adilger.kernel@...ger.ca,
 ojaswin@...ux.ibm.com, ritesh.list@...il.com, yi.zhang@...wei.com,
 yizhang089@...il.com, libaokun1@...wei.com, yangerkun@...wei.com,
 yukuai@...as.com
Subject: Re: [PATCH] ext4: do not check fast symlink during orphan recovery

On 1/28/2026 5:59 PM, Jan Kara wrote:
> On Wed 28-01-26 10:16:09, Zhang Yi wrote:
>> From: Zhang Yi <yi.zhang@...wei.com>
>>
>> Commit '5f920d5d6083 ("ext4: verify fast symlink length")' causes the
>> generic/475 test to fail during orphan cleanup of zero-length symlinks.
>>
>>   generic/475  84s ... _check_generic_filesystem: filesystem on /dev/vde is inconsistent
>>
>> The fsck reports are provided below:
>>
>>   Deleted inode 9686 has zero dtime.
>>   Deleted inode 158230 has zero dtime.
>>   ...
>>   Inode bitmap differences:  -9686 -158230
>>   Orphan file (inode 12) block 13 is not clean.
>>   Failed to initialize orphan file.
>>
>> In ext4_symlink(), a newly created symlink can be added to the orphan
>> list due to ENOSPC. Its data has not been initialized, and its size is
>> zero. Therefore, we need to disregard the length check of the symbolic
>> link when cleaning up orphan inodes.
>>
>> Fixes: 5f920d5d6083 ("ext4: verify fast symlink length")
>> Signed-off-by: Zhang Yi <yi.zhang@...wei.com>
> 
> Thanks for the patch!
> 
>> @@ -6079,18 +6079,22 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
>>  			inode->i_op = &ext4_encrypted_symlink_inode_operations;
>>  		} else if (ext4_inode_is_fast_symlink(inode)) {
>>  			inode->i_op = &ext4_fast_symlink_inode_operations;
>> -			if (inode->i_size == 0 ||
>> -			    inode->i_size >= sizeof(ei->i_data) ||
>> -			    strnlen((char *)ei->i_data, inode->i_size + 1) !=
>> -								inode->i_size) {
>> -				ext4_error_inode(inode, function, line, 0,
>> -					"invalid fast symlink length %llu",
>> -					 (unsigned long long)inode->i_size);
>> -				ret = -EFSCORRUPTED;
>> -				goto bad_inode;
>> +
>> +			/* Orphan cleanup can get a zero-sized symlink. */
> 
> I was mulling over this for a while. I'd expand the comment here a bit:
> 
> 			/*
> 			 * Orphan cleanup can see inodes with i_size == 0
> 			 * and i_data uninitialized. Skip size checks in
> 			 * that case. This is safe because the first thing
> 			 * ext4_evict_inode() does for fast symlinks is
> 			 * clearing of i_data and i_size.
> 			 */
> 
> and I think we also need to verify that i_nlink is 0 (as otherwise we'd
> leave potentially invalid accessible inode in cache).
> 
> 								Honza

Thank you for the review and suggestions. These makes sense to me, I will
add them in my next iteration.

Thanks,
Yi.

> 
>> +			if (!(EXT4_SB(sb)->s_mount_state & EXT4_ORPHAN_FS)) {
>> +				if (inode->i_size == 0 ||
>> +				    inode->i_size >= sizeof(ei->i_data) ||
>> +				    strnlen((char *)ei->i_data, inode->i_size + 1) !=
>> +						inode->i_size) {
>> +					ext4_error_inode(inode, function, line, 0,
>> +						"invalid fast symlink length %llu",
>> +						(unsigned long long)inode->i_size);
>> +					ret = -EFSCORRUPTED;
>> +					goto bad_inode;
>> +				}
>> +				inode_set_cached_link(inode, (char *)ei->i_data,
>> +						      inode->i_size);
>>  			}
>> -			inode_set_cached_link(inode, (char *)ei->i_data,
>> -					      inode->i_size);
>>  		} else {
>>  			inode->i_op = &ext4_symlink_inode_operations;
>>  		}
>> -- 
>> 2.52.0
>>