Free allocated configuration memory

Most of these were found with valgrind by repeatedly doing lxc_container_new
followed by lxc_container_put. Also free memory when config items are
re-parsed, as happens when lxcapi_set_config_item() is called. Refactored
path type config items to use a common underlying routine.

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 2e6a7921331eea8bcf996524967a4b69ef079249..075322173215551a2b9e912c80f6398150c044ad 100644 (file)
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -2473,6 +2473,7 @@ static void lxc_remove_nic(struct lxc_list *it)
                free(it2->elem);
                free(it2);
        }
+       free(netdev);
        free(it);
 }
 
@@ -2655,6 +2656,14 @@ void lxc_conf_free(struct lxc_conf *conf)
                free(conf->console.path);
        if (conf->rootfs.mount != default_rootfs_mount)
                free(conf->rootfs.mount);
+       if (conf->rootfs.path)
+               free(conf->rootfs.path);
+       if (conf->utsname)
+               free(conf->utsname);
+       if (conf->ttydir)
+               free(conf->ttydir);
+       if (conf->fstab)
+               free(conf->fstab);
        lxc_clear_config_network(conf);
 #if HAVE_APPARMOR
        if (conf->aa_profile)

diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index 9e4fe91df675eec26e1a36caa112a247c1eabdd1..c1cb693ba8feadc2839751ee002b9e89d04eb3b9 100644 (file)
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -488,17 +488,21 @@ static int config_network_hwaddr(const char *key, char *value,
                                 struct lxc_conf *lxc_conf)
 {
        struct lxc_netdev *netdev;
+       char *hwaddr;
 
        netdev = network_netdev(key, value, &lxc_conf->network);
        if (!netdev)
                return -1;
 
-       netdev->hwaddr = strdup(value);
-       if (!netdev->hwaddr) {
+       hwaddr = strdup(value);
+       if (!hwaddr) {
                SYSERROR("failed to dup string '%s'", value);
                return -1;
        }
 
+       if (netdev->hwaddr)
+               free(netdev->hwaddr);
+       netdev->hwaddr = hwaddr;
        return 0;
 }
 
@@ -521,17 +525,21 @@ static int config_network_mtu(const char *key, char *value,
                              struct lxc_conf *lxc_conf)
 {
        struct lxc_netdev *netdev;
+       char *mtu;
 
        netdev = network_netdev(key, value, &lxc_conf->network);
        if (!netdev)
                return -1;
 
-       netdev->mtu = strdup(value);
-       if (!netdev->mtu) {
+       mtu = strdup(value);
+       if (!mtu) {
                SYSERROR("failed to dup string '%s'", value);
                return -1;
        }
 
+       if (netdev->mtu)
+               free(netdev->mtu);
+       netdev->mtu = mtu;
        return 0;
 }
 
@@ -787,6 +795,8 @@ static int config_seccomp(const char *key, char *value,
                return -1;
        }
 
+       if (lxc_conf->seccomp)
+               free(lxc_conf->seccomp);
        lxc_conf->seccomp = path;
 
        return 0;
@@ -859,6 +869,8 @@ static int config_ttydir(const char *key, char *value,
                return -1;
        }
 
+       if (lxc_conf->ttydir)
+               free(lxc_conf->ttydir);
        lxc_conf->ttydir = path;
 
        return 0;
@@ -877,6 +889,8 @@ static int config_aa_profile(const char *key, char *value, struct lxc_conf *lxc_
                return -1;
        }
 
+       if (lxc_conf->aa_profile)
+               free(lxc_conf->aa_profile);
        lxc_conf->aa_profile = path;
 
        return 0;
@@ -951,22 +965,33 @@ out:
        return -1;
 }
 
-static int config_fstab(const char *key, char *value, struct lxc_conf *lxc_conf)
+static int config_path_item(const char *key, const char *value,
+                           struct lxc_conf *lxc_conf, char **conf_item)
 {
+       char *valdup;
        if (strlen(value) >= MAXPATHLEN) {
                ERROR("%s path is too long", value);
                return -1;
        }
 
-       lxc_conf->fstab = strdup(value);
-       if (!lxc_conf->fstab) {
+       valdup = strdup(value);
+       if (!valdup) {
                SYSERROR("failed to duplicate string %s", value);
                return -1;
        }
+       if (*conf_item)
+               free(*conf_item);
+       *conf_item = valdup;
 
        return 0;
 }
 
+static int config_fstab(const char *key, const char *value,
+                       struct lxc_conf *lxc_conf)
+{
+       return config_path_item(key, value, lxc_conf, &lxc_conf->fstab);
+}
+
 static int config_mount(const char *key, char *value, struct lxc_conf *lxc_conf)
 {
        char *fstab_token = "lxc.mount";
@@ -1006,7 +1031,7 @@ static int config_mount(const char *key, char *value, struct lxc_conf *lxc_conf)
 static int config_cap_drop(const char *key, char *value,
                           struct lxc_conf *lxc_conf)
 {
-       char *dropcaps, *sptr, *token;
+       char *dropcaps, *dropptr, *sptr, *token;
        struct lxc_list *droplist;
        int ret = -1;
 
@@ -1021,13 +1046,12 @@ static int config_cap_drop(const char *key, char *value,
 
        /* in case several capability drop is specified in a single line
         * split these caps in a single element for the list */
-       for (;;) {
-                token = strtok_r(dropcaps, " \t", &sptr);
+       for (dropptr = dropcaps;;dropptr = NULL) {
+                token = strtok_r(dropptr, " \t", &sptr);
                 if (!token) {
                        ret = 0;
                         break;
                }
-               dropcaps = NULL;
 
                droplist = malloc(sizeof(*droplist));
                if (!droplist) {
@@ -1035,10 +1059,6 @@ static int config_cap_drop(const char *key, char *value,
                        break;
                }
 
-               /* note - i do believe we're losing memory here,
-                  note that token is already pointing into dropcaps
-                  which is strdup'd.  we never free that bit.
-                */
                droplist->elem = strdup(token);
                if (!droplist->elem) {
                        SYSERROR("failed to dup '%s'", token);
@@ -1065,6 +1085,8 @@ static int config_console(const char *key, char *value,
                return -1;
        }
 
+       if (lxc_conf->console.path)
+               free(lxc_conf->console.path);
        lxc_conf->console.path = path;
 
        return 0;
@@ -1078,50 +1100,17 @@ static int config_includefile(const char *key, char *value,
 
 static int config_rootfs(const char *key, char *value, struct lxc_conf *lxc_conf)
 {
-       if (strlen(value) >= MAXPATHLEN) {
-               ERROR("%s path is too long", value);
-               return -1;
-       }
-
-       lxc_conf->rootfs.path = strdup(value);
-       if (!lxc_conf->rootfs.path) {
-               SYSERROR("failed to duplicate string %s", value);
-               return -1;
-       }
-
-       return 0;
+       return config_path_item(key, value, lxc_conf, &lxc_conf->rootfs.path);
 }
 
 static int config_rootfs_mount(const char *key, char *value, struct lxc_conf *lxc_conf)
 {
-       if (strlen(value) >= MAXPATHLEN) {
-               ERROR("%s path is too long", value);
-               return -1;
-       }
-
-       lxc_conf->rootfs.mount = strdup(value);
-       if (!lxc_conf->rootfs.mount) {
-               SYSERROR("failed to duplicate string '%s'", value);
-               return -1;
-       }
-
-       return 0;
+       return config_path_item(key, value, lxc_conf, &lxc_conf->rootfs.mount);
 }
 
 static int config_pivotdir(const char *key, char *value, struct lxc_conf *lxc_conf)
 {
-       if (strlen(value) >= MAXPATHLEN) {
-               ERROR("%s path is too long", value);
-               return -1;
-       }
-
-       lxc_conf->rootfs.pivot = strdup(value);
-       if (!lxc_conf->rootfs.pivot) {
-               SYSERROR("failed to duplicate string %s", value);
-               return -1;
-       }
-
-       return 0;
+       return config_path_item(key, value, lxc_conf, &lxc_conf->rootfs.pivot);
 }
 
 static int config_utsname(const char *key, char *value, struct lxc_conf *lxc_conf)
@@ -1141,6 +1130,8 @@ static int config_utsname(const char *key, char *value, struct lxc_conf *lxc_con
        }
 
        strcpy(utsname->nodename, value);
+       if (lxc_conf->utsname)
+               free(lxc_conf->utsname);
        lxc_conf->utsname = utsname;
 
        return 0;

diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 37f5ed7b08464d969a8c61f7e6985974ca49b24c..ac995a6e9e4068db5609ccb8a0fdacacf49fe4a9 100644 (file)
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -65,6 +65,10 @@ static void lxc_container_free(struct lxc_container *c)
                free(c->error_string);
                c->error_string = NULL;
        }
+       if (c->slock) {
+               sem_close(c->slock);
+               c->slock = NULL;
+       }
        if (c->privlock) {
                sem_destroy(c->privlock);
                free(c->privlock);
@@ -74,11 +78,10 @@ static void lxc_container_free(struct lxc_container *c)
                free(c->name);
                c->name = NULL;
        }
-       /*
-        * XXX TODO
-        * note, c->lxc_conf is going to have to be freed, but the fn
-        * to do that hasn't been written yet near as I can tell
-        */
+       if (c->lxc_conf) {
+               lxc_conf_free(c->lxc_conf);
+               c->lxc_conf = NULL;
+       }
        free(c);
 }