sbabic/libubootenv][PATCH v2] Non-existing devices results in parsing error (Issue #33)

4 views
Skip to first unread message

Harald Brinkmann

unread,
Feb 20, 2026, 6:21:19 AM (2 days ago) Feb 20
to swup...@googlegroups.com
@LeSpocky requested changes on this pull request:
> Other part of the code initialize the buffer with all zeros to avoid
> truncation issues before calling strncpy(), example: memset(normalized,
> 0, PATH_MAX);
Fixed.

> How does this affect the call of check_env_device() in function
> libuboot_read_config_ext() (file src/uboot_env.c line 913).
It does not fail if one of the env devices is not readable.
Is there an issue down the line if there is a non-existing env device?

This might be a quck and dirty hack. It sure was quick.

On Feb 18, 2026, 10:39:25, Stefano Babic wrote:
> The right solution is to rearrange code and call check_env() when this
> is used. This must be caleld just once, that is the reason it is now
> done as extension of the configuration
If you clarify this, I have a go at a clean solution.


Ignore devices which are not existing. This might be partitions on
unplugged SD cards.

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

diff --git a/src/common.c b/src/common.c
index 2cbde93..9e9f037 100644
--- a/src/common.c
+++ b/src/common.c
@@ -65,8 +65,10 @@ int normalize_device_path(char *path, struct uboot_flash_env *dev)

if ((normalized = realpath(path, NULL)) == NULL)
{
- /* device file didn't exist */
- return -EINVAL;
+ /* device file does not exist */
+ normalized = malloc(PATH_MAX);
+ memset(normalized, 0, PATH_MAX);
+ strncpy(normalized, path, PATH_MAX - 1);
}

normalized_len = strlen(normalized);
@@ -184,8 +186,10 @@ int check_env_device(struct uboot_flash_env *dev)
}

ret = stat(dev->devname, &st);
- if (ret < 0)
- return -EBADF;
+ if (ret < 0) {
+ /* device is not readable, no further checks possible */
+ return 0;
+ }
fd = open(dev->devname, O_RDONLY);
if (fd < 0)
return -EBADF;
--
2.52.0

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

Stefano Babic

unread,
Feb 20, 2026, 11:36:51 AM (2 days ago) Feb 20
to Harald Brinkmann, swup...@googlegroups.com
Hi Harald,

On 2/20/26 11:49, Harald Brinkmann wrote:
> @LeSpocky requested changes on this pull request:
>> Other part of the code initialize the buffer with all zeros to avoid
>> truncation issues before calling strncpy(), example: memset(normalized,
>> 0, PATH_MAX);
> Fixed.
>
>> How does this affect the call of check_env_device() in function
>> libuboot_read_config_ext() (file src/uboot_env.c line 913).
> It does not fail if one of the env devices is not readable.
> Is there an issue down the line if there is a non-existing env device?
>
> This might be a quck and dirty hack. It sure was quick.
>
> On Feb 18, 2026, 10:39:25, Stefano Babic wrote:
>> The right solution is to rearrange code and call check_env() when this
>> is used. This must be caleld just once, that is the reason it is now
>> done as extension of the configuration
> If you clarify this, I have a go at a clean solution.

It is a change in the supported feature. Currently, it requires that the
configured file (fw_env.config) points to existing device. And the
current use case, if the device are not existing (but still static), is
to adjust fw_env.config at runtime.

Changes here are not acceptable because they simply skip the check and
solves only when fw_setenv / fw_printev are called as tools. However,
this project is thought to be a library (with yes, replacement for old
tools, too), and a user application can link to the library and use it,
mainly calling libuboot_set_env() and libuboot_get_env(), so without
reinitializing again.

That means, specially if the environment is not on a device (MTD or
whatever, but just on a file, this file could maybe been created to a
later time, and then the check should be done when devic eis really
used. Note also that your patch is skipping the error and it is just
storing that the configuration is ok, but it is not. I can then call
libuboot_get_namespace() for a bad configured environment without
notice, or worse I get an error much later.

So what I would like to see:

- a namespace can get an additional atribute, "optional"
- if optional, the check can be skipped, but it must be marked in the
structure as "non valid"
- if a call tries to access a namespace that is marked "non valid", it
must be checked again because run time conditions can change.

Best regards,
Stefano Babic
--
_______________________________________________________________________
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

Reply all
Reply to author
Forward
0 new messages