[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: segfault in grub_device_iterate on 64bit platforms

From: Dan Merillat
Subject: Re: segfault in grub_device_iterate on 64bit platforms
Date: Tue, 19 Jan 2010 22:25:40 -0500
User-agent: Mozilla-Thunderbird (X11/20090707)

Colin Watson wrote:
On Tue, Jan 19, 2010 at 08:23:44PM -0500, Dan Merillat wrote:
And here's a freebie to avoid a null pointer deref that -lefence pointed

(Nit: stick to the prevailing whitespace style.)

:)  Glad that's the worst problem with my submission.

grub_lvm_memberlist just constructs its return value from lv->vg->pvs,
so we'll need to look at how that's constructed.  The only time pv->disk
is assigned a non-NULL value is in grub_lvm_scan_device:

  /* 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))
            /* This could happen to LVM on RAID, pv->disk points to the
               raid device, we shouldn't change it.  */
            if (! pv->disk)
              pv->disk = grub_disk_open (name);

So it's possible for pv->disk to be NULL if that PV doesn't have an LVM
signature (as I interpret this).  That's either an error condition or an
indication that we aren't managing to understand LVM well enough.  If
you can reproduce this it would be useful if you could clarify which.

Ok, here goes - grub_lvm_scan_device is called once ("md0"), it finds
the LVM signature, searches the vg list to see if we've seen this vg
before, creates a new vg, pv list (disk = NULL) etc.

Then it attaches the disk with this name to the appropriate pv entry in
that code snippet you posted.

So the only way for a PV->disk to be attached is if it is called by name
in grub_lvm_scan_device().

I'm running the following setup:
/dev/md0 pv0 contains lv_root
/dev/sdb2 pv1 contains lv_test1, lv_test2, etc.

invoke as:
# grub-probe -t abstraction /

Grub discovers / is on /dev/md0, probes md0 for LVM information, never
looks at other disks, segfaults on a null pointer dereference because
pv1 disk = NULL.

So either you can ignore it via my patch, or add a loop to get disks by
ID and add their information to the pvs.

Does my analysis make sense?

reply via email to

[Prev in Thread] Current Thread [Next in Thread]