segfault within libubootenv when closing and opening environment
diff mbox series

Message ID CALBzuyVo8oNkzfYPrf4dBtYCN2mW6LpArU59SGeEeKDM4my38A@mail.gmail.com
State Not Applicable
Headers show
Series
  • segfault within libubootenv when closing and opening environment
Related show

Commit Message

'Darko Komljenovic' via swupdate Feb. 13, 2020, 2:56 p.m. UTC
Hi,

i am writing to you regarding libubootenv library where i see a
segfault when opening and closing environment to uboot.

My goal in my application is to initialize libuboot only once and open
and close uboot environment whenever necessary so that uboot env does
not stay open (and in blocking mode)

To show the bug easily i have modified fw_printenv.c so that you can
easily retry this. You will get a segfault the fist time you access
libuboot_get_env method.

                                value = libuboot_get_env(ctx, argv[i]);
                                if (noheader)
                                        fprintf(stdout, "%s\n", value
? value : "");

So what i am basically doing here is closing the connection (as it was
opened previously in the file), opening it again and then getting some
environment variable what segfaults immediately.

I am refering to the documentation of libuboot_close where you see
that this should work.

"
 * Release allocated recource for the environment, but
 * maintain the context. This allows to call
 * libuboot_open() again.
"

I am doing something wrong?

Thanks for your help.
Andreas

Comments

Stefano Babic Feb. 13, 2020, 4:46 p.m. UTC | #1
Hi Andreas,

On 13.02.20 15:56, 'Chmielewski Andreas' via swupdate wrote:
> Hi,
> 
> i am writing to you regarding libubootenv library where i see a
> segfault when opening and closing environment to uboot.
> 
> My goal in my application is to initialize libuboot only once and open
> and close uboot environment whenever necessary so that uboot env does
> not stay open (and in blocking mode)
> 
> To show the bug easily i have modified fw_printenv.c so that you can
> easily retry this. You will get a segfault the fist time you access
> libuboot_get_env method.
> 
> diff --git a/src/fw_printenv.c b/src/fw_printenv.c
> index 18887f9..18196c6 100644
> --- a/src/fw_printenv.c
> +++ b/src/fw_printenv.c
> @@ -143,6 +143,16 @@ int main (int argc, char **argv) {
>                         }
>                 } else {
>                         for (i = 0; i < argc; i++) {
> +                               libuboot_close(ctx);
> +
> +                               if ((ret = libuboot_open(ctx)) < 0) {
> +                                       fprintf(stderr, "Cannot read
> environment, using default\n");
> +                                       if ((ret =
> libuboot_load_file(ctx, defenvfile)) < 0) {
> +                                               fprintf(stderr,
> "Cannot read default environment from file\n");
> +                                               exit (ret);
> +                                       }
> +                               }
> +
>                                 value = libuboot_get_env(ctx, argv[i]);
>                                 if (noheader)
>                                         fprintf(stdout, "%s\n", value
> ? value : "");
> 
> So what i am basically doing here is closing the connection (as it was
> opened previously in the file), opening it again and then getting some
> environment variable what segfaults immediately.
> 
> I am refering to the documentation of libuboot_close where you see
> that this should work.
> 
> "
>  * Release allocated recource for the environment, but
>  * maintain the context. This allows to call
>  * libuboot_open() again.
> "
> 
> I am doing something wrong?
> 

I think the cause is that the list is not cleaned up during the close.
Check if the following patch will help:

diff --git a/src/uboot_env.c b/src/uboot_env.c
index 8926373..d619ee0 100644
--- a/src/uboot_env.c
+++ b/src/uboot_env.c
@@ -1369,6 +1369,7 @@ void libuboot_close(struct uboot_ctx *ctx) {
                        free(e->name);
                if (e->value)
                        free(e->value);
+               LIST_REMOVE(e, next);
                free(e);
        }
 }


Best regards,
Stefano Babic

Patch
diff mbox series

diff --git a/src/fw_printenv.c b/src/fw_printenv.c
index 18887f9..18196c6 100644
--- a/src/fw_printenv.c
+++ b/src/fw_printenv.c
@@ -143,6 +143,16 @@  int main (int argc, char **argv) {
                        }
                } else {
                        for (i = 0; i < argc; i++) {
+                               libuboot_close(ctx);
+
+                               if ((ret = libuboot_open(ctx)) < 0) {
+                                       fprintf(stderr, "Cannot read
environment, using default\n");
+                                       if ((ret =
libuboot_load_file(ctx, defenvfile)) < 0) {
+                                               fprintf(stderr,
"Cannot read default environment from file\n");
+                                               exit (ret);
+                                       }
+                               }
+