From: Michael Chang via Grub-devel <grub-devel@gnu.org>
To: The development of GNU GRUB <grub-devel@gnu.org>
Cc: Michael Chang <mchang@suse.com>,
Glenn Washburn <development@efficientek.com>,
Daniel Kiper <daniel.kiper@oracle.com>,
Peter Jones <pjones@redhat.com>,
Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>,
Rogier <rogier777@gmail.com>,
Horst Prote <prote@fmi.uni-stuttgart.de>,
mlewando@redhat.com
Subject: Re: [PATCH] lvm: Grub2 fails to detect LVM volumes due to an incorrect computation of mda_end
Date: Thu, 16 May 2024 12:21:45 +0800 [thread overview]
Message-ID: <couq2abm66wuyghala5tbppesillfgawpuooemuts2ayydqysk@utrsf6xb2cny> (raw)
In-Reply-To: <mpzqt6kvvrdulqaboc544eewmsyo5pnbde5h2ynf5ik5uzjivp@5b6yiuyg7vyg>
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
prev parent reply other threads:[~2024-05-16 4:22 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-06 20:18 [PATCH] lvm: Grub2 fails to detect LVM volumes due to an incorrect computation of mda_end Glenn Washburn
2024-05-08 15:48 ` Daniel Kiper via Grub-devel
2024-05-16 4:04 ` Michael Chang via Grub-devel
2024-05-16 4:21 ` Michael Chang via Grub-devel [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=couq2abm66wuyghala5tbppesillfgawpuooemuts2ayydqysk@utrsf6xb2cny \
--to=grub-devel@gnu.org \
--cc=daniel.kiper@oracle.com \
--cc=development@efficientek.com \
--cc=mchang@suse.com \
--cc=mlewando@redhat.com \
--cc=phcoder@gmail.com \
--cc=pjones@redhat.com \
--cc=prote@fmi.uni-stuttgart.de \
--cc=rogier777@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).