[lxc-devel] [PATCH] Fix use of list item memory after free
Stéphane Graber
stgraber at ubuntu.com
Mon Nov 26 17:55:28 UTC 2012
On 11/26/2012 12:40 PM, Serge Hallyn wrote:
> Quoting Dwight Engen (dwight.engen at oracle.com):
>> 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 at oracle.com>
>
> Thanks (boy do I feel like an idiot)
>
> Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
Thanks, applied to staging.
>> ---
>> src/lxc/conf.c | 38 +++++++++++++++++++-------------------
>> src/lxc/confile.c | 4 ++--
>> src/lxc/list.h | 5 +++++
>> 3 files changed, 26 insertions(+), 21 deletions(-)
>>
>> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
>> index fe574ac..79760a0 100644
>> --- a/src/lxc/conf.c
>> +++ b/src/lxc/conf.c
>> @@ -725,7 +725,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;
>>
>> @@ -765,7 +765,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)) {
>> @@ -2368,7 +2368,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);
>>
>> @@ -2386,12 +2386,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);
>> @@ -2433,15 +2433,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);
>> @@ -2489,8 +2489,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;
>> @@ -2498,9 +2498,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);
>> @@ -2510,14 +2510,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;
>> @@ -2532,9 +2532,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);
>> @@ -2544,7 +2544,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;
>> @@ -2554,7 +2554,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 cf1c891..8a08f58 100644
>> --- a/src/lxc/confile.c
>> +++ b/src/lxc/confile.c
>> @@ -1219,7 +1219,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) {
>> @@ -1228,7 +1228,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 5213e80..24dffa2 100644
>> --- 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;
>> --
>> 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/5a4cd601/attachment.pgp>
More information about the lxc-devel
mailing list