[lxc-devel] [PATCH] Fix use of list item memory after free
Serge Hallyn
serge.hallyn at canonical.com
Mon Nov 26 17:40:40 UTC 2012
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>
> ---
> 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
More information about the lxc-devel
mailing list