[PATCH] libubootenv: initialize allocated memory

26 views
Skip to first unread message

Harald Brinkmann

unread,
Jun 2, 2026, 8:05:25 AM (3 days ago) Jun 2
to swup...@googlegroups.com
This prevents leaking leftover heap data into the end of the environment.

Signed-off-by: Harald Brinkmann <brin...@stulz-digital-solutions.com>
---
src/uboot_env.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/uboot_env.c b/src/uboot_env.c
index 96d23bf..3121a52 100644
--- a/src/uboot_env.c
+++ b/src/uboot_env.c
@@ -526,10 +526,13 @@ int libuboot_env_store(struct uboot_ctx *ctx)
/*
* Allocate the bigger of the case
*/
- image = malloc(sizeof(struct uboot_env_redund) + ctx->size);
+ size = sizeof(struct uboot_env_redund) + ctx->size;
+ image = malloc(size);
if (!image)
return -ENOMEM;

+ memset(image, 0, size);
+
if (ctx->redundant)
offsetdata = offsetof(struct uboot_env_redund, data);
else
--
2.53.0

Sitz der Gesellschaft: Hamburg Amtsgericht Hamburg | HRB 172947 Geschäftsführer: Markus Trautwein, Thorsten Weiß

Stefano Babic

unread,
Jun 2, 2026, 8:28:51 AM (3 days ago) Jun 2
to Harald Brinkmann, swup...@googlegroups.com
Hallo Harald,

On 6/2/26 14:05, Harald Brinkmann wrote:
> This prevents leaking leftover heap data into the end of the environment.
>
> Signed-off-by: Harald Brinkmann <brin...@stulz-digital-solutions.com>
> ---
> src/uboot_env.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/uboot_env.c b/src/uboot_env.c
> index 96d23bf..3121a52 100644
> --- a/src/uboot_env.c
> +++ b/src/uboot_env.c
> @@ -526,10 +526,13 @@ int libuboot_env_store(struct uboot_ctx *ctx)
> /*
> * Allocate the bigger of the case
> */
> - image = malloc(sizeof(struct uboot_env_redund) + ctx->size);
> + size = sizeof(struct uboot_env_redund) + ctx->size;
> + image = malloc(size);
> if (!image)
> return -ENOMEM;
>
> + memset(image, 0, size);
> +

It is not very clear to me which is the issue. The only change is that
the data is zeroed. But on the other side, this is on the heap, the
exact amount of written data is when devwrite() is called, and after
that this temporary storage is freed.

Which is the issue ?

Best regards,
Stefano Babic

> if (ctx->redundant)
> offsetdata = offsetof(struct uboot_env_redund, data);
> else
> --
> 2.53.0
>
> Sitz der Gesellschaft: Hamburg Amtsgericht Hamburg | HRB 172947 Geschäftsführer: Markus Trautwein, Thorsten Weiß
>

--
_______________________________________________________________________
Nabla Software Engineering GmbH
Hirschstr. 111A | 86156 Augsburg | Tel: +49 821 45592596
Geschäftsführer : Stefano Babic | HRB 40522 Augsburg
E-Mail: sba...@nabladev.com

Harald Brinkmann

unread,
Jun 2, 2026, 8:43:06 AM (3 days ago) Jun 2
to Stefano Babic, swup...@googlegroups.com
Hi Stefano,

On 2026-06-02 14:28, Stefano Babic wrote:
> It is not very clear to me which is the issue. The only change is that
> the data is zeroed. But on the other side, this is on the heap, the
> exact amount of written data is when devwrite() is called, and after
> that this temporary storage is freed.
>
> Which is the issue ?

image is not initialized and ends up being written to storage in _full_.
I found the contents of /etc/fw_env.config in my environment, because it
happened to be a bit after where image was malloc'ed.

It is still correctly parsed by u-boot and libubootenv because both just
read until 0x00.

Stefano Babic

unread,
Jun 2, 2026, 8:57:00 AM (3 days ago) Jun 2
to Harald Brinkmann, swup...@googlegroups.com
Hi Harald,

On 6/2/26 14:43, Harald Brinkmann wrote:
> Hi Stefano,
>
> On 2026-06-02 14:28, Stefano Babic wrote:
>> It is not very clear to me which is the issue. The only change is that
>> the data is zeroed. But on the other side, this is on the heap, the
>> exact amount of written data is when devwrite() is called, and after
>> that this temporary storage is freed.
>>
>> Which is the issue ?
>
> image is not initialized and ends up being written to storage in _full_.
> I found the contents of /etc/fw_env.config in my environment, because it
> happened to be a bit after where image was malloc'ed.

Not clear what you mind. The size in fw_env.config must be the same than
in U-Boot, else you get CRC error.

>
> It is still correctly parsed by u-boot and libubootenv because both just
> read until 0x00.

...exactly, and then I do not yet understand where is the problem.....

Nevertheless, we can else zeroes, then use calloc() instead of malloc +
memset.

Best regards,
Stefano

Harald Brinkmann

unread,
Jun 2, 2026, 9:12:34 AM (3 days ago) Jun 2
to Stefano Babic, swup...@googlegroups.com
Hi Stefano,

On 2026-06-02 14:56, Stefano Babic wrote:
> Not clear what you mind. The size in fw_env.config must be the same than
> in U-Boot, else you get CRC error.
Yes. Size and CRC match, because the CRC is calculated over the
initialized and uninitialized parts of image.

> > It is still correctly parsed by u-boot and libubootenv because both just
> > read until 0x00.
>
> ...exactly, and then I do not yet understand where is the problem.....
I found the actual file contents of /etc/fw_env.config in my
environment on SPI flash. Because it was loaded there by fw_setenv and
then the memory got reused as part of image.
Writing uninitialized heap to storage is an issue, right?

> Nevertheless, we can else zeroes, then use calloc() instead of malloc +
> memset.
I'll update it.

Harald Brinkmann

unread,
Jun 2, 2026, 9:13:13 AM (3 days ago) Jun 2
to Stefano Babic, swup...@googlegroups.com
This prevents leaking leftover heap data into the end of the environment.

Signed-off-by: Harald Brinkmann <brin...@stulz-digital-solutions.com>
---
src/uboot_env.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/uboot_env.c b/src/uboot_env.c
index 96d23bf..14db869 100644
--- a/src/uboot_env.c
+++ b/src/uboot_env.c
@@ -526,7 +526,7 @@ int libuboot_env_store(struct uboot_ctx *ctx)
/*
* Allocate the bigger of the case
*/
- image = malloc(sizeof(struct uboot_env_redund) + ctx->size);
+ image = calloc(1, sizeof(struct uboot_env_redund) + ctx->size);
if (!image)
return -ENOMEM;

--
2.53.0

Stefano Babic

unread,
Jun 2, 2026, 9:14:46 AM (3 days ago) Jun 2
to Harald Brinkmann, swup...@googlegroups.com
On 6/2/26 15:13, Harald Brinkmann wrote:
> This prevents leaking leftover heap data into the end of the environment.
>
> Signed-off-by: Harald Brinkmann <brin...@stulz-digital-solutions.com>
> ---
> src/uboot_env.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/uboot_env.c b/src/uboot_env.c
> index 96d23bf..14db869 100644
> --- a/src/uboot_env.c
> +++ b/src/uboot_env.c
> @@ -526,7 +526,7 @@ int libuboot_env_store(struct uboot_ctx *ctx)
> /*
> * Allocate the bigger of the case
> */
> - image = malloc(sizeof(struct uboot_env_redund) + ctx->size);
> + image = calloc(1, sizeof(struct uboot_env_redund) + ctx->size);
> if (!image)
> return -ENOMEM;
>

Reviewed-by: Stefano Babic <stefan...@swupdate.org>

> --
> 2.53.0
>
> Sitz der Gesellschaft: Hamburg Amtsgericht Hamburg | HRB 172947 Geschäftsführer: Markus Trautwein, Thorsten Weiß


Reply all
Reply to author
Forward
0 new messages