2008-04-13 Robert Millan <rmh@aybabtu.com>

Improve robustness when handling LVM.

        * disk/lvm.c (grub_lvm_getvalue): Return 0 when `*p' is NULL
        (and leave `*p' unmodified).
        (grub_lvm_iterate): Don't assume `vg->lvs != NULL' when iterating
        through it.
        (grub_lvm_memberlist): Don't assume `lv->vg->pvs != NULL' when
        iterating through it.
        (grub_lvm_open): Don't assume `vg->lvs != NULL' when iterating
        through it.
        (grub_lvm_scan_device): Check the return value (and fail gracefuly
        when due) on each grub_lvm_getvalue() or grub_strstr() call.
        Don't assume `vg->pvs != NULL' when iterating through it.

ChangeLog

@@ -1,3 +1,19 @@
+2008-04-13  Robert Millan  <rmh@aybabtu.com>
+
+	Improve robustness when handling LVM.
+
+	* disk/lvm.c (grub_lvm_getvalue): Return 0 when `*p' is NULL
+        (and leave `*p' unmodified).
+	(grub_lvm_iterate): Don't assume `vg->lvs != NULL' when iterating
+	through it.
+	(grub_lvm_memberlist): Don't assume `lv->vg->pvs != NULL' when
+	iterating through it.
+	(grub_lvm_open): Don't assume `vg->lvs != NULL' when iterating
+	through it.
+	(grub_lvm_scan_device): Check the return value (and fail gracefuly
+	when due) on each grub_lvm_getvalue() or grub_strstr() call.
+	Don't assume `vg->pvs != NULL' when iterating through it.
+
 2008-04-13  Robert Millan  <rmh@aybabtu.com>
 
 	* gendistlist.sh (EXTRA_DISTFILES): Add `genpartmaplist.sh'.

disk/lvm.c

@@ -29,11 +29,15 @@ static int lv_count;
 
 
 /* Go the string STR and return the number after STR.  *P will point
-   at the number. */
+   at the number.  In case STR is not found, *P will be NULL and the
+   return value will be 0.  */
 static int
 grub_lvm_getvalue (char **p, char *str)
 {
-  *p = grub_strstr (*p, str) + grub_strlen (str);
+  *p = grub_strstr (*p, str);
+  if (! *p)
+    return 0;
+  *p += grub_strlen (str);
   return grub_strtoul (*p, NULL, 10);
 }
 
@@ -44,6 +48,7 @@ grub_lvm_iterate (int (*hook) (const char *name))
   for (vg = vg_list; vg; vg = vg->next)
     {
       struct grub_lvm_lv *lv;
+      if (vg->lvs)
 	for (lv = vg->lvs; lv; lv = lv->next)
 	  if (hook (lv->name))
 	    return 1;
@@ -60,6 +65,7 @@ grub_lvm_memberlist (grub_disk_t disk)
   grub_disk_memberlist_t list = NULL, tmp;
   struct grub_lvm_pv *pv;
 
+  if (lv->vg->pvs)
     for (pv = lv->vg->pvs; pv; pv = pv->next)
       {
 	tmp = grub_malloc (sizeof (*tmp));
@@ -79,6 +85,7 @@ grub_lvm_open (const char *name, grub_disk_t disk)
   struct grub_lvm_lv *lv = NULL;
   for (vg = vg_list; vg; vg = vg->next)
     {
+      if (vg->lvs)
 	for (lv = vg->lvs; lv; lv = lv->next)
 	  if (! grub_strcmp (lv->name, name))
 	    break;
@@ -330,14 +337,18 @@ grub_lvm_scan_device (const char *name)
       grub_memcpy (vg->id, vg_id, GRUB_LVM_ID_STRLEN+1);
 
       vg->extent_size = grub_lvm_getvalue (&p, "extent_size = ");
+      if (p == NULL)
+	goto fail2;
 
       vg->lvs = NULL;
       vg->pvs = NULL;
       vg->next = vg_list;
       vg_list = vg;
 
-      p = grub_strstr (p, "physical_volumes {")
+      p = grub_strstr (p, "physical_volumes {");
-	+ sizeof ("physical_volumes {") - 1;
+      if (p)
+	{
+	  p += sizeof ("physical_volumes {") - 1;
 	  
 	  /* Add all the pvs to the volume group. */
 	  while (1)
@@ -360,19 +371,32 @@ grub_lvm_scan_device (const char *name)
 	      pv->name[s] = '\0';
 	      
 	      p = grub_strstr (p, "id = \"") + sizeof("id = \"") - 1;
+	      if (p == NULL)
+		goto pvs_fail;
 	      
 	      grub_memcpy (pv->id, p, GRUB_LVM_ID_STRLEN);
 	      pv->id[GRUB_LVM_ID_STRLEN] = '\0';
 	      
 	      pv->start = grub_lvm_getvalue (&p, "pe_start = ");
+	      if (p == NULL)
+		goto pvs_fail;
 	      pv->disk = NULL;
 	      pv->next = vg->pvs;
 	      vg->pvs = pv;
 	      
 	      p = grub_strchr (p, '}') + 1;
+	      
+	      continue;
+	    pvs_fail:
+	      grub_free (pv->name);
+	      grub_free (pv);
+	      goto fail2;
+	    }
 	}
 
       p = grub_strstr (p, "logical_volumes");
+      if (p)
+	{
 	  p += 18;
 	  
 	  /* And add all the lvs to the volume group. */
@@ -404,6 +428,8 @@ grub_lvm_scan_device (const char *name)
 	      lv->size = 0;
 	      
 	      lv->segment_count = grub_lvm_getvalue (&p, "segment_count = ");
+	      if (p == NULL)
+		goto lvs_fail;
 	      lv->segments = grub_malloc (sizeof (*seg) * lv->segment_count);
 	      seg = lv->segments;
 	      
@@ -412,10 +438,18 @@ grub_lvm_scan_device (const char *name)
 		  struct grub_lvm_stripe *stripe;
 		  
 		  p = grub_strstr (p, "segment");
+		  if (p == NULL)
+		    goto lvs_segment_fail;
 		  
 		  seg->start_extent = grub_lvm_getvalue (&p, "start_extent = ");
+		  if (p == NULL)
+		    goto lvs_segment_fail;
 		  seg->extent_count = grub_lvm_getvalue (&p, "extent_count = ");
+		  if (p == NULL)
+		    goto lvs_segment_fail;
 		  seg->stripe_count = grub_lvm_getvalue (&p, "stripe_count = ");
+		  if (p == NULL)
+		    goto lvs_segment_fail;
 		  
 		  lv->size += seg->extent_count * vg->extent_size;
 		  
@@ -426,14 +460,19 @@ grub_lvm_scan_device (const char *name)
 					      * seg->stripe_count);
 		  stripe = seg->stripes;
 		  
-	      p = grub_strstr (p, "stripes = [")
+		  p = grub_strstr (p, "stripes = [");
-		+ sizeof("stripes = [") - 1;
+		  if (p == NULL)
+		    goto lvs_segment_fail2;
+		  p += sizeof("stripes = [") - 1;
 		  
 		  for (j = 0; j < seg->stripe_count; j++)
 		    {
 		      char pvname[10];
 		      
-		  q = p = grub_strchr (p, '"') + 1;
+		      p = grub_strchr (p, '"');
+		      if (p == NULL)
+			continue;
+		      q = ++p;
 		      while (*q != '"')
 			q++;
 
@@ -441,6 +480,7 @@ grub_lvm_scan_device (const char *name)
 		      grub_memcpy (pvname, p, s);
 		      pvname[s] = '\0';
 		      
+		      if (vg->pvs)
 			for (pv = vg->pvs; pv; pv = pv->next)
 			  {
 			    if (! grub_strcmp (pvname, pv->name))
@@ -450,13 +490,20 @@ grub_lvm_scan_device (const char *name)
 			      }
 			  }
 		      
-		  p = grub_strchr (p, ',') + 1;
+		      stripe->start = grub_lvm_getvalue (&p, ",");
-		  stripe->start = grub_strtoul (p, NULL, 10);
+		      if (p == NULL)
+			continue;
 		      
 		      stripe++;
 		    }
 		  
 		  seg++;
+
+		  continue;
+		lvs_segment_fail2:
+		  grub_free (seg->stripes);
+		lvs_segment_fail:
+		  goto fail2;
 		}
 
 	      lv->number = lv_count++;
@@ -464,7 +511,17 @@ grub_lvm_scan_device (const char *name)
 	      lv->next = vg->lvs;
 	      vg->lvs = lv;
 	      
-	  p = grub_strchr (p, '}') + 3;
+	      p = grub_strchr (p, '}');
+	      if (p == NULL)
+		goto lvs_fail;
+	      p += 3;
+	      
+	      continue;
+	    lvs_fail:
+	      grub_free (lv->name);
+	      grub_free (lv);
+	      goto fail2;
+	    }
 	}
     }
   else
@@ -474,6 +531,7 @@ grub_lvm_scan_device (const char *name)
 
   /* Match the device we are currently reading from with the right
      PV. */
+  if (vg->pvs)
     for (pv = vg->pvs; pv; pv = pv->next)
       {
 	if (! grub_memcmp (pv->id, pv_id, GRUB_LVM_ID_STRLEN))