All of lore.kernel.org
 help / color / mirror / Atom feed
* grub 1.96 svn 20080813 and circular lvm2 metadata
@ 2008-08-28 11:14 Hans Lambermont
  2008-08-28 11:25 ` Felix Zielcke
  0 siblings, 1 reply; 17+ messages in thread
From: Hans Lambermont @ 2008-08-28 11:14 UTC (permalink / raw)
  To: grub-devel; +Cc: Jan Derk Gerlings

Hi all,

My colleague Jan Derk Gerlings found a bug in the way grub 1.96, svn
version 20080813 (and earlier) reads the circular lvm2 metadata buffer.

If the lvm2 metadata is too large to fit in the buffer given an offset,
then it is continued at the start of the buffer. In this case the
current grub-setup will exit with a segmentation fault and on boot grub
will not find the root drive.

The origin of this bug is traced to the way grub handles lvm2 metadata.
Lvm2 has defined (and uses) the on-disk metadata as a circular list.
Each change to the metadate (lvm lvchange, lvm create, lvm remove, etc.)
forces lvm to write the complete metadata to the circular buffer. It
will be placed after the last instance of the metadata with a new
sequence number.

When the end of the buffer is reached, lvm will continue (wrap-around
to) at the start of the buffer. Grub treats the buffer as linear, so
when wrap-around occurs grub fails because it does not continue at the
start of the buffer. This results in the grub-setup segmentation error,
and the inability to boot.

On request we can send two (literal) copies of the circular buffer (it's
too big for the mailing list 100kibi limit).
In md0_ok.txt the latest sequence number of the metadata is 280. It ends
before it reaches the end of the buffer. Grub-install works and the
machine boots.
In md0_err the latest sequence number of the metadata is 282. It wraps
around at the end of the buffer, continuing at the start of the buffer.
The end of the metadata comes before the start. Grub-install fails and
so does the boot.

At grub-1.96_svn20080813/disk/lvm.c line 303 and further down the notion
of circular metadata is not taken into account.

Shall we attempt to fix this and send a patch ?

regards,
   Hans Lambermont, Jan Derk Gerlings
-- 
Hans Lambermont, M.Sc.  -  Newtec  -  OS-Platform&VAS
http://newtec.eu/    t:+31408519234    m:+31629064887



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: grub 1.96 svn 20080813 and circular lvm2 metadata
  2008-08-28 11:14 grub 1.96 svn 20080813 and circular lvm2 metadata Hans Lambermont
@ 2008-08-28 11:25 ` Felix Zielcke
  2008-08-29  9:00   ` Hans Lambermont
  0 siblings, 1 reply; 17+ messages in thread
From: Felix Zielcke @ 2008-08-28 11:25 UTC (permalink / raw)
  To: The development of GRUB 2; +Cc: Jan Derk Gerlings

Am Donnerstag, den 28.08.2008, 13:14 +0200 schrieb Hans Lambermont:
> Hi all,

Hello,


> My colleague Jan Derk Gerlings found a bug in the way grub 1.96, svn
> version 20080813 (and earlier) reads the circular lvm2 metadata buffer.

Great, on Debian BTS we have 2 bugs [0] related to LVM and I wasn't able
yet to trace them down, but that could be the reason for them.


> Shall we attempt to fix this and send a patch ?

Yes please do :)

Because GRUB is a GNU project, you have to obey the GNU Coding Standards [1]

The Maintainer standards [2] says that on ~15 line changes you need to
assign copyright to FSF, but for this the others can tell you more after
you did your patch.

So just try to obey the coding style used in GRUB and please try to
write a changelog entry, see GCS as a manual and the changelog file for
examples 

[0] http://bugs.debian.org/462835
    http://bugs.debian.org/495949
[1] http://www.gnu.org/prep/standards/html_node/index.html
[2] http://www.gnu.org/prep/maintain/maintain.html#Legally-Significant

-- 
Felix Zielcke




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: grub 1.96 svn 20080813 and circular lvm2 metadata
  2008-08-28 11:25 ` Felix Zielcke
@ 2008-08-29  9:00   ` Hans Lambermont
  2008-08-29  9:31     ` Felix Zielcke
  2008-08-29 16:08     ` Marco Gerards
  0 siblings, 2 replies; 17+ messages in thread
From: Hans Lambermont @ 2008-08-29  9:00 UTC (permalink / raw)
  To: The development of GRUB 2; +Cc: Felix Zielcke, Jan Derk Gerlings

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

Felix Zielcke wrote on 20080828:

> Am Donnerstag, den 28.08.2008, 13:14 +0200 schrieb Hans Lambermont:
>> My colleague Jan Derk Gerlings found a bug in the way grub 1.96, svn
>> version 20080813 (and earlier) reads the circular lvm2 metadata
>> buffer.
> 
> Great, on Debian BTS we have 2 bugs [0] related to LVM and I wasn't
> able yet to trace them down, but that could be the reason for them.
> 
>> Shall we attempt to fix this and send a patch ?
> 
> Yes please do :)

Please find the patch attached, this was tested with circular metadata
and the segfault in grub-setup is gone and the system boots fine.

> Because GRUB is a GNU project, you have to obey the GNU Coding Standards [1]

Ok, I hope I got it right. If not please let me know.

regards,
   Hans Lambermont
-- 
Hans Lambermont, M.Sc.  -  Newtec  -  OS-Platform&VAS
http://newtec.eu/    t:+31408519234    m:+31629064887

[-- Attachment #2: circular-metadata.patch --]
[-- Type: text/x-diff, Size: 1412 bytes --]

diff -uwr grub-1.96_svn20080813-org/ChangeLog grub-1.96_svn20080813-new/ChangeLog
--- grub-1.96_svn20080813-org/ChangeLog	2008-08-13 17:24:36.000000000 +0200
+++ grub-1.96_svn20080813-new/ChangeLog	2008-08-29 10:33:03.000000000 +0200
@@ -1,3 +1,8 @@
+2008-08-28 Hans Lambermont <hans.lambermont@newtec.eu> (tiny change)
+	   Jan Derk Gerlings <jan.derk.gerlings@newtec.eu> (tiny change)
+
+	* disk/lvm.c: Add capability to read circular metadata
+
 2008-08-12  Robert Millan  <rmh@aybabtu.com>
 
 	* loader/i386/pc/multiboot.c (grub_multiboot_load_elf32): Move part
diff -uwr grub-1.96_svn20080813-org/disk/lvm.c grub-1.96_svn20080813-new/disk/lvm.c
--- grub-1.96_svn20080813-org/disk/lvm.c	2008-08-28 14:32:53.000000000 +0200
+++ grub-1.96_svn20080813-new/disk/lvm.c	2008-08-28 18:31:19.000000000 +0200
@@ -281,7 +281,8 @@
       goto fail;
     }
 
-  metadatabuf = grub_malloc (mda_size);
+  /* alloc for circular worst-case scenario */
+  metadatabuf = grub_malloc (2*mda_size);
   if (! metadatabuf)
     goto fail;
 
@@ -300,6 +301,12 @@
     }
 
   rlocn = mdah->raw_locns;
+  if (rlocn->offset + rlocn->size > mdah->size)
+    {
+      /* metadata is circular */
+      grub_memcpy(metadatabuf + mda_size, metadatabuf + mdah->start,
+		  ((rlocn->offset + rlocn->size) - mdah->size));
+    }
   p = q = metadatabuf + grub_le_to_cpu64 (rlocn->offset);
 
   while (*q != ' ' && q < metadatabuf + mda_size)

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: grub 1.96 svn 20080813 and circular lvm2 metadata
  2008-08-29  9:00   ` Hans Lambermont
@ 2008-08-29  9:31     ` Felix Zielcke
  2008-08-29 15:00       ` Vesa Jääskeläinen
  2008-08-29 16:08     ` Marco Gerards
  1 sibling, 1 reply; 17+ messages in thread
From: Felix Zielcke @ 2008-08-29  9:31 UTC (permalink / raw)
  To: The development of GRUB 2; +Cc: Jan Derk Gerlings

Am Freitag, den 29.08.2008, 11:00 +0200 schrieb Hans Lambermont:


> Please find the patch attached, this was tested with circular metadata
> and the segfault in grub-setup is gone and the system boots fine.

I just forwarded it now to the reporters of these 2 Debian bugs I
mentioned. I hope they try it out soon and reply if it helps for them.

Thanks again to your colleague and you.

> > Because GRUB is a GNU project, you have to obey the GNU Coding Standards [1]
> 
> Ok, I hope I got it right. If not please let me know.

Oh it seems I forgot you to tell you 2 things ;)
The Changelog entry should be seperate, i.e. not a diff against the
actual ChangeLog file but you can have it above inside your patch
For example here my last small patch send to the list

http://lists.gnu.org/archive/html/grub-devel/2008-08/txtUBorY42rw2.txt

Second please use `svn diff --diff-cmd diff -x -up' so the C function
name is printed inside the diff.

2008-08-28 Hans Lambermont <hans.lambermont@newtec.eu> (tiny change)

Yep, GCS mentions this `tiny change' but it's never used in GRUB's
ChangeLog.

+  metadatabuf = grub_malloc (2*mda_size);

should be (2 * mda_size)

+      grub_memcpy(metadatabuf + mda_size, metadatabuf + mdah->start,

again another space missing 

Except of this it looks fine for me, but I just started to contribute to
GRUB and in fact my C experience isn't that great either.
And I only started to use LVM for bugs in GRUB.

No need to send another patch just for these little changes, better just
wait a few days for comments from the others and if nobody replies then
feel free to bring it up again with a 2nd patch :)


-- 
Felix Zielcke




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: grub 1.96 svn 20080813 and circular lvm2 metadata
  2008-08-29  9:31     ` Felix Zielcke
@ 2008-08-29 15:00       ` Vesa Jääskeläinen
  2008-08-29 15:22         ` Felix Zielcke
  2008-08-29 15:37         ` Marco Gerards
  0 siblings, 2 replies; 17+ messages in thread
From: Vesa Jääskeläinen @ 2008-08-29 15:00 UTC (permalink / raw)
  To: The development of GRUB 2

Felix Zielcke wrote:
> The Changelog entry should be seperate, i.e. not a diff against the
> actual ChangeLog file but you can have it above inside your patch
> For example here my last small patch send to the list

That is perfectly fine there :)

Thou, you can also copy it within your email too. But everyone can spot
it from patch too. Most important aspect is that it has to be there...

Thanks,
Vesa Jääskeläinen



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: grub 1.96 svn 20080813 and circular lvm2 metadata
  2008-08-29 15:00       ` Vesa Jääskeläinen
@ 2008-08-29 15:22         ` Felix Zielcke
  2008-08-29 15:37         ` Marco Gerards
  1 sibling, 0 replies; 17+ messages in thread
From: Felix Zielcke @ 2008-08-29 15:22 UTC (permalink / raw)
  To: The development of GRUB 2

Hello,

Am Freitag, den 29.08.2008, 18:00 +0300 schrieb Vesa Jääskeläinen:
> Felix Zielcke wrote:
> > The Changelog entry should be seperate, i.e. not a diff against the
> > actual ChangeLog file but you can have it above inside your patch
> > For example here my last small patch send to the list
> 
> That is perfectly fine there :)
> 
> Thou, you can also copy it within your email too. But everyone can spot
> it from patch too. Most important aspect is that it has to be there...

Right ;)

I think that in general it's just better to not make an actual diff
against it, but I failed to tell him.
So it's anyway my fault again :)

Hm but now that I think about it, the `ChangeLog' file will probable be
mostly first on the diffs'
After a few days it just can't be applies cleanly but for the ChangeLog
entry you anyway need to copy and modify it first before commiting :)




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: grub 1.96 svn 20080813 and circular lvm2 metadata
  2008-08-29 15:00       ` Vesa Jääskeläinen
  2008-08-29 15:22         ` Felix Zielcke
@ 2008-08-29 15:37         ` Marco Gerards
  1 sibling, 0 replies; 17+ messages in thread
From: Marco Gerards @ 2008-08-29 15:37 UTC (permalink / raw)
  To: The development of GRUB 2

Vesa Jääskeläinen <chaac@nic.fi> writes:

> Felix Zielcke wrote:
>> The Changelog entry should be seperate, i.e. not a diff against the
>> actual ChangeLog file but you can have it above inside your patch
>> For example here my last small patch send to the list
>
> That is perfectly fine there :)
>
> Thou, you can also copy it within your email too. But everyone can spot
> it from patch too. Most important aspect is that it has to be there...

The problem is often when multiple files are patched, the ChangeLog
entry won't be the first in the list.  Furthermore this makes a patch
harder to apply.

--
Marco




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: grub 1.96 svn 20080813 and circular lvm2 metadata
  2008-08-29  9:00   ` Hans Lambermont
  2008-08-29  9:31     ` Felix Zielcke
@ 2008-08-29 16:08     ` Marco Gerards
  2008-09-03  6:49       ` Felix Zielcke
  2008-09-03 21:15       ` Felix Zielcke
  1 sibling, 2 replies; 17+ messages in thread
From: Marco Gerards @ 2008-08-29 16:08 UTC (permalink / raw)
  To: The development of GRUB 2; +Cc: Felix Zielcke, Jan Derk Gerlings

Hi Hans,

hans.lambermont@newtec.eu (Hans Lambermont) writes:

> Felix Zielcke wrote on 20080828:
>
>> Am Donnerstag, den 28.08.2008, 13:14 +0200 schrieb Hans Lambermont:
>>> My colleague Jan Derk Gerlings found a bug in the way grub 1.96, svn
>>> version 20080813 (and earlier) reads the circular lvm2 metadata
>>> buffer.
>> 
>> Great, on Debian BTS we have 2 bugs [0] related to LVM and I wasn't
>> able yet to trace them down, but that could be the reason for them.
>> 
>>> Shall we attempt to fix this and send a patch ?
>> 
>> Yes please do :)
>
> Please find the patch attached, this was tested with circular metadata
> and the segfault in grub-setup is gone and the system boots fine.

Great!  It is really nice to have this fixed :-)

>> Because GRUB is a GNU project, you have to obey the GNU Coding Standards [1]
>
> Ok, I hope I got it right. If not please let me know.

Okay.

> diff -uwr grub-1.96_svn20080813-org/ChangeLog grub-1.96_svn20080813-new/ChangeLog
> --- grub-1.96_svn20080813-org/ChangeLog	2008-08-13 17:24:36.000000000 +0200
> +++ grub-1.96_svn20080813-new/ChangeLog	2008-08-29 10:33:03.000000000 +0200
> @@ -1,3 +1,8 @@
> +2008-08-28 Hans Lambermont <hans.lambermont@newtec.eu> (tiny change)
> +	   Jan Derk Gerlings <jan.derk.gerlings@newtec.eu> (tiny change)
> +
> +	* disk/lvm.c: Add capability to read circular metadata

Please describe changes in the changelog entry, not the effect.

For example:

	* disk/lvm.c (grub_lvm_scan_device): Allocate buffer space for the
	worst case scenario.
	(grub_lvm_scan_device): ...

Where ... has to be filled in, I have no idea what this code actually
does or what you changed ;-)

The tiny change syntax does not seem familiar to me, AFAIK it is not
from the GCS.  Can you please change that?  Furthermore, if you both
worked on there two parts of the patch, please send in separate
patches.  It will make my life a lot easier... :-)

If only one person worked on this, for example Jan Derk, which Hans
only forwards this patch, please only list Jan Derk as the
contributor.

>  2008-08-12  Robert Millan  <rmh@aybabtu.com>
>  
>  	* loader/i386/pc/multiboot.c (grub_multiboot_load_elf32): Move part
> diff -uwr grub-1.96_svn20080813-org/disk/lvm.c grub-1.96_svn20080813-new/disk/lvm.c
> --- grub-1.96_svn20080813-org/disk/lvm.c	2008-08-28 14:32:53.000000000 +0200
> +++ grub-1.96_svn20080813-new/disk/lvm.c	2008-08-28 18:31:19.000000000 +0200
> @@ -281,7 +281,8 @@
>        goto fail;
>      }
>  
> -  metadatabuf = grub_malloc (mda_size);
> +  /* alloc for circular worst-case scenario */

Nitpick: Please start a sentence with a capital letter and end with a
`.'.  So this will become:

  /* Assume circular buffer in a worst case scenario.  */


> +  metadatabuf = grub_malloc (2*mda_size);

Please use spaces around operators:

2 * md_size

>    if (! metadatabuf)
>      goto fail;
>  
> @@ -300,6 +301,12 @@
>      }
>  
>    rlocn = mdah->raw_locns;
> +  if (rlocn->offset + rlocn->size > mdah->size)

Here rlcon->offset seems to be little endian (64 bits), so please use
grub_le_to_cpu64.  Same for rlocn->size, please check the size of this
member before you use a macro (I couldn't find it immediately...).

> +    {
> +      /* metadata is circular */

Same as above.

> +      grub_memcpy(metadatabuf + mda_size, metadatabuf + mdah->start,
> +		  ((rlocn->offset + rlocn->size) - mdah->size));
> +    }

Please check and correct the endianess.  you use a lot of
parenthesises.  Actually, I think none are requires.

Thanks,
Marco




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: grub 1.96 svn 20080813 and circular lvm2 metadata
  2008-08-29 16:08     ` Marco Gerards
@ 2008-09-03  6:49       ` Felix Zielcke
  2008-09-03 21:15       ` Felix Zielcke
  1 sibling, 0 replies; 17+ messages in thread
From: Felix Zielcke @ 2008-09-03  6:49 UTC (permalink / raw)
  To: The development of GRUB 2

Am Freitag, den 29.08.2008, 18:08 +0200 schrieb Marco Gerards:
> Hi Hans,
> 
> hans.lambermont@newtec.eu (Hans Lambermont) writes:
> > Please find the patch attached, this was tested with circular metadata
> > and the segfault in grub-setup is gone and the system boots fine.
> 
> Great!  It is really nice to have this fixed :-)

Just for your information. (I don't want to hurry.)
Reporter of Debian bug #462835 just confirmed that it fixes his problem.
I hope that the other one replys soon now too.


-- 
Felix Zielcke




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: grub 1.96 svn 20080813 and circular lvm2 metadata
  2008-08-29 16:08     ` Marco Gerards
  2008-09-03  6:49       ` Felix Zielcke
@ 2008-09-03 21:15       ` Felix Zielcke
  2008-09-08 10:28         ` Hans Lambermont
  2008-09-09 13:03         ` Hans Lambermont
  1 sibling, 2 replies; 17+ messages in thread
From: Felix Zielcke @ 2008-09-03 21:15 UTC (permalink / raw)
  To: The development of GRUB 2

Hans,

could you please address Marco's issues and send a new patch so the
topic is brought up again?

Am Freitag, den 29.08.2008, 18:08 +0200 schrieb Marco Gerards:
> 
> > diff -uwr grub-1.96_svn20080813-org/ChangeLog grub-1.96_svn20080813-new/ChangeLog
> > --- grub-1.96_svn20080813-org/ChangeLog	2008-08-13 17:24:36.000000000 +0200
> > +++ grub-1.96_svn20080813-new/ChangeLog	2008-08-29 10:33:03.000000000 +0200
> > @@ -1,3 +1,8 @@
> > +2008-08-28 Hans Lambermont <hans.lambermont@newtec.eu> (tiny change)
> > +	   Jan Derk Gerlings <jan.derk.gerlings@newtec.eu> (tiny change)
> > +
> > +	* disk/lvm.c: Add capability to read circular metadata
> 
> Please describe changes in the changelog entry, not the effect.
> 
> For example:
> 
> 	* disk/lvm.c (grub_lvm_scan_device): Allocate buffer space for the
> 	worst case scenario.
> 	(grub_lvm_scan_device): ...
> 
> Where ... has to be filled in, I have no idea what this code actually
> does or what you changed ;-)
> 
> The tiny change syntax does not seem familiar to me, AFAIK it is not
> from the GCS.  Can you please change that?  Furthermore, if you both
> worked on there two parts of the patch, please send in separate
> patches.  It will make my life a lot easier... :-)
> 
> If only one person worked on this, for example Jan Derk, which Hans
> only forwards this patch, please only list Jan Derk as the
> contributor.
> 
> >  2008-08-12  Robert Millan  <rmh@aybabtu.com>
> >  
> >  	* loader/i386/pc/multiboot.c (grub_multiboot_load_elf32): Move part
> > diff -uwr grub-1.96_svn20080813-org/disk/lvm.c grub-1.96_svn20080813-new/disk/lvm.c
> > --- grub-1.96_svn20080813-org/disk/lvm.c	2008-08-28 14:32:53.000000000 +0200
> > +++ grub-1.96_svn20080813-new/disk/lvm.c	2008-08-28 18:31:19.000000000 +0200
> > @@ -281,7 +281,8 @@
> >        goto fail;
> >      }
> >  
> > -  metadatabuf = grub_malloc (mda_size);
> > +  /* alloc for circular worst-case scenario */
> 
> Nitpick: Please start a sentence with a capital letter and end with a
> `.'.  So this will become:
> 
>   /* Assume circular buffer in a worst case scenario.  */
> 
> 
> > +  metadatabuf = grub_malloc (2*mda_size);
> 
> Please use spaces around operators:
> 
> 2 * md_size
> 
> >    if (! metadatabuf)
> >      goto fail;
> >  
> > @@ -300,6 +301,12 @@
> >      }
> >  
> >    rlocn = mdah->raw_locns;
> > +  if (rlocn->offset + rlocn->size > mdah->size)
> 
> Here rlcon->offset seems to be little endian (64 bits), so please use
> grub_le_to_cpu64.  Same for rlocn->size, please check the size of this
> member before you use a macro (I couldn't find it immediately...).
> 
> > +    {
> > +      /* metadata is circular */
> 
> Same as above.
> 
> > +      grub_memcpy(metadatabuf + mda_size, metadatabuf + mdah->start,
> > +		  ((rlocn->offset + rlocn->size) - mdah->size));
> > +    }
> 
> Please check and correct the endianess.  you use a lot of
> parenthesises.  Actually, I think none are requires.


-- 
Felix Zielcke




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: grub 1.96 svn 20080813 and circular lvm2 metadata
  2008-09-03 21:15       ` Felix Zielcke
@ 2008-09-08 10:28         ` Hans Lambermont
  2008-09-09 13:03         ` Hans Lambermont
  1 sibling, 0 replies; 17+ messages in thread
From: Hans Lambermont @ 2008-09-08 10:28 UTC (permalink / raw)
  To: The development of GRUB 2

Felix Zielcke wrote on 20080903:

> could you please address Marco's issues and send a new patch so the
> topic is brought up again?

Yes, I'll make some time for it.

regards,
   Hans Lambermont
-- 
Hans Lambermont, M.Sc.  -  Newtec  -  OS-Platform&VAS
http://newtec.eu/    t:+31408519234    m:+31629064887



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: grub 1.96 svn 20080813 and circular lvm2 metadata
  2008-09-03 21:15       ` Felix Zielcke
  2008-09-08 10:28         ` Hans Lambermont
@ 2008-09-09 13:03         ` Hans Lambermont
  2008-09-23  8:29           ` Hans Lambermont
  1 sibling, 1 reply; 17+ messages in thread
From: Hans Lambermont @ 2008-09-09 13:03 UTC (permalink / raw)
  To: The development of GRUB 2; +Cc: Jan Derk Gerlings

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

Felix Zielcke wrote on 20080903:

> could you please address Marco's issues and send a new patch so the
> topic is brought up again?

Please find attached a new patch. I hope I managed to fix all remarks
that were made ;-) If not then please let me know.

This is intended for the Changelog :

2008-09-09  Hans Lambermont  <hans.lambermont@newtec.eu>

       * disk/lvm.c (grub_lvm_scan_device): Allocate buffer space for the
       circular metadata worst case scenario. If the metadata is circular
       then copy the wrap in place.
       * include/grub/lvm.h: Add GRUB_LVM_MDA_HEADER_SIZE, from the LVM2
       project lib/format_text/layout.h
       Circular metadata bug found and patch debugged by
       Jan Derk Gerlings  <jan.derk.gerlings@newtec.eu>

The 'svn diff --diff-cmd diff -x -up' style patch is attached.

regards,
   Hans Lambermont
-- 
Hans Lambermont, M.Sc.  -  Newtec  -  OS-Platform&VAS
http://newtec.eu/    t:+31408519234    m:+31629064887

[-- Attachment #2: grublvm.patch --]
[-- Type: text/x-diff, Size: 1522 bytes --]

Index: disk/lvm.c
===================================================================
--- disk/lvm.c	(revision 1858)
+++ disk/lvm.c	(working copy)
@@ -281,7 +281,8 @@ grub_lvm_scan_device (const char *name)
       goto fail;
     }
 
-  metadatabuf = grub_malloc (mda_size);
+  /* Allocate buffer space for the circular worst-case scenario. */
+  metadatabuf = grub_malloc (2 * mda_size);
   if (! metadatabuf)
     goto fail;
 
@@ -300,6 +301,16 @@ grub_lvm_scan_device (const char *name)
     }
 
   rlocn = mdah->raw_locns;
+  if (grub_le_to_cpu64 (rlocn->offset) + grub_le_to_cpu64 (rlocn->size) >
+      grub_le_to_cpu64 (mdah->size))
+    {
+      /* Metadata is circular. Copy the wrap in place. */
+      grub_memcpy (metadatabuf + mda_size,
+                   metadatabuf + GRUB_LVM_MDA_HEADER_SIZE,
+                   grub_le_to_cpu64 (rlocn->offset) +
+                   grub_le_to_cpu64 (rlocn->size) -
+                   grub_le_to_cpu64 (mdah->size));
+    }
   p = q = metadatabuf + grub_le_to_cpu64 (rlocn->offset);
 
   while (*q != ' ' && q < metadatabuf + mda_size)
Index: include/grub/lvm.h
===================================================================
--- include/grub/lvm.h	(revision 1858)
+++ include/grub/lvm.h	(working copy)
@@ -103,6 +103,7 @@ struct grub_lvm_pv_header {
 
 #define GRUB_LVM_FMTT_MAGIC "\040\114\126\115\062\040\170\133\065\101\045\162\060\116\052\076"
 #define GRUB_LVM_FMTT_VERSION 1
+#define GRUB_LVM_MDA_HEADER_SIZE 512
 
 /* On disk */
 struct grub_lvm_raw_locn {

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: grub 1.96 svn 20080813 and circular lvm2 metadata
  2008-09-09 13:03         ` Hans Lambermont
@ 2008-09-23  8:29           ` Hans Lambermont
  2008-09-23  8:58             ` Felix Zielcke
  0 siblings, 1 reply; 17+ messages in thread
From: Hans Lambermont @ 2008-09-23  8:29 UTC (permalink / raw)
  To: The development of GRUB 2, Felix Zielcke; +Cc: Jan Derk Gerlings

Hans Lambermont wrote on 20080909:
> Felix Zielcke wrote on 20080903:
>> could you please address Marco's issues and send a new patch so the
>> topic is brought up again?

The new patch was sent to the list 2 weeks ago.
(See http://lists.gnu.org/archive/html/grub-devel/2008-09/msg00210.html )

Is there anything else that is needed ?

regards,
   Hans Lambermont
-- 
Hans Lambermont, M.Sc.  -  Newtec  -  OS-Platform&VAS
http://newtec.eu/    t:+31408519234    m:+31629064887



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: grub 1.96 svn 20080813 and circular lvm2 metadata
  2008-09-23  8:29           ` Hans Lambermont
@ 2008-09-23  8:58             ` Felix Zielcke
  2008-09-24  9:45               ` Robert Millan
  0 siblings, 1 reply; 17+ messages in thread
From: Felix Zielcke @ 2008-09-23  8:58 UTC (permalink / raw)
  To: The development of GRUB 2

Am Dienstag, den 23.09.2008, 10:29 +0200 schrieb Hans Lambermont:
> The new patch was sent to the list 2 weeks ago.
> (See http://lists.gnu.org/archive/html/grub-devel/2008-09/msg00210.html )
> 
> Is there anything else that is needed ?


For me personally it looks fine, but it would be good if Marco has a
look over it and unfornatunately he doestn't have much free time now.




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: grub 1.96 svn 20080813 and circular lvm2 metadata
  2008-09-23  8:58             ` Felix Zielcke
@ 2008-09-24  9:45               ` Robert Millan
  2008-09-30 12:34                 ` Hans Lambermont
  0 siblings, 1 reply; 17+ messages in thread
From: Robert Millan @ 2008-09-24  9:45 UTC (permalink / raw)
  To: The development of GRUB 2

On Tue, Sep 23, 2008 at 10:58:01AM +0200, Felix Zielcke wrote:
> Am Dienstag, den 23.09.2008, 10:29 +0200 schrieb Hans Lambermont:
> > The new patch was sent to the list 2 weeks ago.
> > (See http://lists.gnu.org/archive/html/grub-devel/2008-09/msg00210.html )
> > 
> > Is there anything else that is needed ?
> 
> 
> For me personally it looks fine, but it would be good if Marco has a
> look over it and unfornatunately he doestn't have much free time now.

So let's put him in CC.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: grub 1.96 svn 20080813 and circular lvm2 metadata
  2008-09-24  9:45               ` Robert Millan
@ 2008-09-30 12:34                 ` Hans Lambermont
  2008-10-05 10:50                   ` Robert Millan
  0 siblings, 1 reply; 17+ messages in thread
From: Hans Lambermont @ 2008-09-30 12:34 UTC (permalink / raw)
  To: The development of GRUB 2

Robert Millan wrote on 20080924:

> On Tue, Sep 23, 2008 at 10:58:01AM +0200, Felix Zielcke wrote:
>> Am Dienstag, den 23.09.2008, 10:29 +0200 schrieb Hans Lambermont:
>>> The new patch was sent to the list 2 weeks ago.
>>> (See http://lists.gnu.org/archive/html/grub-devel/2008-09/msg00210.html )
>>> Is there anything else that is needed ?
>> For me personally it looks fine, but it would be good if Marco has a
>> look over it and unfornatunately he doestn't have much free time now.
> So let's put him in CC.

My email address I put in the changelog proposal will stop working soon, so
here's a new one that'll last longer :

This is intended for the Changelog :

2008-09-09  Hans Lambermont  <hans@lambermont.dyndns.org>

       * disk/lvm.c (grub_lvm_scan_device): Allocate buffer space for the
       circular metadata worst case scenario. If the metadata is circular
       then copy the wrap in place.
       * include/grub/lvm.h: Add GRUB_LVM_MDA_HEADER_SIZE, from the LVM2
       project lib/format_text/layout.h
       Circular metadata bug found and patch debugged by
       Jan Derk Gerlings  <jan.derk.gerlings@newtec.eu>

The address of Jan Derk will stop working too, I'll send an new one as
soon as I have it.

regards,
   Hans Lambermont
-- 
Hans Lambermont, M.Sc.  -  Newtec  -  OS-Platform&VAS
http://newtec.eu/    t:+31408519234    m:+31629064887



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: grub 1.96 svn 20080813 and circular lvm2 metadata
  2008-09-30 12:34                 ` Hans Lambermont
@ 2008-10-05 10:50                   ` Robert Millan
  0 siblings, 0 replies; 17+ messages in thread
From: Robert Millan @ 2008-10-05 10:50 UTC (permalink / raw)
  To: The development of GRUB 2

On Tue, Sep 30, 2008 at 02:34:36PM +0200, Hans Lambermont wrote:
> Robert Millan wrote on 20080924:
> 
> > On Tue, Sep 23, 2008 at 10:58:01AM +0200, Felix Zielcke wrote:
> >> Am Dienstag, den 23.09.2008, 10:29 +0200 schrieb Hans Lambermont:
> >>> The new patch was sent to the list 2 weeks ago.
> >>> (See http://lists.gnu.org/archive/html/grub-devel/2008-09/msg00210.html )
> >>> Is there anything else that is needed ?
> >> For me personally it looks fine, but it would be good if Marco has a
> >> look over it and unfornatunately he doestn't have much free time now.
> > So let's put him in CC.
> 
> My email address I put in the changelog proposal will stop working soon, so
> here's a new one that'll last longer :
> 
> This is intended for the Changelog :
> 
> 2008-09-09  Hans Lambermont  <hans@lambermont.dyndns.org>
> 
>        * disk/lvm.c (grub_lvm_scan_device): Allocate buffer space for the
>        circular metadata worst case scenario. If the metadata is circular
>        then copy the wrap in place.
>        * include/grub/lvm.h: Add GRUB_LVM_MDA_HEADER_SIZE, from the LVM2
>        project lib/format_text/layout.h
>        Circular metadata bug found and patch debugged by
>        Jan Derk Gerlings  <jan.derk.gerlings@newtec.eu>
> 
> The address of Jan Derk will stop working too, I'll send an new one as
> soon as I have it.

Hi,

I checked this in, but ommitted Jan's address, since it will stop working as
you said.

Thanks!

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."



^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2008-10-05 10:52 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-08-28 11:14 grub 1.96 svn 20080813 and circular lvm2 metadata Hans Lambermont
2008-08-28 11:25 ` Felix Zielcke
2008-08-29  9:00   ` Hans Lambermont
2008-08-29  9:31     ` Felix Zielcke
2008-08-29 15:00       ` Vesa Jääskeläinen
2008-08-29 15:22         ` Felix Zielcke
2008-08-29 15:37         ` Marco Gerards
2008-08-29 16:08     ` Marco Gerards
2008-09-03  6:49       ` Felix Zielcke
2008-09-03 21:15       ` Felix Zielcke
2008-09-08 10:28         ` Hans Lambermont
2008-09-09 13:03         ` Hans Lambermont
2008-09-23  8:29           ` Hans Lambermont
2008-09-23  8:58             ` Felix Zielcke
2008-09-24  9:45               ` Robert Millan
2008-09-30 12:34                 ` Hans Lambermont
2008-10-05 10:50                   ` Robert Millan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.