Re: [PATCH] ext4:Fix the initial value of not_found in ext4_destroy_inline_data_nolock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The patch initializes `is.s.not_found` to `-ENODATA` but fails to check this value before unconditionally deleting the inline data xattr, leading to a catastrophic size_t underflow and KASAN out-of-bounds write (kernel panic).
--

commit af434a75deba4ee29e45b41767340323974fc530
Author: shuo chen <1289151713@qq.com>

ext4:Fix the initial value of not_found in ext4_destroy_inline_data_nolock

If a file has EXT4_INODE_INLINE_DATA set but lacks the EXT4_STATE_XATTR
flag, ext4_xattr_ibody_find() will exit early without modifying
is.s.not_found. This may cause ext4_xattr_ibody_set() to behave
incorrectly.

> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index caa51473207d7..4f02e7e9f5a30 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -405,7 +405,7 @@ static int ext4_destroy_inline_data_nolock(handle_t *handle,
>  {
>  	struct ext4_inode_info *ei = EXT4_I(inode);
>  	struct ext4_xattr_ibody_find is = {
> -		.s = { .not_found = 0, },
> +		.s = { .not_found = -ENODATA, },

[Severity: Critical]
This is a pre-existing issue, but does this patch fully resolve the problem?

While is.s.not_found is now correctly initialized to -ENODATA,
ext4_destroy_inline_data_nolock() still unconditionally calls
ext4_xattr_ibody_inline_set() without checking if is.s.not_found indicates
the attribute is missing.

When ext4_xattr_ibody_find() returns 0 and leaves not_found as -ENODATA,
this leads to a deletion operation (i.value == NULL) in
ext4_xattr_set_entry() on a non-existent entry.

Because the deletion path in ext4_xattr_set_entry() ignores s->not_found:

fs/ext4/xattr.c:ext4_xattr_set_entry() {
	...
	if (!i->value) {
		/* Remove old name. */
		size_t size = EXT4_XATTR_LEN(name_len);

		last = ENTRY((void *)last - size);
		memmove(here, (void *)here + size,
			(void *)last - (void *)here + sizeof(__u32));
		memset(last, 0, size);
	}
	...
}

Executing this when the entry doesn't exist causes an integer underflow in the
memmove() length parameter, which leads to an out-of-bounds write and kernel
panic.

Should ext4_destroy_inline_data_nolock() explicitly check is.s.not_found
and skip ext4_xattr_ibody_inline_set() when it is -ENODATA?

>  	};
>  	struct ext4_xattr_info i = {
>  		.name_index = EXT4_XATTR_INDEX_SYSTEM,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_CD1AF9C3A7875C42F2187B57D74A1C5EE609@qq.com?part=1