[PATCH 1/2] ext4: Fix integer overflow in ext4fs_read_symlink()

[Heinrich Schuchardt]

On 12.07.24 13:14, Richard Weinberger wrote:
> Am Freitag, 12. Juli 2024, 13:10:12 CEST schrieb 'Heinrich Schuchardt' via upstream:
>> On 02.07.24 21:42, Richard Weinberger wrote:
>>> While zalloc() takes a size_t type, adding 1 to the le32 variable
>>> will overflow.
>>> A carefully crafted ext4 filesystem can exhibit an inode size of 0xffffffff
>>> and as consequence zalloc() will do a zero allocation.
>>>
>>> Later in the function the inode size is again used for copying data.
>>> So an attacker can overwrite memory.
>>>
>>> Avoid the overflow by using the __builtin_add_overflow() helper.
>>>
>>> Signed-off-by: Richard Weinberger <richard at nod.at>
>>> ---
>>>    fs/ext4/ext4_common.c | 7 ++++++-
>>>    1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
>>> index 2ff0dca249..32364b72fb 100644
>>> --- a/fs/ext4/ext4_common.c
>>> +++ b/fs/ext4/ext4_common.c
>>> @@ -2183,13 +2183,18 @@ static char *ext4fs_read_symlink(struct ext2fs_node *node)
>>>    	struct ext2fs_node *diro = node;
>>>    	int status;
>>>    	loff_t actread;
>>> +	size_t alloc_size;
>>>
>>>    	if (!diro->inode_read) {
>>>    		status = ext4fs_read_inode(diro->data, diro->ino, &diro->inode);
>>>    		if (status == 0)
>>>    			return NULL;
>>>    	}
>>> -	symlink = zalloc(le32_to_cpu(diro->inode.size) + 1);
>>> +
>>> +	if (__builtin_add_overflow(le32_to_cpu(diro->inode.size), 1, &alloc_size))
>>
>> U-Boot is freestanding code. You cannot use built-ins.
>
> Hm, I see man built-ins in the U-Boot source.
> Why is this one special?

See the definition of COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW in
include/linux/compiler-clang.h.

Best regards

Heinrich