Fix use of list item memory after free

Valgrind showed use of ->next field after item has been free()ed.
Introduce a lxc_list_for_each_safe() which allows traversal of a list
when the body of the loop may remove the currently iterated item.

Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index f1c41f13453b995cf4d89e327357aa1b8cae7064..2e6a7921331eea8bcf996524967a4b69ef079249 100644 (file)
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -734,7 +734,7 @@ static int umount_oldrootfs(const char *oldrootfs)
 {
        char path[MAXPATHLEN];
        void *cbparm[2];
-       struct lxc_list mountlist, *iterator;
+       struct lxc_list mountlist, *iterator, *next;
        int ok, still_mounted, last_still_mounted;
        int rc;
 
@@ -774,7 +774,7 @@ static int umount_oldrootfs(const char *oldrootfs)
                last_still_mounted = still_mounted;
                still_mounted = 0;
 
-               lxc_list_for_each(iterator, &mountlist) {
+               lxc_list_for_each_safe(iterator, &mountlist, next) {
 
                        /* umount normally */
                        if (!umount(iterator->elem)) {
@@ -2445,7 +2445,7 @@ int run_lxc_hooks(const char *name, char *hook, struct lxc_conf *conf)
 static void lxc_remove_nic(struct lxc_list *it)
 {
        struct lxc_netdev *netdev = it->elem;
-       struct lxc_list *it2;
+       struct lxc_list *it2,*next;
 
        lxc_list_del(it);
 
@@ -2463,12 +2463,12 @@ static void lxc_remove_nic(struct lxc_list *it)
                free(netdev->ipv4_gateway);
        if (netdev->ipv6_gateway)
                free(netdev->ipv6_gateway);
-       lxc_list_for_each(it2, &netdev->ipv4) {
+       lxc_list_for_each_safe(it2, &netdev->ipv4, next) {
                lxc_list_del(it2);
                free(it2->elem);
                free(it2);
        }
-       lxc_list_for_each(it2, &netdev->ipv6) {
+       lxc_list_for_each_safe(it2, &netdev->ipv6, next) {
                lxc_list_del(it2);
                free(it2->elem);
                free(it2);
@@ -2510,15 +2510,15 @@ int lxc_clear_nic(struct lxc_conf *c, char *key)
        if (!p1) {
                lxc_remove_nic(it);
        } else if (strcmp(p1, "ipv4") == 0) {
-               struct lxc_list *it2;
-               lxc_list_for_each(it2, &netdev->ipv4) {
+               struct lxc_list *it2,*next;
+               lxc_list_for_each_safe(it2, &netdev->ipv4, next) {
                        lxc_list_del(it2);
                        free(it2->elem);
                        free(it2);
                }
        } else if (strcmp(p1, "ipv6") == 0) {
-               struct lxc_list *it2;
-               lxc_list_for_each(it2, &netdev->ipv6) {
+               struct lxc_list *it2,*next;
+               lxc_list_for_each_safe(it2, &netdev->ipv6, next) {
                        lxc_list_del(it2);
                        free(it2->elem);
                        free(it2);
@@ -2566,8 +2566,8 @@ int lxc_clear_nic(struct lxc_conf *c, char *key)
 
 int lxc_clear_config_network(struct lxc_conf *c)
 {
-       struct lxc_list *it;
-       lxc_list_for_each(it, &c->network) {
+       struct lxc_list *it,*next;
+       lxc_list_for_each_safe(it, &c->network, next) {
                lxc_remove_nic(it);
        }
        return 0;
@@ -2575,9 +2575,9 @@ int lxc_clear_config_network(struct lxc_conf *c)
 
 int lxc_clear_config_caps(struct lxc_conf *c)
 {
-       struct lxc_list *it;
+       struct lxc_list *it,*next;
 
-       lxc_list_for_each(it, &c->caps) {
+       lxc_list_for_each_safe(it, &c->caps, next) {
                lxc_list_del(it);
                free(it->elem);
                free(it);
@@ -2587,14 +2587,14 @@ int lxc_clear_config_caps(struct lxc_conf *c)
 
 int lxc_clear_cgroups(struct lxc_conf *c, char *key)
 {
-       struct lxc_list *it;
+       struct lxc_list *it,*next;
        bool all = false;
        char *k = key + 11;
 
        if (strcmp(key, "lxc.cgroup") == 0)
                all = true;
 
-       lxc_list_for_each(it, &c->cgroup) {
+       lxc_list_for_each_safe(it, &c->cgroup, next) {
                struct lxc_cgroup *cg = it->elem;
                if (!all && strcmp(cg->subsystem, k) != 0)
                        continue;
@@ -2609,9 +2609,9 @@ int lxc_clear_cgroups(struct lxc_conf *c, char *key)
 
 int lxc_clear_mount_entries(struct lxc_conf *c)
 {
-       struct lxc_list *it;
+       struct lxc_list *it,*next;
 
-       lxc_list_for_each(it, &c->mount_list) {
+       lxc_list_for_each_safe(it, &c->mount_list, next) {
                lxc_list_del(it);
                free(it->elem);
                free(it);
@@ -2621,7 +2621,7 @@ int lxc_clear_mount_entries(struct lxc_conf *c)
 
 int lxc_clear_hooks(struct lxc_conf *c, char *key)
 {
-       struct lxc_list *it;
+       struct lxc_list *it,*next;
        bool all = false, done = false;
        char *k = key + 9;
        int i;
@@ -2631,7 +2631,7 @@ int lxc_clear_hooks(struct lxc_conf *c, char *key)
 
        for (i=0; i<NUM_LXC_HOOKS; i++) {
                if (all || strcmp(k, lxchook_names[i]) == 0) {
-                       lxc_list_for_each(it, &c->hooks[i]) {
+                       lxc_list_for_each_safe(it, &c->hooks[i], next) {
                                lxc_list_del(it);
                                free(it->elem);
                                free(it);

diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index 3d9f36e99981a8ae2269311438f71fbde3a258f9..9e4fe91df675eec26e1a36caa112a247c1eabdd1 100644 (file)
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -1231,7 +1231,7 @@ int lxc_config_define_add(struct lxc_list *defines, char* arg)
 
 int lxc_config_define_load(struct lxc_list *defines, struct lxc_conf *conf)
 {
-       struct lxc_list *it;
+       struct lxc_list *it,*next;
        int ret = 0;
 
        lxc_list_for_each(it, defines) {
@@ -1240,7 +1240,7 @@ int lxc_config_define_load(struct lxc_list *defines, struct lxc_conf *conf)
                        break;
        }
 
-       lxc_list_for_each(it, defines) {
+       lxc_list_for_each_safe(it, defines, next) {
                lxc_list_del(it);
                free(it);
        }

diff --git a/src/lxc/list.h b/src/lxc/list.h
index 5213e8085d98424d434bcf4fe5d70d5538c680c4..24dffa2d19b31e127234839cdc19ca05054254a3 100644 (file)
--- a/src/lxc/list.h
+++ b/src/lxc/list.h
@@ -14,6 +14,11 @@ struct lxc_list {
             __iterator != __list;                                      \
             __iterator = __iterator->next)
 
+#define lxc_list_for_each_safe(__iterator, __list, __next)             \
+       for (__iterator = (__list)->next, __next = __iterator->next;    \
+            __iterator != __list;                                      \
+            __iterator = __next, __next = __next->next)
+
 static inline void lxc_list_init(struct lxc_list *list)
 {
        list->elem = NULL;