[lxc-devel] [PATCH] Free allocated configuration memory
Serge Hallyn
serge.hallyn at canonical.com
Mon Nov 26 18:07:57 UTC 2012
Quoting Dwight Engen (dwight.engen at oracle.com):
> 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>
Thanks, Dwight.
Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.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
>
>
> ------------------------------------------------------------------------------
> Monitor your physical, virtual and cloud infrastructure from a single
> web console. Get in-depth insight into apps, servers, databases, vmware,
> SAP, cloud infrastructure, etc. Download 30-day Free Trial.
> Pricing starts from $795 for 25 servers or applications!
> http://p.sf.net/sfu/zoho_dev2dev_nov
> _______________________________________________
> Lxc-devel mailing list
> Lxc-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/lxc-devel
More information about the lxc-devel
mailing list