fs/ext4/inode.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-)
From: Zhang Yi <yi.zhang@huawei.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@huawei.com>
---
fs/ext4/inode.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 6fba4948e040..44054a04fc4b 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -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. */
+ 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
On Wed 28-01-26 10:16:09, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.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@huawei.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
> + 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
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
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@huawei.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@huawei.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
>>
© 2016 - 2026 Red Hat, Inc.