[PATCH] lvm: Grub2 fails to detect LVM volumes due to an incorrect computation of mda_end

[PATCH] lvm: Grub2 fails to detect LVM volumes due to an incorrect computation of mda_end

[Glenn Washburn]

From: Rogier <rogier777@gmail.com>

When handling a regular LVM volume, Grub can fail with the message:
error: disk `lvmid/******-****-****-****-****-****-
****/******-****-****-****-****-****-******' not found.

If the condition which triggers this exists, grub-probe will report the
error mentioned above. Similarly, the grub boot code will fail to detect
LVM volumes, resulting in a failure to boot off of LVM disks/partitions.
The condition can be created on any LVM VG by an LVM configuration change,
so any system with /boot on LVM can become unbootable at 'any' time (after
any LVM configuration change).

The problem is caused by an incorrect computation of mda_end in lvm.c, when
the metadata area wraps around. Apparently, this can start happening at
around 220 metadata changes to the VG.

Fixes: 879c4a834 (lvm: Fix two more potential data-dependent alloc overflows)
Fixes: https://savannah.gnu.org/bugs/?61620

Signed-off-by: Rogier <rogier777@gmail.com>
Signed-off-by: Glenn Washburn <development@efficientek.com>
---
I have done no testing of this patch. I've only created a suitable patch
for review. This seems like a fairly serious issue that might one day bite
me, so I think it deserves a review.

Glenn
---
 grub-core/disk/lvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 794248540cd3..8535d5a5863a 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -290,7 +290,7 @@ grub_lvm_detect (grub_disk_t disk,
 
   p = q = (char *)ptr;
 
-  if (grub_add ((grub_size_t)metadatabuf, (grub_size_t)mda_size, &ptr))
+  if (grub_add (ptr, (grub_size_t)grub_le_to_cpu64 (rlocn->size), &ptr))
     goto error_parsing_metadata;
 
   mda_end = (char *)ptr;
-- 
2.34.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] lvm: Grub2 fails to detect LVM volumes due to an incorrect computation of mda_end

[Daniel Kiper via Grub-devel]

Adding Marta...

On Mon, May 06, 2024 at 03:18:45PM -0500, Glenn Washburn wrote:
> From: Rogier <rogier777@gmail.com>
>
> When handling a regular LVM volume, Grub can fail with the message:
> error: disk `lvmid/******-****-****-****-****-****-
> ****/******-****-****-****-****-****-******' not found.
>
> If the condition which triggers this exists, grub-probe will report the
> error mentioned above. Similarly, the grub boot code will fail to detect
> LVM volumes, resulting in a failure to boot off of LVM disks/partitions.
> The condition can be created on any LVM VG by an LVM configuration change,
> so any system with /boot on LVM can become unbootable at 'any' time (after
> any LVM configuration change).
>
> The problem is caused by an incorrect computation of mda_end in lvm.c, when
> the metadata area wraps around. Apparently, this can start happening at
> around 220 metadata changes to the VG.
>
> Fixes: 879c4a834 (lvm: Fix two more potential data-dependent alloc overflows)
> Fixes: https://savannah.gnu.org/bugs/?61620
>
> Signed-off-by: Rogier <rogier777@gmail.com>
> Signed-off-by: Glenn Washburn <development@efficientek.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Marta, may I ask you to test this patch?

> ---
> I have done no testing of this patch. I've only created a suitable patch
> for review. This seems like a fairly serious issue that might one day bite
> me, so I think it deserves a review.
>
> Glenn
> ---
>  grub-core/disk/lvm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
> index 794248540cd3..8535d5a5863a 100644
> --- a/grub-core/disk/lvm.c
> +++ b/grub-core/disk/lvm.c
> @@ -290,7 +290,7 @@ grub_lvm_detect (grub_disk_t disk,
>
>    p = q = (char *)ptr;
>
> -  if (grub_add ((grub_size_t)metadatabuf, (grub_size_t)mda_size, &ptr))
> +  if (grub_add (ptr, (grub_size_t)grub_le_to_cpu64 (rlocn->size), &ptr))
>      goto error_parsing_metadata;
>
>    mda_end = (char *)ptr;

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] lvm: Grub2 fails to detect LVM volumes due to an incorrect computation of mda_end

[Michael Chang via Grub-devel]

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

On Wed, May 08, 2024 at 05:48:15PM GMT, Daniel Kiper via Grub-devel wrote:
> Adding Marta...
> 
> On Mon, May 06, 2024 at 03:18:45PM -0500, Glenn Washburn wrote:
> > From: Rogier <rogier777@gmail.com>
> >
> > When handling a regular LVM volume, Grub can fail with the message:
> > error: disk `lvmid/******-****-****-****-****-****-
> > ****/******-****-****-****-****-****-******' not found.
> >
> > If the condition which triggers this exists, grub-probe will report the
> > error mentioned above. Similarly, the grub boot code will fail to detect
> > LVM volumes, resulting in a failure to boot off of LVM disks/partitions.
> > The condition can be created on any LVM VG by an LVM configuration change,
> > so any system with /boot on LVM can become unbootable at 'any' time (after
> > any LVM configuration change).
> >
> > The problem is caused by an incorrect computation of mda_end in lvm.c, when
> > the metadata area wraps around. Apparently, this can start happening at
> > around 220 metadata changes to the VG.

The number of times to commit a wrap actually depends on the size of the
metadata area, how many LVs and VGs are connected to the PV and
whichever would take part in the size of active raw metadata block
within the area.

I managed to use the attached shell script to prepare a LVM disk where
the metadata area is in wrapped state. It took about 100 cycles for me.
With that, I could see the error occurred, and with the patch, the
problem is gone.

So, Feel free to add my: 

Tested-By: Michael Chang <mchang@suse.com>

Thanks,
Michael

> >
> > Fixes: 879c4a834 (lvm: Fix two more potential data-dependent alloc overflows)
> > Fixes: https://savannah.gnu.org/bugs/?61620
> >
> > Signed-off-by: Rogier <rogier777@gmail.com>
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> 
> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
> 
> Marta, may I ask you to test this patch?
> 
> > ---
> > I have done no testing of this patch. I've only created a suitable patch
> > for review. This seems like a fairly serious issue that might one day bite
> > me, so I think it deserves a review.
> >
> > Glenn
> > ---
> >  grub-core/disk/lvm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
> > index 794248540cd3..8535d5a5863a 100644
> > --- a/grub-core/disk/lvm.c
> > +++ b/grub-core/disk/lvm.c
> > @@ -290,7 +290,7 @@ grub_lvm_detect (grub_disk_t disk,
> >
> >    p = q = (char *)ptr;
> >
> > -  if (grub_add ((grub_size_t)metadatabuf, (grub_size_t)mda_size, &ptr))
> > +  if (grub_add (ptr, (grub_size_t)grub_le_to_cpu64 (rlocn->size), &ptr))
> >      goto error_parsing_metadata;
> >
> >    mda_end = (char *)ptr;
> 
> Daniel
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

[-- Attachment #2: lvm-mda-wrap.sh --]
[-- Type: application/x-sh, Size: 1427 bytes --]

[-- Attachment #3: Type: text/plain, Size: 141 bytes --]

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] lvm: Grub2 fails to detect LVM volumes due to an incorrect computation of mda_end

[Michael Chang via Grub-devel]

On Thu, May 16, 2024 at 12:04:21PM GMT, Michael Chang wrote:
> On Wed, May 08, 2024 at 05:48:15PM GMT, Daniel Kiper via Grub-devel wrote:
> > Adding Marta...
> > 
> > On Mon, May 06, 2024 at 03:18:45PM -0500, Glenn Washburn wrote:
> > > From: Rogier <rogier777@gmail.com>
> > >
> > > When handling a regular LVM volume, Grub can fail with the message:
> > > error: disk `lvmid/******-****-****-****-****-****-
> > > ****/******-****-****-****-****-****-******' not found.
> > >
> > > If the condition which triggers this exists, grub-probe will report the
> > > error mentioned above. Similarly, the grub boot code will fail to detect
> > > LVM volumes, resulting in a failure to boot off of LVM disks/partitions.
> > > The condition can be created on any LVM VG by an LVM configuration change,
> > > so any system with /boot on LVM can become unbootable at 'any' time (after
> > > any LVM configuration change).
> > >
> > > The problem is caused by an incorrect computation of mda_end in lvm.c, when
> > > the metadata area wraps around. Apparently, this can start happening at
> > > around 220 metadata changes to the VG.
> 
> The number of times to commit a wrap actually depends on the size of the
> metadata area, how many LVs and VGs are connected to the PV and
> whichever would take part in the size of active raw metadata block
> within the area.
> 
> I managed to use the attached shell script to prepare a LVM disk where
> the metadata area is in wrapped state. It took about 100 cycles for me.

Hm. In each cycle, two changes (lvcreate/lvremove) are committed. Combined
with the initial 30 LVs commit, the total is very close to your 220
metadata changes.

Thanks,
Michael


> With that, I could see the error occurred, and with the patch, the
> problem is gone.
> 
> So, Feel free to add my: 
> 
> Tested-By: Michael Chang <mchang@suse.com>
> 
> Thanks,
> Michael
> 
> > >
> > > Fixes: 879c4a834 (lvm: Fix two more potential data-dependent alloc overflows)
> > > Fixes: https://savannah.gnu.org/bugs/?61620
> > >
> > > Signed-off-by: Rogier <rogier777@gmail.com>
> > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > 
> > Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
> > 
> > Marta, may I ask you to test this patch?
> > 
> > > ---
> > > I have done no testing of this patch. I've only created a suitable patch
> > > for review. This seems like a fairly serious issue that might one day bite
> > > me, so I think it deserves a review.
> > >
> > > Glenn
> > > ---
> > >  grub-core/disk/lvm.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
> > > index 794248540cd3..8535d5a5863a 100644
> > > --- a/grub-core/disk/lvm.c
> > > +++ b/grub-core/disk/lvm.c
> > > @@ -290,7 +290,7 @@ grub_lvm_detect (grub_disk_t disk,
> > >
> > >    p = q = (char *)ptr;
> > >
> > > -  if (grub_add ((grub_size_t)metadatabuf, (grub_size_t)mda_size, &ptr))
> > > +  if (grub_add (ptr, (grub_size_t)grub_le_to_cpu64 (rlocn->size), &ptr))
> > >      goto error_parsing_metadata;
> > >
> > >    mda_end = (char *)ptr;
> > 
> > Daniel
> > 
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel



_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel