libkmod: Release memory on error paths

Do not override pointers to first list nodes if appending failed,
otherwise it's impossible to release already existing nodes of these
lists afterwards.

Remove the now unused function kmod_list_remove_n_latest as well.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Link: https://github.com/kmod-project/kmod/pull/228
Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>

diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h
index c27be01b4f188ad0beb80344e11c0f0ae2303f7a..31979b5f0df70df7c81471261ca66d1a0ed43936 100644 (file)
--- a/libkmod/libkmod-internal.h
+++ b/libkmod/libkmod-internal.h
@@ -58,7 +58,6 @@ _must_check_ _nonnull_(2) struct kmod_list *kmod_list_append(struct kmod_list *l
 _must_check_ _nonnull_(2) struct kmod_list *kmod_list_prepend(struct kmod_list *list, const void *data);
 _must_check_ struct kmod_list *kmod_list_remove(struct kmod_list *list);
 _must_check_ _nonnull_(2) struct kmod_list *kmod_list_remove_data(struct kmod_list *list, const void *data);
-_must_check_ struct kmod_list *kmod_list_remove_n_latest(struct kmod_list *list, unsigned int n);
 _nonnull_(2) struct kmod_list *kmod_list_insert_after(struct kmod_list *list, const void *data);
 _nonnull_(2) struct kmod_list *kmod_list_insert_before(struct kmod_list *list, const void *data);
 _must_check_ struct kmod_list *kmod_list_append_list(struct kmod_list *list1, struct kmod_list *list2);

diff --git a/libkmod/libkmod-list.c b/libkmod/libkmod-list.c
index 591fd7e4cfed3d2c5869ab36d46a8cd131cdba49..5b5899c5b886a7c1a1d6114e46bb29ce6fc4ad67 100644 (file)
--- a/libkmod/libkmod-list.c
+++ b/libkmod/libkmod-list.c
@@ -198,23 +198,6 @@ struct kmod_list *kmod_list_remove_data(struct kmod_list *list, const void *data
        return container_of(node, struct kmod_list, node);
 }
 
-/*
- * n must be greater to or equal the number of elements (we don't check the
- * condition)
- */
-struct kmod_list *kmod_list_remove_n_latest(struct kmod_list *list, unsigned int n)
-{
-       struct kmod_list *l = list;
-       unsigned int i;
-
-       for (i = 0; i < n; i++) {
-               l = kmod_list_last(l);
-               l = kmod_list_remove(l);
-       }
-
-       return l;
-}
-
 KMOD_EXPORT struct kmod_list *kmod_list_prev(const struct kmod_list *list,
                                             const struct kmod_list *curr)
 {

diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c
index 7262abef1c47463deef473b43215ec24d4d768f0..f9d63a94937cdbadbf9535c90f4e1e5c366be7c8 100644 (file)
--- a/libkmod/libkmod.c
+++ b/libkmod/libkmod.c
@@ -356,6 +356,8 @@ static int kmod_lookup_alias_from_alias_bin(struct kmod_ctx *ctx,
        struct index_file *idx;
        struct index_value *realnames, *realname;
 
+       assert(*list == NULL);
+
        if (ctx->indexes[index_number] != NULL) {
                DBG(ctx, "use mmapped index '%s' for name=%s\n",
                    index_files[index_number].fn, name);
@@ -378,6 +380,7 @@ static int kmod_lookup_alias_from_alias_bin(struct kmod_ctx *ctx,
 
        for (realname = realnames; realname; realname = realname->next) {
                struct kmod_module *mod;
+               struct kmod_list *node;
 
                err = kmod_module_new_from_alias(ctx, name, realname->value, &mod);
                if (err < 0) {
@@ -386,7 +389,14 @@ static int kmod_lookup_alias_from_alias_bin(struct kmod_ctx *ctx,
                        goto fail;
                }
 
-               *list = kmod_list_append(*list, mod);
+               node = kmod_list_append(*list, mod);
+               if (node == NULL) {
+                       ERR(ctx, "out of memory\n");
+                       kmod_module_unref(mod);
+                       err = -ENOMEM;
+                       goto fail;
+               }
+               *list = node;
                nmatch++;
        }
 
@@ -394,7 +404,7 @@ static int kmod_lookup_alias_from_alias_bin(struct kmod_ctx *ctx,
        return nmatch;
 
 fail:
-       *list = kmod_list_remove_n_latest(*list, nmatch);
+       kmod_list_release(*list, kmod_module_unref);
        index_values_free(realnames);
        return err;
 }
@@ -458,8 +468,6 @@ int kmod_lookup_alias_from_kernel_builtin_file(struct kmod_ctx *ctx, const char
        struct kmod_list *l;
        int ret;
 
-       assert(*list == NULL);
-
        ret = kmod_lookup_alias_from_alias_bin(ctx, KMOD_INDEX_MODULES_BUILTIN_ALIAS,
                                               name, list);
 
@@ -478,6 +486,7 @@ int kmod_lookup_alias_from_builtin_file(struct kmod_ctx *ctx, const char *name,
 
        if (lookup_builtin_file(ctx, name)) {
                struct kmod_module *mod;
+               struct kmod_list *node;
                int err;
 
                err = kmod_module_new_from_name(ctx, name, &mod);
@@ -490,9 +499,13 @@ int kmod_lookup_alias_from_builtin_file(struct kmod_ctx *ctx, const char *name,
                /* already mark it as builtin since it's being created from
                 * this index */
                kmod_module_set_builtin(mod, true);
-               *list = kmod_list_append(*list, mod);
-               if (*list == NULL)
+               node = kmod_list_append(*list, mod);
+               if (node == NULL) {
+                       ERR(ctx, "out of memory\n");
+                       kmod_module_unref(mod);
                        return -ENOMEM;
+               }
+               *list = node;
        }
 
        return 0;
@@ -514,6 +527,8 @@ int kmod_lookup_alias_from_moddep_file(struct kmod_ctx *ctx, const char *name,
        char *line;
        int n = 0;
 
+       assert(*list == NULL);
+
        /*
         * Module names do not contain ':'. Return early if we know it will
         * not be found.
@@ -524,6 +539,7 @@ int kmod_lookup_alias_from_moddep_file(struct kmod_ctx *ctx, const char *name,
        line = kmod_search_moddep(ctx, name);
        if (line != NULL) {
                struct kmod_module *mod;
+               struct kmod_list *node;
 
                n = kmod_module_new_from_name(ctx, name, &mod);
                if (n < 0) {
@@ -532,7 +548,14 @@ int kmod_lookup_alias_from_moddep_file(struct kmod_ctx *ctx, const char *name,
                        goto finish;
                }
 
-               *list = kmod_list_append(*list, mod);
+               node = kmod_list_append(*list, mod);
+               if (node == NULL) {
+                       ERR(ctx, "out of memory\n");
+                       kmod_module_unref(mod);
+                       n = -ENOMEM;
+                       goto finish;
+               }
+               *list = node;
                kmod_module_parse_depline(mod, line);
        }
 
@@ -549,12 +572,15 @@ int kmod_lookup_alias_from_config(struct kmod_ctx *ctx, const char *name,
        struct kmod_list *l;
        int err, nmatch = 0;
 
+       assert(*list == NULL);
+
        kmod_list_foreach(l, config->aliases) {
                const char *aliasname = kmod_alias_get_name(l);
                const char *modname = kmod_alias_get_modname(l);
 
                if (fnmatch(aliasname, name, 0) == 0) {
                        struct kmod_module *mod;
+                       struct kmod_list *node;
 
                        err = kmod_module_new_from_alias(ctx, aliasname, modname, &mod);
                        if (err < 0) {
@@ -564,7 +590,14 @@ int kmod_lookup_alias_from_config(struct kmod_ctx *ctx, const char *name,
                                goto fail;
                        }
 
-                       *list = kmod_list_append(*list, mod);
+                       node = kmod_list_append(*list, mod);
+                       if (node == NULL) {
+                               ERR(ctx, "out of memory\n");
+                               kmod_module_unref(mod);
+                               err = -ENOMEM;
+                               goto fail;
+                       }
+                       *list = node;
                        nmatch++;
                }
        }
@@ -572,7 +605,7 @@ int kmod_lookup_alias_from_config(struct kmod_ctx *ctx, const char *name,
        return nmatch;
 
 fail:
-       *list = kmod_list_remove_n_latest(*list, nmatch);
+       kmod_list_release(*list, kmod_module_unref);
        return err;
 }
 
@@ -583,6 +616,8 @@ int kmod_lookup_alias_from_commands(struct kmod_ctx *ctx, const char *name,
        struct kmod_list *l, *node;
        int err, nmatch = 0;
 
+       assert(*list == NULL);
+
        kmod_list_foreach(l, config->install_commands) {
                const char *modname = kmod_command_get_modname(l);
 
@@ -600,6 +635,7 @@ int kmod_lookup_alias_from_commands(struct kmod_ctx *ctx, const char *name,
                        node = kmod_list_append(*list, mod);
                        if (node == NULL) {
                                ERR(ctx, "out of memory\n");
+                               kmod_module_unref(mod);
                                return -ENOMEM;
                        }
 
@@ -636,6 +672,7 @@ int kmod_lookup_alias_from_commands(struct kmod_ctx *ctx, const char *name,
                        node = kmod_list_append(*list, mod);
                        if (node == NULL) {
                                ERR(ctx, "out of memory\n");
+                               kmod_module_unref(mod);
                                return -ENOMEM;
                        }
 

diff --git a/testsuite/test-list.c b/testsuite/test-list.c
index 0fcbc805ec383e250e84aa2b67e98d8117107324..7a4b4483c81e8aa4ec7e5c5c43b2b0275ce15f5f 100644 (file)
--- a/testsuite/test-list.c
+++ b/testsuite/test-list.c
@@ -76,33 +76,6 @@ static int test_list_prev(const struct test *t)
 }
 DEFINE_TEST(test_list_prev, .description = "test list prev");
 
-static int test_list_remove_n_latest(const struct test *t)
-{
-       struct kmod_list *list = NULL, *l;
-       int i;
-       const char *v[] = { "v1", "v2", "v3", "v4", "v5" };
-       const int N = ARRAY_SIZE(v), M = N / 2;
-
-       for (i = 0; i < N; i++)
-               list = kmod_list_append(list, v[i]);
-       assert_return(len(list) == N, EXIT_FAILURE);
-
-       list = kmod_list_remove_n_latest(list, M);
-       assert_return(len(list) == N - M, EXIT_FAILURE);
-
-       i = 0;
-       kmod_list_foreach(l, list) {
-               assert_return(l->data == v[i], EXIT_FAILURE);
-               i++;
-       }
-
-       kmod_list_remove_all(list);
-
-       return 0;
-}
-DEFINE_TEST(test_list_remove_n_latest,
-           .description = "test list function to remove n latest elements");
-
 static int test_list_remove_data(const struct test *t)
 {
        struct kmod_list *list = NULL, *l;