disk/ldm: Fix memory leaks

Fix memory leaks in make_vg() with new helper functions, free_pv()
and free_lv(). Additionally, correct a check after allocating
comp->segments->nodes that mistakenly checked lv->segments->nodes
instead, likely due to a copy-paste error.

Fixes: CID 473878
Fixes: CID 473884
Fixes: CID 473889
Fixes: CID 473890
Signed-off-by: Lidong Chen <lidong.chen@oracle.com>
Reviewed-by: Vladimir Serbinenko <phcoder@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

diff --git a/grub-core/disk/ldm.c b/grub-core/disk/ldm.c
index 048e29cd09547048dabe2af4886061edf63cb027..0f4f22aaa0f96330d84368f4ad18a2a61e96d302 100644 (file)
--- a/grub-core/disk/ldm.c
+++ b/grub-core/disk/ldm.c
@@ -179,6 +179,36 @@ gpt_ldm_sector (grub_disk_t dsk)
   return sector;
 }
 
+static void
+free_pv (struct grub_diskfilter_pv *pv)
+{
+  if (pv == NULL)
+    return;
+
+  grub_free (pv->internal_id);
+  grub_free (pv->id.uuid);
+  grub_free (pv);
+}
+
+static void
+free_lv (struct grub_diskfilter_lv *lv)
+{
+  if (lv == NULL)
+    return;
+
+  grub_free (lv->internal_id);
+  grub_free (lv->name);
+  grub_free (lv->fullname);
+  if (lv->segments)
+    {
+      unsigned int i;
+      for (i = 0; i < lv->segment_count; i++)
+       grub_free (lv->segments[i].nodes);
+      grub_free (lv->segments);
+    }
+  grub_free (lv);
+}
+
 static struct grub_diskfilter_vg *
 make_vg (grub_disk_t disk,
         const struct grub_ldm_label *label)
@@ -196,12 +226,8 @@ make_vg (grub_disk_t disk,
   vg->name = grub_malloc (LDM_NAME_STRLEN + 1);
   vg->uuid = grub_malloc (LDM_GUID_STRLEN + 1);
   if (! vg->uuid || !vg->name)
-    {
-      grub_free (vg->uuid);
-      grub_free (vg->name);
-      grub_free (vg);
-      return NULL;
-    }
+    goto fail1;
+
   grub_memcpy (vg->uuid, label->group_guid, LDM_GUID_STRLEN);
   grub_memcpy (vg->name, label->group_name, LDM_NAME_STRLEN);
   vg->name[LDM_NAME_STRLEN] = 0;
@@ -261,7 +287,7 @@ make_vg (grub_disk_t disk,
          pv->internal_id = grub_malloc (sz);
          if (!pv->internal_id)
            {
-             grub_free (pv);
+             free_pv (pv);
              goto fail2;
            }
          grub_memcpy (pv->internal_id, ptr, (grub_size_t) ptr[0] + 1);
@@ -271,7 +297,7 @@ make_vg (grub_disk_t disk,
          if (ptr + *ptr + 1 >= vblk[i].dynamic
              + sizeof (vblk[i].dynamic))
            {
-             grub_free (pv);
+             free_pv (pv);
              goto fail2;
            }
          /* ptr = name.  */
@@ -279,23 +305,21 @@ make_vg (grub_disk_t disk,
          if (ptr + *ptr + 1
              >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
            {
-             grub_free (pv);
+             free_pv (pv);
              goto fail2;
            }
          pv->id.uuidlen = *ptr;
 
          if (grub_add (pv->id.uuidlen, 1, &sz))
            {
-             grub_free (pv->internal_id);
-             grub_free (pv);
+             free_pv (pv);
              goto fail2;
            }
 
          pv->id.uuid = grub_malloc (sz);
          if (pv->id.uuid == NULL)
            {
-             grub_free (pv->internal_id);
-             grub_free (pv);
+             free_pv (pv);
              goto fail2;
            }
          grub_memcpy (pv->id.uuid, ptr + 1, pv->id.uuidlen);
@@ -343,7 +367,7 @@ make_vg (grub_disk_t disk,
          lv->segments = grub_zalloc (sizeof (*lv->segments));
          if (!lv->segments)
            {
-             grub_free (lv);
+             free_lv (lv);
              goto fail2;
            }
          lv->segments->start_extent = 0;
@@ -354,26 +378,25 @@ make_vg (grub_disk_t disk,
                                             sizeof (*lv->segments->nodes));
          if (!lv->segments->nodes)
            {
-             grub_free (lv);
+             free_lv (lv);
              goto fail2;
            }
          ptr = vblk[i].dynamic;
          if (ptr + *ptr + 1 >= vblk[i].dynamic
              + sizeof (vblk[i].dynamic))
            {
-             grub_free (lv);
+             free_lv (lv);
              goto fail2;
            }
          if (grub_add (ptr[0], 2, &sz))
            {
-             grub_free (lv->segments);
-             grub_free (lv);
+             free_lv (lv);
              goto fail2;
            }
          lv->internal_id = grub_malloc (sz);
          if (!lv->internal_id)
            {
-             grub_free (lv);
+             free_lv (lv);
              goto fail2;
            }
          grub_memcpy (lv->internal_id, ptr, ptr[0] + 1);
@@ -383,20 +406,18 @@ make_vg (grub_disk_t disk,
          if (ptr + *ptr + 1 >= vblk[i].dynamic
              + sizeof (vblk[i].dynamic))
            {
-             grub_free (lv);
+             free_lv (lv);
              goto fail2;
            }
          if (grub_add (*ptr, 1, &sz))
            {
-             grub_free (lv->internal_id);
-             grub_free (lv);
+             free_lv (lv);
              goto fail2;
            }
          lv->name = grub_malloc (sz);
          if (!lv->name)
            {
-             grub_free (lv->internal_id);
-             grub_free (lv);
+             free_lv (lv);
              goto fail2;
            }
          grub_memcpy (lv->name, ptr + 1, *ptr);
@@ -405,36 +426,28 @@ make_vg (grub_disk_t disk,
                                         vg->uuid, lv->name);
          if (!lv->fullname)
            {
-             grub_free (lv->internal_id);
-             grub_free (lv->name);
-             grub_free (lv);
+             free_lv (lv);
              goto fail2;
            }
          ptr += *ptr + 1;
          if (ptr + *ptr + 1
              >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
            {
-             grub_free (lv->internal_id);
-             grub_free (lv->name);
-             grub_free (lv);
+             free_lv (lv);
              goto fail2;
            }
          /* ptr = volume type.  */
          ptr += *ptr + 1;
          if (ptr >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
            {
-             grub_free (lv->internal_id);
-             grub_free (lv->name);
-             grub_free (lv);
+             free_lv (lv);
              goto fail2;
            }
          /* ptr = flags.  */
          ptr += *ptr + 1;
          if (ptr >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
            {
-             grub_free (lv->internal_id);
-             grub_free (lv->name);
-             grub_free (lv);
+             free_lv (lv);
              goto fail2;
            }
 
@@ -443,17 +456,13 @@ make_vg (grub_disk_t disk,
          /* ptr = number of children.  */
          if (ptr >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
            {
-             grub_free (lv->internal_id);
-             grub_free (lv->name);
-             grub_free (lv);
+             free_lv (lv);
              goto fail2;
            }
          ptr += *ptr + 1;
          if (ptr >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
            {
-             grub_free (lv->internal_id);
-             grub_free (lv->name);
-             grub_free (lv);
+             free_lv (lv);
              goto fail2;
            }
 
@@ -463,9 +472,7 @@ make_vg (grub_disk_t disk,
              || ptr + *ptr + 1>= vblk[i].dynamic
              + sizeof (vblk[i].dynamic))
            {
-             grub_free (lv->internal_id);
-             grub_free (lv->name);
-             grub_free (lv);
+             free_lv (lv);
              goto fail2;
            }
          lv->size = read_int (ptr + 1, *ptr);
@@ -515,18 +522,18 @@ make_vg (grub_disk_t disk,
          ptr = vblk[i].dynamic;
          if (ptr + *ptr + 1 >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
            {
-             grub_free (comp);
+             free_lv (comp);
              goto fail2;
            }
          if (grub_add (ptr[0], 2, &sz))
            {
-             grub_free (comp);
+             free_lv (comp);
              goto fail2;
            }
          comp->internal_id = grub_malloc (sz);
          if (!comp->internal_id)
            {
-             grub_free (comp);
+             free_lv (comp);
              goto fail2;
            }
          grub_memcpy (comp->internal_id, ptr, ptr[0] + 1);
@@ -535,16 +542,14 @@ make_vg (grub_disk_t disk,
          ptr += *ptr + 1;
          if (ptr + *ptr + 1 >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
            {
-             grub_free (comp->internal_id);
-             grub_free (comp);
+             free_lv (comp);
              goto fail2;
            }
          /* ptr = name.  */
          ptr += *ptr + 1;
          if (ptr + *ptr + 1 >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
            {
-             grub_free (comp->internal_id);
-             grub_free (comp);
+             free_lv (comp);
              goto fail2;
            }
          /* ptr = state.  */
@@ -554,8 +559,7 @@ make_vg (grub_disk_t disk,
          ptr += 4;
          if (ptr >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
            {
-             grub_free (comp->internal_id);
-             grub_free (comp);
+             free_lv (comp);
              goto fail2;
            }
 
@@ -563,16 +567,14 @@ make_vg (grub_disk_t disk,
          ptr += *ptr + 1;
          if (ptr >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
            {
-             grub_free (comp->internal_id);
-             grub_free (comp);
+             free_lv (comp);
              goto fail2;
            }
          ptr += 8 + 8;
          if (ptr + *ptr + 1 >= vblk[i].dynamic
              + sizeof (vblk[i].dynamic))
            {
-             grub_free (comp->internal_id);
-             grub_free (comp);
+             free_lv (comp);
              goto fail2;
            }
          for (lv = vg->lvs; lv; lv = lv->next)
@@ -583,8 +585,7 @@ make_vg (grub_disk_t disk,
            }
          if (!lv)
            {
-             grub_free (comp->internal_id);
-             grub_free (comp);
+             free_lv (comp);
              continue;
            }
          comp->size = lv->size;
@@ -596,8 +597,7 @@ make_vg (grub_disk_t disk,
                                            sizeof (*comp->segments));
              if (!comp->segments)
                {
-                 grub_free (comp->internal_id);
-                 grub_free (comp);
+                 free_lv (comp);
                  goto fail2;
                }
            }
@@ -608,8 +608,7 @@ make_vg (grub_disk_t disk,
              comp->segments = grub_malloc (sizeof (*comp->segments));
              if (!comp->segments)
                {
-                 grub_free (comp->internal_id);
-                 grub_free (comp);
+                 free_lv (comp);
                  goto fail2;
                }
              comp->segments->start_extent = 0;
@@ -624,27 +623,21 @@ make_vg (grub_disk_t disk,
                }
              else
                {
-                 grub_free (comp->segments);
-                 grub_free (comp->internal_id);
-                 grub_free (comp);
+                 free_lv (comp);
                  goto fail2;
                }
              ptr += *ptr + 1;
              ptr++;
              if (!(vblk[i].flags & 0x10))
                {
-                 grub_free (comp->segments);
-                 grub_free (comp->internal_id);
-                 grub_free (comp);
+                 free_lv (comp);
                  goto fail2;
                }
              if (ptr >= vblk[i].dynamic + sizeof (vblk[i].dynamic)
                  || ptr + *ptr + 1 >= vblk[i].dynamic
                  + sizeof (vblk[i].dynamic))
                {
-                 grub_free (comp->segments);
-                 grub_free (comp->internal_id);
-                 grub_free (comp);
+                 free_lv (comp);
                  goto fail2;
                }
              comp->segments->stripe_size = read_int (ptr + 1, *ptr);
@@ -652,20 +645,16 @@ make_vg (grub_disk_t disk,
              if (ptr + *ptr + 1 >= vblk[i].dynamic
                  + sizeof (vblk[i].dynamic))
                {
-                 grub_free (comp->segments);
-                 grub_free (comp->internal_id);
-                 grub_free (comp);
+                 free_lv (comp);
                  goto fail2;
                }
              comp->segments->node_count = read_int (ptr + 1, *ptr);
              comp->segments->node_alloc = comp->segments->node_count;
              comp->segments->nodes = grub_calloc (comp->segments->node_alloc,
                                                   sizeof (*comp->segments->nodes));
-             if (!lv->segments->nodes)
+             if (comp->segments->nodes == NULL)
                {
-                 grub_free (comp->segments);
-                 grub_free (comp->internal_id);
-                 grub_free (comp);
+                 free_lv (comp);
                  goto fail2;
                }
            }
@@ -677,20 +666,14 @@ make_vg (grub_disk_t disk,
              if (grub_mul (lv->segments->node_alloc, 2, &lv->segments->node_alloc) ||
                  grub_mul (lv->segments->node_alloc, sizeof (*lv->segments->nodes), &sz))
                {
-                 grub_free (comp->segments->nodes);
-                 grub_free (comp->segments);
-                 grub_free (comp->internal_id);
-                 grub_free (comp);
+                 free_lv (comp);
                  goto fail2;
                }
 
              t = grub_realloc (lv->segments->nodes, sz);
              if (!t)
                {
-                 grub_free (comp->segments->nodes);
-                 grub_free (comp->segments);
-                 grub_free (comp->internal_id);
-                 grub_free (comp);
+                 free_lv (comp);
                  goto fail2;
                }
              lv->segments->nodes = t;
@@ -830,7 +813,10 @@ make_vg (grub_disk_t disk,
              comp->segments[comp->segment_count].nodes
                = grub_malloc (sizeof (*comp->segments[comp->segment_count].nodes));
              if (!comp->segments[comp->segment_count].nodes)
-               goto fail2;
+               {
+                 grub_free (comp->segments);
+                 goto fail2;
+               }
              comp->segments[comp->segment_count].nodes[0] = part;
              comp->segment_count++;
            }
@@ -845,25 +831,19 @@ make_vg (grub_disk_t disk,
     struct grub_diskfilter_pv *pv, *next_pv;
     for (lv = vg->lvs; lv; lv = next_lv)
       {
-       unsigned i;
-       for (i = 0; i < lv->segment_count; i++)
-         grub_free (lv->segments[i].nodes);
-
        next_lv = lv->next;
-       grub_free (lv->segments);
-       grub_free (lv->internal_id);
-       grub_free (lv->name);
-       grub_free (lv->fullname);
-       grub_free (lv);
+       free_lv (lv);
+
       }
     for (pv = vg->pvs; pv; pv = next_pv)
       {
        next_pv = pv->next;
-       grub_free (pv->id.uuid);
-       grub_free (pv);
+       free_pv (pv);
       }
   }
+ fail1:
   grub_free (vg->uuid);
+  grub_free (vg->name);
   grub_free (vg);
   return NULL;
 }