[lxc-devel] [PATCH] Free allocated configuration memory
Dwight Engen
dwight.engen at oracle.com
Mon Nov 26 17:18:06 UTC 2012
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 at oracle.com>
---
src/lxc/conf.c | 9 +++++
src/lxc/confile.c | 93 +++++++++++++++++++++--------------------------
src/lxc/lxccontainer.c | 13 ++++---
3 files changed, 59 insertions(+), 56 deletions(-)
diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index fe574ac..5ff64f6 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -2396,6 +2396,7 @@ static void lxc_remove_nic(struct lxc_list *it)
free(it2->elem);
free(it2);
}
+ free(netdev);
free(it);
}
@@ -2578,6 +2579,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 cf1c891..bacad01 100644
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -486,17 +486,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;
}
@@ -519,17 +523,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;
}
@@ -785,6 +793,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;
@@ -857,6 +867,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;
@@ -875,6 +887,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;
@@ -939,22 +953,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";
@@ -994,7 +1019,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;
@@ -1009,13 +1034,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) {
@@ -1023,10 +1047,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);
@@ -1053,6 +1073,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;
@@ -1066,50 +1088,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)
@@ -1129,6 +1118,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 37f5ed7..ac995a6 100644
--- 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);
}
--
1.7.1
More information about the lxc-devel
mailing list