Re: [PATCH v2 1/3] ext4: Fix integer overflow in ext4fs_read_symlink()

[Richard Weinberger <richard@sigma-star.at>]

Heinrich,

Am Freitag, 9. August 2024, 18:13:27 CEST schrieb 'Heinrich Schuchardt' via upstream:
> Thank you for pointing at the problematic code.
> 
> You are calling __builtin_add_overflow(int, int, size_t *).
> 
> __builtin_add_overflow() is not defined in the C-standard.
> 
> Is there any well defined behavior for this on systems where size_t has
> more bits than int?

https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html says:

Built-in Function: bool __builtin_add_overflow (type1 a, type2 b, type3 *res)

These built-in functions promote the first two operands into infinite precision signed type and perform addition on those promoted operands.
The result is then cast to the type the third pointer argument points to and stored there.
If the stored result is equal to the infinite precision result, the built-in functions return false, otherwise they return true.
As the addition is performed in infinite signed precision, these built-in functions have fully defined behavior for all argument values. 

So, I don't really see a problem here.

> I could imagine implementations just copying an int to the first 32 bits
> of alloc_size and leaving the other bits untouched.
> 
> Hopefully your C-compiler simply refuses to compile this code.
> 
> I would prefer to stick to standard C:
> 
> alloc_size = le32_to_cpu(diro->inode.size) + 1UL;
> if (!alloc_size)
> 	return NULL;
> 
> Here an overflow can only occur on 32bit systems.

In this specific case we could avoid the builtis, yes.
But I prefer using __builtin_add_overflow() as it offers a generic
and bullet prove mechanism to avoid these kind of bugs.

It's not that they are something new and funky.
Other projects use them since ages.

Thanks,
//richard

-- 
​​​​​sigma star gmbh | Eduard-Bodem-Gasse 6, 6020 Innsbruck, AUT
UID/VAT Nr: ATU 66964118 | FN: 374287y