Re: grub-probe gets a segfault

["Isaac M. Marcos" <isaacmarcos100010@gmail.com>]

El Wednesday 28 May 2008 09:06:51 Robert Millan escribió:
> 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).

I also believe It should be included. How is that done?

> > 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)



-- 
Isaac M. Marcos
GPG key 0xC9045C1B
5633 ECAF 44B1 8A5D 9371 DCDA 4620 A016 C904 5C1B

The only place success comes before work is in the dictionary.
VINCE LOMBARDI