uimagefs segmentation fault

Hi!

I have prepared a linux uImage file for testing my uImage-related patches.

barebox$ dumpimage -l ./defaultenv/defaultenv-2-base/uImage

Image Name:   Linux-6.7.0-rc4-00003-ge185a416f
Created:      Wed Jan  3 15:28:27 2024
Image Type:   ARM Linux Kernel Image (uncompressed)
Data Size:    6131616 Bytes = 5987.91 KiB = 5.85 MiB
Load Address: 00000000
Entry Point:  00000000

I tryed to mount my uImage under mainline master sandbox barebox:

barebox@Sandbox:/ mount -t uimagefs env/uImage /mnt/ Segmentation fault

The problem is in the uimagefs_add_str(). if s == NULL then strlen() fails:

static int uimagefs_add_str(struct uimagefs_handle *priv, enum uimagefs_type type,
char *s)

{
        struct uimagefs_handle_data *d;

        d = xzalloc(sizeof(*d));
        d->type = type;
        d->name = xstrdup(uimagefs_type_to_str(type));
        d->data = s;
        d->size = strlen(s); <<<<<<<<<<<<<<<<<<<<<<<<<<< s == NULL!

        list_add_tail(&d->list, &priv->list);

        return 0;
}

It looks like my uImage lacks some attributes, e.g. "os".

So I can propose several solutions for the problem:

1. don't call strlen(s) if s == NULL:

@@ -226,7 +226,7 @@ static int uimagefs_add_str(struct uimagefs_handle *priv, enum uimagefs_type typ

        d->type = type;
        d->name = xstrdup(uimagefs_type_to_str(type));
        d->data = s;

- d->size = strlen(s);
+ d->size = (s != NULL) ? strlen(s) : 0;

        list_add_tail(&d->list, &priv->list);

So we will have empty files (len == 0) for absent attributes.

2. regard the s == NULL situation as error:

@@ -222,6 +222,9 @@ static int uimagefs_add_str(struct uimagefs_handle *priv, enum uimagefs_type typ

 {
        struct uimagefs_handle_data *d;

+ if (!s)
+ return -ENODATA;
+

        d = xzalloc(sizeof(*d));
        d->type = type;
        d->name = xstrdup(uimagefs_type_to_str(type));

So we can't mount uImages without required attributes.

3. don't show absent attributes, e.g.:

@@ -222,6 +222,9 @@ static int uimagefs_add_str(struct uimagefs_handle *priv, enum uimagefs_type typ

 {
        struct uimagefs_handle_data *d;

+ if (!s)
+ return -ENODATA;
+

        d = xzalloc(sizeof(*d));
        d->type = type;
        d->name = xstrdup(uimagefs_type_to_str(type));
@@ -421,7 +421,7 @@ static int __uimage_open(struct uimagefs_handle *priv)
                goto err_out;
 
        ret = uimagefs_add_os(priv);
-       if (ret)
+       if (ret && ret != -ENODATA)
                goto err_out;
 
        ret = uimagefs_add_time(priv);

I'm not a skilled uImage user so I have no idea which solution is acceptable for the users.

Please comment!

Best regards,
  Antony Pavlov

Hi Antony,

I don't see a point in refusing to mount uImages without these properties, they might not be required and the user should decide if they are needed or not.

I don't have any preference for 1. or 3. though, so unless anybody else has a good reason for one or the other go and roll a dice ;)

Sascha

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |