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

[Tom Rini]

On Fri, Aug 09, 2024 at 06:29:34PM +0200, Richard Weinberger wrote:
> 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.

I'm fine with using this buildin, it's widely enough implemented, for
good reason.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240809/2dbda787/attachment.sig>