grub-probe gets a segfault

grub-probe gets a segfault

[Isaac M. Marcos]

I have open a ticket in bugs.debian.org numbered 478401 referring to a 
segfault that grub-probe is getting.

Robert Millan suggested to bring this problem here.

What should i do?
Am i in the right place?

-- 
Isaac M. Marcos

Re: grub-probe gets a segfault

[Pavel Roskin]

On Thu, 2008-05-08 at 04:54 -0430, Isaac M. Marcos wrote:
> I have open a ticket in bugs.debian.org numbered 478401 referring to a 
> segfault that grub-probe is getting.

URL for convenience of others:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=478401

> Robert Millan suggested to bring this problem here.
> 
> What should i do?

Try running update-grub manually.  Try reproducing the crash in a way
that doesn't involve anything Debian specific.  Try the CVS version.
Try debugging the codedump.

> Am i in the right place?

Yes

-- 
Regards,
Pavel Roskin

Re: grub-probe gets a segfault

[Isaac M. Marcos]

On Thursday 08 May 2008 11:25:24 Pavel Roskin wrote:
> On Thu, 2008-05-08 at 04:54 -0430, Isaac M. Marcos wrote:
> > I have open a ticket in bugs.debian.org numbered 478401 referring to a
> > segfault that grub-probe is getting.
>
> URL for convenience of others:
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=478401
>
> > Robert Millan suggested to bring this problem here.
> >
> > What should i do?
>
> Try running update-grub manually.  Try reproducing the crash in a way
> that doesn't involve anything Debian specific.

Does:
# grub-probe -t device / 
involve anything Debian specific ????????

>  Try the CVS version.  Try debugging the codedump.

Sure, only if i knew how, :-)

> > Am i in the right place?
>
> Yes

Good, next step?

-- 
Isaac M. Marcos

Re: grub-probe gets a segfault

[Pavel Roskin]

On Thu, 2008-05-08 at 11:53 -0430, Isaac M. Marcos wrote:

> Does:
> # grub-probe -t device / 
> involve anything Debian specific ????????

It was a generic recommendation.  But if grub-probe was compiled by
Debian, it's Debian specific in a certain sense.

I cannot reproduce the crash, even on a Fedora installed on a LVM
volume.

> >  Try the CVS version.  Try debugging the codedump.
> 
> Sure, only if i knew how, :-)

That's how to check out GRUB from CVS:

cvs -d :pserver:anoncvs@cvs.savannah.gnu.org:/cvsroot/grub co grub2

To build it, use:

./configure
make

As for debugging, I see you have found something already.

> Good, next step?

Try the version from CVS.  Describe the problem in a concise way, so
that nobody has to go through the whole discussion in the Debian bug
tracker.  Try using grub-probe on paths that are not on LVM (like a
mounted floppy or a flash drive).

-- 
Regards,
Pavel Roskin

Re: grub-probe gets a segfault

[Robert Millan]

On Thu, May 08, 2008 at 06:15:27PM -0400, Pavel Roskin wrote:
> On Thu, 2008-05-08 at 11:53 -0430, Isaac M. Marcos wrote:
> 
> > Does:
> > # grub-probe -t device / 
> > involve anything Debian specific ????????
> 
> It was a generic recommendation.  But if grub-probe was compiled by
> Debian, it's Debian specific in a certain sense.

I don't think there's any change in Debian affecting this area.  But anyway I
recommend that you follow Pavel's advice and run it from CVS directly.

> I cannot reproduce the crash, even on a Fedora installed on a LVM
> volume.

Neither could I :-(

> > Good, next step?

I'm thinking that perhaps we can check the offset values against the bounds
of the structure we have allocated, and abort if they miss.  We need to do
this anyway to support broken PVs, but I don't know if this is hiding a bug
in our code somewhere else.

-- 
Robert Millan

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

Re: grub-probe gets a segfault

[Isaac M. Marcos]

El Friday 09 May 2008 08:32:01 Robert Millan escribió:
> On Thu, May 08, 2008 at 06:15:27PM -0400, Pavel Roskin wrote:
> > On Thu, 2008-05-08 at 11:53 -0430, Isaac M. Marcos wrote:
> > > Does:
> > > # grub-probe -t device /
> > > involve anything Debian specific ????????
> >
> > It was a generic recommendation.  But if grub-probe was compiled by
> > Debian, it's Debian specific in a certain sense.
>
> I don't think there's any change in Debian affecting this area.  But anyway
> I recommend that you follow Pavel's advice and run it from CVS directly.

Yes, i will, but it would have to wait a bit. There are other things to do.

> > I cannot reproduce the crash, even on a Fedora installed on a LVM
> > volume.
>
> Neither could I :-(

It does not happen in other two AMD64 machines here!!
Only on this one.

> > > Good, next step?
>
> I'm thinking that perhaps we can check the offset values against the bounds
> of the structure we have allocated, and abort if they miss.  We need to do
> this anyway to support broken PVs, but I don't know if this is hiding a bug
> in our code somewhere else.

Could you hold your idea for a little time? Thanks.

-- 
Isaac M. Marcos

Re: grub-probe gets a segfault

[Pavel Roskin]

On Fri, 2008-05-09 at 12:13 -0430, Isaac M. Marcos wrote:

> > Neither could I :-(
> 
> It does not happen in other two AMD64 machines here!!
> Only on this one.

That confirms my suspicion that the problem is not related to any x86_64
issues.

I checked the bug history more carefully, and I think that rlocn->offset
may be correct.  92672 is 0x16a00, which is a nice rounded number.

If q is out of bounds, it probably means that mda_size was less than
0x16a00.  Could you please print mda_size in the debugger?

Also, pvck should display the metadata size.  Please run this:

pvck /dev/sda6

What's the metadata size?  It is the same as mda_size should by gdb?

-- 
Regards,
Pavel Roskin

Re: grub-probe gets a segfault

[Isaac M. Marcos]

El Friday 09 May 2008 16:26:28 Pavel Roskin escribió:
> On Fri, 2008-05-09 at 12:13 -0430, Isaac M. Marcos wrote:
> > > Neither could I :-(
> >
> > It does not happen in other two AMD64 machines here!!
> > Only on this one.
>
> That confirms my suspicion that the problem is not related to any x86_64
> issues.
>
> I checked the bug history more carefully, and I think that rlocn->offset
> may be correct.  92672 is 0x16a00, which is a nice rounded number.
>
> If q is out of bounds, it probably means that mda_size was less than
> 0x16a00.  Could you please print mda_size in the debugger?

Program received signal SIGSEGV, Segmentation fault.
0x0000000000421b7c in grub_lvm_scan_device (name=0x638210 "hd1,5") 
at /home/immf/grub2-1.96+20080429/disk/lvm.c:305
305     /home/immf/grub2-1.96+20080429/disk/lvm.c: No such file or directory.
        in /home/immf/grub2-1.96+20080429/disk/lvm.c
(gdb) print mda_size
$1 = 192512

not what you though :-(

>
> Also, pvck should display the metadata size.  Please run this:
>
> pvck /dev/sda6

$ sudo pvck /dev/sda6
  Device /dev/sda6 not found (or ignored by filtering).

But fdisk reports the disks:

$ sudo fdisk -l
Disco /dev/sda: 120.0 GB, 120034123776 bytes
255 heads, 63 sectors/track, 14593 cylinders
Units = cilindros of 16065 * 512 = 8225280 bytes
Disk identifier: 0x680c4421
Disposit. Inicio    Comienzo      Fin      Bloques  Id  Sistema
/dev/sda1   *           1          26      208813+  83  Linux
/dev/sda2              27         153     1020127+  83  Linux
/dev/sda3             154         231      626535   82  Linux swap / Solaris
/dev/sda4             232       14593   115362765    5  Extendida
/dev/sda5             232        3396    25422831   fd  Linux raid autodetect
/dev/sda6            3397        6561    25422831   fd  Linux raid autodetect
/dev/sda7            6562        9726    25422831   fd  Linux raid autodetect
/dev/sda8            9727       14593    39094146   8e  Linux LVM
[ some other disks ....]

> What's the metadata size?  It is the same as mda_size should by gdb?

It does not seem so!!


------------------ with cvs version -------------------------------
> That's how to check out GRUB from CVS:
> cvs -d :pserver:anoncvs@cvs.savannah.gnu.org:/cvsroot/grub co grub2
done
> To build it, use:
> ./configure
> make
done OK,

/_____________________________________
# ./grub-probe -t device /
Violación de segmento

# gdb ./grub-probe
GNU gdb 6.7.1-debian
Copyright (C) 2007 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu"...
Using host libthread_db library "/lib/libthread_db.so.1".
(gdb) run -t device /
Starting program: /home/immf/DescargaFirefox/grub-cvs/grub2/grub-probe -t 
device /

Program received signal SIGSEGV, Segmentation fault.
grub_lvm_scan_device (name=0x62d1f0 "hd1,5") at disk/lvm.c:305
305       while (*q != ' ')
(gdb) print q
$1 = 0x780000 <Address 0x780000 out of bounds>
(gdb) print metadatabuf
$2 = <value optimized out>
(gdb) print rlocn->offset
No symbol "rlocn" in current context.
(gdb) print mda_size
$3 = <value optimized out>
(gdb)                         
_____________________________________________/
cvs version does not contain debug symbols?
Is this a useful "line of work"?

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

Re: grub-probe gets a segfault

[Isaac M. Marcos]

On Thursday 08 May 2008 17:45:27 Pavel Roskin wrote:
> On Thu, 2008-05-08 at 11:53 -0430, Isaac M. Marcos wrote:
> > >  Try the CVS version.  Try debugging the codedump.
> >

Done, same segfault.

> As for debugging, I see you have found something already.
>
> > Good, next step?
>
> Try the version from CVS.  Describe the problem in a concise way, so
> that nobody has to go through the whole discussion in the Debian bug
> tracker.  Try using grub-probe on paths that are not on LVM (like a
> mounted floppy or a flash drive).

Neither" /" or "/boot" are in a LVM partition!!!!!!!!!

cvs version 
# ./grub-probe -t device /
	Violación de segmento
# ./grub-probe -t device /boot
	Violación de segmento
# ./grub-probe -t device /dev/cdrom		# cd
	Violación de segmento
# ./grub-probe -t device /fd0				# floppy
	Violación de segmento
# ./grub-probe -t device dvd				# dvd
	Violación de segmento
# ./grub-probe -t device /dev/	sdg1			# USB pen drive
	Violación de segmento
# ./grub-probe help						# ?? anything
	Violación de segmento
# ./grub-probe -help
	Works
# ./grub-probe --help
	Works

system version
# grub-probe -t device /
	Violación de segmento
# grub-probe -t device /boot
	Violación de segmento
# grub-probe help						# ?? again
	Violación de segmento
# grub-probe -help
	Works ok!

Same commands tested on two other machines (one with lvm's one without)
All commands don't segfault on the system version (not cvs).

Could you make anything out of this?
Does this help in any way?

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

Re: grub-probe gets a segfault

[Pavel Roskin]

On Mon, 2008-05-12 at 09:14 -0430, Isaac M. Marcos wrote:

> (gdb) print mda_size
> $1 = 192512
> 
> not what you though :-(

OK, we have something interesting here.  mda_size is 192512 (0x2f000).
rlocn->offset is 92672 (0x16a00).  But (q - metadatabuf) is 325120
(0x4f600).  So, the initial value of q was within the buffer, but the
value that caused the segfault was outside the buffer.

It means that GRUB could not find any space character in the buffer and
kept scanning until it hit unallocated space.  I believe it's not valid
LVM metadata.

> $ sudo pvck /dev/sda6
>   Device /dev/sda6 not found (or ignored by filtering).

So, it's not a LVM physical volume.

> /dev/sda6            3397        6561    25422831   fd  Linux raid autodetect

It's a RAID partition.  I guess RAID partitions have a different layout.

The fix would be to reject RAID partitions in grub_lvm_scan_device().
The only validity check in that function is presence of the LVM label in
the first 4 sectors.  Perhaps additional checks are needed.

-- 
Regards,
Pavel Roskin

Re: grub-probe gets a segfault

[Isaac Marcos]

[-- Attachment #1: Type: text/plain, Size: 1463 bytes --]

2008/5/16 Pavel Roskin <proski@gnu.org>:

> On Mon, 2008-05-12 at 09:14 -0430, Isaac M. Marcos wrote:
>
> > (gdb) print mda_size
> > $1 = 192512
> >
> > not what you though :-(
>
> OK, we have something interesting here.  mda_size is 192512 (0x2f000).
> rlocn->offset is 92672 (0x16a00).  But (q - metadatabuf) is 325120
> (0x4f600).  So, the initial value of q was within the buffer, but the
> value that caused the segfault was outside the buffer.
>
> It means that GRUB could not find any space character in the buffer and
> kept scanning until it hit unallocated space.  I believe it's not valid
> LVM metadata.
>
> > $ sudo pvck /dev/sda6
> >   Device /dev/sda6 not found (or ignored by filtering).
>
> So, it's not a LVM physical volume.
>
> > /dev/sda6            3397        6561    25422831   fd  Linux raid
> autodetect
>
> It's a RAID partition.  I guess RAID partitions have a different layout.
>
> The fix would be to reject RAID partitions in grub_lvm_scan_device().
> The only validity check in that function is presence of the LVM label in
> the first 4 sectors.  Perhaps additional checks are needed.
>
>
Good find Pavel. :-)

Just a question, /dev/sda5 has an equivalent setup:
# pvck /dev/sda5
  Device /dev/sda5 not found (or ignored by filtering).
/dev/sda5             232        3396    25422831   fd  Linux raid
autodetect

Why grub-probe doesn't fail on /dev/sda5?

-- 
Cases are always threesome:
Best case, Worst case, and Just in case

[-- Attachment #2: Type: text/html, Size: 2202 bytes --]

Re: grub-probe gets a segfault

[Pavel Roskin]

On Sat, 2008-05-17 at 13:18 +0000, Isaac Marcos wrote:

>         The fix would be to reject RAID partitions in
>         grub_lvm_scan_device().
>         The only validity check in that function is presence of the
>         LVM label in
>         the first 4 sectors.  Perhaps additional checks are needed.

> Good find Pavel. :-)
> 
> Just a question, /dev/sda5 has an equivalent setup:
> # pvck /dev/sda5
>   Device /dev/sda5 not found (or ignored by filtering).
> /dev/sda5             232        3396    25422831   fd  Linux raid
> autodetect
> 
> Why grub-probe doesn't fail on /dev/sda5?

I think because it has no LVM label.  Maybe /dev/sda6 was holding a LVM
physical volume before, and the label was not erased when it became a
RAID partition.

As you can see, pvck doesn't display information partition.  It means it
cannot be fooled by the label.  And that's what GRUB should do.

-- 
Regards,
Pavel Roskin

Re: grub-probe gets a segfault

[Jeff Chua]

On Mon, May 12, 2008 at 10:16 PM, Isaac M. Marcos
<isaacmarcos100010@gmail.com> wrote:
> On Thursday 08 May 2008 17:45:27 Pavel Roskin wrote:
>> On Thu, 2008-05-08 at 11:53 -0430, Isaac M. Marcos wrote:
>> > >  Try the CVS version.  Try debugging the codedump.
>> >
>
> Done, same segfault.

I'm seeing a similar segmentation fault today on a new Dell R200
(quad-core). The same version (everything the same) works on my
dual-core X60s.

Command is 'grub-probe -d /dev/sda14'

Using an older CVS version from February works fine. I'll need to
verify further as the older grub version is run on 2.6.25-rc and now
I'm in 2.6.26-rc3. Don't know whether that makes a difference. And
partition is reiserfs.

Jeff.

Re: grub-probe gets a segfault

[Pavel Roskin]

On Wed, 2008-05-21 at 00:37 +0800, Jeff Chua wrote:

> I'm seeing a similar segmentation fault today on a new Dell R200
> (quad-core). The same version (everything the same) works on my
> dual-core X60s.
> 
> Command is 'grub-probe -d /dev/sda14'
> 
> Using an older CVS version from February works fine. I'll need to
> verify further as the older grub version is run on 2.6.25-rc and now
> I'm in 2.6.26-rc3. Don't know whether that makes a difference. And
> partition is reiserfs.

The original report was about a segfault due to a RAID partition
mistaken for an LVM partition.  I don't see any information in your
report suggesting that your case is similar.

-- 
Regards,
Pavel Roskin

Re: grub-probe gets a segfault

[Jeff Chua]

On Wed, May 21, 2008 at 3:53 AM, Pavel Roskin <proski@gnu.org> wrote:
> On Wed, 2008-05-21 at 00:37 +0800, Jeff Chua wrote:
>
>> I'm seeing a similar segmentation fault today on a new Dell R200
>> (quad-core). The same version (everything the same) works on my
>> dual-core X60s.
>>
>> Command is 'grub-probe -d /dev/sda14'
>>
>> Using an older CVS version from February works fine. I'll need to
>> verify further as the older grub version is run on 2.6.25-rc and now
>> I'm in 2.6.26-rc3. Don't know whether that makes a difference. And
>> partition is reiserfs.
>
> The original report was about a segfault due to a RAID partition
> mistaken for an LVM partition.  I don't see any information in your
> report suggesting that your case is similar.

Sorry. But, I got segmentation fault and I don't know why. But I dig
into it when I get back to office tomorrow.

Thanks,
Jeff.

Re: grub-probe gets a segfault

[Isaac M. Marcos]

[-- Attachment #1: Type: text/plain, Size: 2692 bytes --]

El Friday 16 May 2008 13:32:40 Pavel Roskin escribió:
> On Sat, 2008-05-17 at 13:18 +0000, Isaac Marcos wrote:
> >         The fix would be to reject RAID partitions in
> >         grub_lvm_scan_device().
> >         The only validity check in that function is presence of the
> >         LVM label in
> >         the first 4 sectors.  Perhaps additional checks are needed.
> >
> > Good find Pavel. :-)
> >
> > Just a question, /dev/sda5 has an equivalent setup:
> > # pvck /dev/sda5
> >   Device /dev/sda5 not found (or ignored by filtering).
> > /dev/sda5             232        3396    25422831   fd  Linux raid
> > autodetect
> >
> > Why grub-probe doesn't fail on /dev/sda5?
>
> I think because it has no LVM label.  Maybe /dev/sda6 was holding a LVM
> physical volume before, and the label was not erased when it became a
> RAID partition.

I cheked, and both partitions (sda5,sda6) have LVM labels, and correctly so:

/dev/sda5 (for example):
0000:0200 LABELONE........?ÍÉC ...LVM2 001npzaBI7ZXIhG29Uxcg1lkUIlTNTS6wbR
0000:0240 .../.............................................ð..............

sda5,sdb5,sdc5,sdd5 form a RAID5 disk (md1) ON which LVM was set up.
sda6,sdb6,sdc6,sdd6 form a RAID5 disk (md2) ON which LVM was set up.

pvck /dev/md1
  Found label on /dev/md1, sector 1, type=LVM2 001
  Found text metadata area: offset=4096, size=192512
pvck /dev/md2
  Found label on /dev/md2, sector 1, type=LVM2 001
  Found text metadata area: offset=4096, size=192512

Some kind of deep interaction is going on?


> As you can see, pvck doesn't display information partition.  It means it
> cannot be fooled by the label.  And that's what GRUB should do.

Yes, but in the meanwhile :

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

Let me explain something:
Neither "/" or "/boot" has anything to do with RAID or LVM in this system. It 
was setup like this exactly to prevent this kind of interferences on boot. 
They are "/dev/sda1" and "/dev/sda2", plain ext3 partitions.

And here comes grub-probe, testing every disk and partition on the system, 
testing for LVM, and many other things, and failing in the process.


I think it boils down to this main idea:
Could it test simple partitions first and if / is found, stop scanning?
Why it needs to scan anything else if the requested partition has been found?

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: grub-probe gets a segfault

[Pavel Roskin]

On Wed, 2008-05-21 at 20:20 -0430, Isaac M. Marcos wrote:
> El Friday 16 May 2008 13:32:40 Pavel Roskin escribió:

> > I think because it has no LVM label.  Maybe /dev/sda6 was holding a LVM
> > physical volume before, and the label was not erased when it became a
> > RAID partition.
> 
> I cheked, and both partitions (sda5,sda6) have LVM labels, and correctly so:
> 
> /dev/sda5 (for example):
> 0000:0200 LABELONE........?ÍÉC ...LVM2 001npzaBI7ZXIhG29Uxcg1lkUIlTNTS6wbR
> 0000:0240 .../.............................................ð..............
> 
> sda5,sdb5,sdc5,sdd5 form a RAID5 disk (md1) ON which LVM was set up.
> sda6,sdb6,sdc6,sdd6 form a RAID5 disk (md2) ON which LVM was set up.

I suspected something like that.

> pvck /dev/md1
>   Found label on /dev/md1, sector 1, type=LVM2 001
>   Found text metadata area: offset=4096, size=192512
> pvck /dev/md2
>   Found label on /dev/md2, sector 1, type=LVM2 001
>   Found text metadata area: offset=4096, size=192512
> 
> Some kind of deep interaction is going on?

Nothing deep, really.  It just shows that searching for some marker on a
partition is inherently wrong.  Whatever fixed marker is used, there is
a substantial probability of it being found without it being a valid
marker.  Data on partitions is not random.

> Let me explain something:
> Neither "/" or "/boot" has anything to do with RAID or LVM in this system. It 
> was setup like this exactly to prevent this kind of interferences on boot. 
> They are "/dev/sda1" and "/dev/sda2", plain ext3 partitions.
> 
> And here comes grub-probe, testing every disk and partition on the system, 
> testing for LVM, and many other things, and failing in the process.

I can fix the LVM code not to overrun the buffer.  The right solution
would be to improve check for LVM partitions to match pvck behavior (in
addition to the overrun check).

> I think it boils down to this main idea:
> Could it test simple partitions first and if / is found, stop scanning?
> Why it needs to scan anything else if the requested partition has been found?

I think the idea is to reuse the code used in the bootloader.  We want
to give the user the full list of partitions at the boot time.  And we
don't want the bootloader to hang.  It's better to check the same code
while in the OS and find possible problems early and in the protected
environment.

-- 
Regards,
Pavel Roskin

Re: grub-probe gets a segfault

[Pavel Roskin]

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.

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)


-- 
Regards,
Pavel Roskin

Re: grub-probe gets a segfault

[Isaac M. Marcos]

[-- Attachment #1: Type: text/plain, Size: 1414 bytes --]

El Thursday 22 May 2008 06:22:02 Pavel Roskin escribió:
> 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.

I *completely* agree with you.

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

Sorry for the delay, other problems had higher priority :-)

Fine, to the problem at hand:

Patch applied, compiled, and  working OK:
	$ sudo grub-probe -t device /
	/dev/sda2

Would this change be committed to cvs?

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: grub-probe gets a segfault

[Pavel Roskin]

On Fri, 2008-05-23 at 19:40 -0430, Isaac M. Marcos wrote:

> Sorry for the delay, other problems had higher priority :-)
> 
> Fine, to the problem at hand:
> 
> Patch applied, compiled, and  working OK:
> 	$ sudo grub-probe -t device /
> 	/dev/sda2
> 
> Would this change be committed to cvs?

Probably not.  There are more sanity check that are needed there, so I'm
leaving it to whoever is familiar with LVM.

-- 
Regards,
Pavel Roskin

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

Re: grub-probe gets a segfault

[Isaac M. Marcos]

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

Re: grub-probe gets a segfault

[Pavel Roskin]

Quoting "Isaac M. Marcos" <isaacmarcos100010@gmail.com>:

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

OK, I've applied the patch.

-- 
Regards,
Pavel Roskin