Hi,
On Tue, 2008-10-28 at 01:09:24 +0100, Julien Cristau wrote:
> On Mon, Oct 27, 2008 at 23:43:40 +0200, Guillem Jover wrote:
> > +
> > + p = grub_strchr (p, '}') + 1;
> > + if (p == NULL)
> > + goto pvs_fail;
> > +
>
> This test seems buggy. The +1 needs to be after the NULL check.
Bah! Right, didn't notice. Attached now the patch split in two parts,
one fixing this and another instance of this problem and the other one
fixing this current segfault.
regards,
guillem
>From 0eafe5ddadee8e690672b452121eb8cf88736293 Mon Sep 17 00:00:00 2001
From: Guillem Jover <[EMAIL PROTECTED]>
Date: Tue, 28 Oct 2008 18:46:28 +0200
Subject: [PATCH] lvm: Fix possible NULL value handling
Add a missing NULL check, and correct them by moving the pointer
operations after the actual check.
---
disk/lvm.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/disk/lvm.c b/disk/lvm.c
index cd9e447..df8c99b 100644
--- a/disk/lvm.c
+++ b/disk/lvm.c
@@ -373,9 +373,10 @@ grub_lvm_scan_device (const char *name)
grub_memcpy (pv->name, p, s);
pv->name[s] = '\0';
- p = grub_strstr (p, "id = \"") + sizeof("id = \"") - 1;
+ p = grub_strstr (p, "id = \"");
if (p == NULL)
goto pvs_fail;
+ p += sizeof("id = \"") - 1;
grub_memcpy (pv->id, p, GRUB_LVM_ID_STRLEN);
pv->id[GRUB_LVM_ID_STRLEN] = '\0';
@@ -387,7 +388,10 @@ grub_lvm_scan_device (const char *name)
pv->next = vg->pvs;
vg->pvs = pv;
- p = grub_strchr (p, '}') + 1;
+ p = grub_strchr (p, '}');
+ if (p == NULL)
+ goto pvs_fail;
+ p++;
continue;
pvs_fail:
--
1.6.0.2
>From e3ad4a3bea534a1269ecc03d307d189b098d0b62 Mon Sep 17 00:00:00 2001
From: Guillem Jover <[EMAIL PROTECTED]>
Date: Mon, 27 Oct 2008 23:22:06 +0200
Subject: [PATCH] lvm: Fix error recovery by only adding objects when we cannot fail
The error unwinding code was keeping objects referenced in the lists
even when those were being unfreed. Delay the addition of those objects
to the lists until the code is not going to be able to fail, so that
that part does not need to be unwound.
---
disk/lvm.c | 22 ++++++++++++----------
1 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/disk/lvm.c b/disk/lvm.c
index df8c99b..2ee1f7f 100644
--- a/disk/lvm.c
+++ b/disk/lvm.c
@@ -345,8 +345,6 @@ grub_lvm_scan_device (const char *name)
vg->lvs = NULL;
vg->pvs = NULL;
- vg->next = vg_list;
- vg_list = vg;
p = grub_strstr (p, "physical_volumes {");
if (p)
@@ -384,14 +382,15 @@ grub_lvm_scan_device (const char *name)
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, '}');
if (p == NULL)
goto pvs_fail;
p++;
+
+ pv->disk = NULL;
+ pv->next = vg->pvs;
+ vg->pvs = pv;
continue;
pvs_fail:
@@ -520,16 +519,16 @@ grub_lvm_scan_device (const char *name)
goto fail4;
}
- lv->number = lv_count++;
- lv->vg = vg;
- lv->next = vg->lvs;
- vg->lvs = lv;
-
p = grub_strchr (p, '}');
if (p == NULL)
goto lvs_fail;
p += 3;
+ lv->number = lv_count++;
+ lv->vg = vg;
+ lv->next = vg->lvs;
+ vg->lvs = lv;
+
continue;
lvs_fail:
grub_free (lv->name);
@@ -537,6 +536,9 @@ grub_lvm_scan_device (const char *name)
goto fail4;
}
}
+
+ vg->next = vg_list;
+ vg_list = vg;
}
else
{
--
1.6.0.2
