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

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

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@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?

Thanks,
//richard

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