Re: grub-probe gets a segfault

[Robert Millan]

On Thu, May 22, 2008 at 06:52:02AM -0400, Pavel Roskin wrote:
> On Wed, 2008-05-21 at 20:20 -0430, Isaac M. Marcos wrote:
> > Today, I bump on this problem again on updating kernel. Because of this 
> > fault, 
> > update-initramfs reports a failure. It can't find "/".
> > 
> > grub-probe -t device /      -->     segfault!!
> 
> This patch should help.  It checks for the buffer overrun.  I still feel
> uneasy about applying it because it hides a bigger problem.

The constraints are a bit unusual here.  We don't have a handler for
exceptions, so this error could mean your machine stops booting.  And
additionally this code is in fact run in situations where LVM might not
be used (e.g. grub-probe is probing something in an unrelated partition).

In both cases IMHO it's much more desireable to avoid the problem and accept
the lesser evil that our LVM code might not be as complete, but at least it's
robust.

So I'd recommend to get this patch in.  If that makes you feel better about
this, how about adding a warning message in this fail condition?  (though,
note we have many fail conditions like this one in that function, and these
don't have any warning atm).

> diff --git a/disk/lvm.c b/disk/lvm.c
> index 1d898ff..997d15c 100644
> --- a/disk/lvm.c
> +++ b/disk/lvm.c
> @@ -302,9 +302,12 @@ grub_lvm_scan_device (const char *name)
>    rlocn = mdah->raw_locns;
>    p = q = metadatabuf + grub_le_to_cpu64 (rlocn->offset);
>  
> -  while (*q != ' ')
> +  while (*q != ' ' && q < metadatabuf + mda_size)
>      q++;
>  
> +  if (q == metadatabuf + mda_size)
> +    goto fail2;
> +
>    vgname_len = q - p;
>    vgname = grub_malloc (vgname_len + 1);
>    if (!vgname)

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What good is a phone call… if you are unable to speak?
(as seen on /.)