[lxc-devel] [PATCH] Free allocated configuration memory
Stéphane Graber
stgraber at ubuntu.com
Mon Nov 26 18:14:45 UTC 2012
On 11/26/2012 01:07 PM, Serge Hallyn wrote:
> 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>
Thanks, applied to staging.
>> ---
>> 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
>
> ------------------------------------------------------------------------------
> 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
>
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 899 bytes
Desc: OpenPGP digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20121126/d2a5f51a/attachment.pgp>
More information about the lxc-devel
mailing list