[PATCH v1 1/2] lib: hashtable: fix integer overflow in himport_r
Rasmus Villemoes
ravi at prevas.dk
Thu Apr 9 11:50:52 CEST 2026
On Wed, Apr 08 2026, Aristo Chen <aristo.chen at 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 at 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
More information about the U-Boot
mailing list