Re: [PATCH v1 1/2] lib: hashtable: fix integer overflow in himport_r

[Rasmus Villemoes <ravi@prevas.dk>]

On Wed, Apr 08 2026, Aristo Chen <aristo.chen@canonical.com> wrote:

> When size == SIZE_MAX, the expression malloc(size + 1) wraps to
> malloc(0) due to unsigned integer overflow. malloc(0) may return a
> non-NULL pointer, causing the subsequent memcpy(data, env, size) to
> write SIZE_MAX bytes into a zero-byte allocation.
>
> This is reachable from the U-Boot console via "env import", where size
> is taken directly from a user-supplied hex argument.
>
> Add an explicit check for SIZE_MAX before the malloc call and return
> EINVAL.
>
> Signed-off-by: Aristo Chen <aristo.chen@canonical.com>
> ---
>  lib/hashtable.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/lib/hashtable.c b/lib/hashtable.c
> index 75c263b5053..902fa6f3e98 100644
> --- a/lib/hashtable.c
> +++ b/lib/hashtable.c
> @@ -820,6 +820,13 @@ int himport_r(struct hsearch_data *htab,
>  		return 0;
>  	}
>  
> +	/* Check for potential integer overflow */
> +	if (size == SIZE_MAX) {
> +		debug("%s: size too large, would overflow\n", __func__);
> +		__set_errno(EINVAL);
> +		return 0;
> +	}
> +

Well, you can corrupt arbitrary memory from the u-boot shell, so "taken
directly from a user-supplied hex argument" is not really a very
compelling argument in the context of U-Boot.

Instead of adding such ad hoc checks that mostly just increase code size
a little, I think it's better to zoom out and see what this really
does. And this is ripe for adding a memdup_nul() helper (linux has that
under the name kmemdup_nul). If we add that, we can do the overflow
check inside that in that one place, and we can convert a lot of similar
users all over the tree, and eliminate quite a lot of #loc.

I'll try to write something.

Rasmus