From: Jan Kiszka <[email protected]>

When done with the with using it, bgenv should be finalized in order to
release resources allocated during bgenv_init and the later usage of
config_parts. Adjust the users accordingly.

Furthermore, bgenv_init should be made robust against multiple
succeeding invocations. They shall simply be ignored so that we do not
leak resources allocated on first init.

While at it, drop pointless zeroing of global var config_parts.

Reported-by: Tolga Hosgor <[email protected]>
Signed-off-by: Jan Kiszka <[email protected]>
---
 env/env_api.c     |  1 +
 env/env_api_fat.c | 24 +++++++++++++++++++++---
 include/env_api.h |  1 +
 tools/bg_setenv.c | 25 ++++++++++++++++---------
 4 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/env/env_api.c b/env/env_api.c
index c2c5196..16de319 100644
--- a/env/env_api.c
+++ b/env/env_api.c
@@ -236,6 +236,7 @@ int ebg_env_close(ebgenv_t *e)
        }
        bgenv_close(env_current);
        e->bgenv = NULL;
+       bgenv_finalize();
        return res;
 }

diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index ac408f6..2b23e7f 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -128,10 +128,13 @@ bool write_env(CONFIG_PART *part, BG_ENVDATA *env)
 CONFIG_PART config_parts[ENV_NUM_CONFIG_PARTS];
 BG_ENVDATA envdata[ENV_NUM_CONFIG_PARTS];

-bool bgenv_init()
+static bool initialized;
+
+bool bgenv_init(void)
 {
-       memset((void *)&config_parts, 0,
-              sizeof(CONFIG_PART) * ENV_NUM_CONFIG_PARTS);
+       if (initialized) {
+               return true;
+       }
        /* enumerate all config partitions */
        if (!probe_config_partitions(config_parts)) {
                VERBOSE(stderr, "Error finding config partitions.\n");
@@ -149,9 +152,24 @@ bool bgenv_init()
                            sizeof(BG_ENVDATA) - sizeof(envdata[i].crc32));
                }
        }
+       initialized = true;
        return true;
 }

+void bgenv_finalize(void)
+{
+       if (!initialized) {
+               return;
+       }
+       for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+               free(config_parts[i].devpath);
+               config_parts[i].devpath = NULL;
+               free(config_parts[i].mountpoint);
+               config_parts[i].mountpoint = NULL;
+       }
+       initialized = false;
+}
+
 BGENV *bgenv_open_by_index(uint32_t index)
 {
        BGENV *handle;
diff --git a/include/env_api.h b/include/env_api.h
index 8051964..9a5c2a8 100644
--- a/include/env_api.h
+++ b/include/env_api.h
@@ -79,6 +79,7 @@ extern char *str16to8(char *buffer, wchar_t *src);
 extern wchar_t *str8to16(wchar_t *buffer, char *src);

 extern bool bgenv_init(void);
+extern void bgenv_finalize(void);
 extern BGENV *bgenv_open_by_index(uint32_t index);
 extern BGENV *bgenv_open_oldest(void);
 extern BGENV *bgenv_open_latest(void);
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c
index 0f34917..24b4067 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c
@@ -618,6 +618,7 @@ int main(int argc, char **argv)
        dump_envs();

        if (!write_mode) {
+               bgenv_finalize();
                return 0;
        }

@@ -631,13 +632,16 @@ int main(int argc, char **argv)
                if (!env_current) {
                        fprintf(stderr, "Failed to retrieve latest environment."
                                        "\n");
-                       return 1;
+                       result = 1;
+                       goto cleanup;
                }
                env_new = bgenv_open_oldest();
                if (!env_new) {
                        fprintf(stderr, "Failed to retrieve oldest environment."
                                        "\n");
-                       return 1;
+                       bgenv_close(env_current);
+                       result = 1;
+                       goto cleanup;
                }
                if (verbosity) {
                        fprintf(stdout,
@@ -647,9 +651,9 @@ int main(int argc, char **argv)

                if (!env_current->data || !env_new->data) {
                        fprintf(stderr, "Invalid environment data pointer.\n");
-                       bgenv_close(env_new);
                        bgenv_close(env_current);
-                       return 1;
+                       result = 1;
+                       goto cleanup;
                }

                memcpy((char *)env_new->data, (char *)env_current->data,
@@ -666,7 +670,8 @@ int main(int argc, char **argv)
                if (!env_new) {
                        fprintf(stderr, "Failed to retrieve environment by "
                                        "index.\n");
-                       return 1;
+                       result = 1;
+                       goto cleanup;
                }
        }

@@ -679,11 +684,13 @@ int main(int argc, char **argv)
        }
        if (!bgenv_write(env_new)) {
                fprintf(stderr, "Error storing environment.\n");
-               return 1;
+               result = 1;
+       } else {
+               fprintf(stdout, "Environment update was successful.\n");
        }
-       bgenv_close(env_new);
-
-       fprintf(stdout, "Environment update was successful.\n");

+cleanup:
+       bgenv_close(env_new);
+       bgenv_finalize();
        return result;
 }
--
2.26.2

-- 
You received this message because you are subscribed to the Google Groups "EFI 
Boot Guard" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/efibootguard-dev/1a38da3fbab3fc0e37025c484d19ad5ab9c9e081.1592139558.git.jan.kiszka%40web.de.

Reply via email to