Re: [PATCH 1/1] riscv: use time register in grub_efi_get_time_ms()
On Mon, Aug 12, 2024 at 04:13:18PM +0200, Heinrich Schuchardt wrote: > The cycle register is not guaranteed to count at constant frequency. > If it is counting at all depends on the state the performance monitoring > unit. > > Use the time register to measure time. > > Signed-off-by: Heinrich Schuchardt Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] tests: Let grub_cmd_cryptomount by default operate in /tmp rather than in /
On Wed, Jul 24, 2024 at 06:48:45PM +0200, Thomas Schmitt via Grub-devel wrote:
> If not TMPDIR is set by the user then the test grub_cmd_cryptomount
> creates about 20 directories named *LUKS*_test* in the root directory
> and leaves them there when the test ends.
>
> Initialize in the test script the variable TMPDIR to /tmp if it is not
> set or if it set to empty text. To be consistent with the usage of
> ${TMPDIR:-/tmp} in the script, use ${TMPDIR:=/tmp} not ${TMPDIR=/tmp}.
>
> Further delete each created directory as soon as the command of its
> test case is finished.
>
> Signed-off-by: Thomas Schmitt
> ---
> tests/grub_cmd_cryptomount.in | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/tests/grub_cmd_cryptomount.in b/tests/grub_cmd_cryptomount.in
> index f4d8f3547..696e61e96 100644
> --- a/tests/grub_cmd_cryptomount.in
> +++ b/tests/grub_cmd_cryptomount.in
> @@ -44,12 +44,23 @@ _testcase() {
> local output
> shift 2
>
> +# Use the environment variable TMPDIR, falling back to /tmp. This allows
> +# users to specify a different temporary directory, for example, if their
> +# /tmp is filled up or too small.
> +# Some other GRUB tests use this gesture with "=" rather than ":=".
> +# But in sync with the many occurences of ${TMPDIR:-/tmp}, this test uses
> +# ":=" to fill empty TMPDIR with "/tmp", regardless whether TMPDIR was
> +# set to empty or was not set at all.
> +: ${TMPDIR:=/tmp}
> +
> # Create a subdir in TMPDIR for each testcase
> _TMPDIR=$TMPDIR
> TMPDIR=$TMPDIR/`echo -n "$(date +%s).$LOGPREFIX" | sed -e 's,[ /],_,g'
> -e 's,:$,,g'`
> mkdir -p "$TMPDIR"
>
> output=`"$@" 2>&1` || res=$?
> +
> +rmdir "$TMPDIR"
s/rmdir/rm -rf/?
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Fix integer overflow in badram_iter
On Mon, Jul 29, 2024 at 09:07:48PM -0700, Arjun wrote:
> Fixes support for 64-bit badram entries. Previously, whenever the start
> address
> of an mmap region exceeded the maximum address attainable via an addr,mask
> pair,
> GRUB would erroneously attempt to binary-search up to the integer limit for an
> unsigned 64-bit integer in search of the "start" of its iterator over badram
> locations in the current range.
>
> On its own this wouldn't be an issue, as the iterator should be empty anyway
> in
> this case. However, the binary search code directly adds two 64-bit unsigned
> integers as an intermediate step, causing an integer overflow and a subsequent
> infinite loop.
>
> This patch fixes this behavior with two changes:
> 1. We set the initial upper bound of the binary search to the actual maximum
> possible index for the iterator to start at (i.e. (1 << var) - 1), rather
> than the 64-bit unsigned integer limit.
> 2. We avoid any integer overflows in the binary search by adding the high
> and
> low operands more carefully.
>
> The user-facing effect of this is that 64-bit badram entries no longer cause
> GRUB to hang indefinitely. This change has been tested and verified working on
> x86_64 EFI.
Missing Signed-off-by: ...
> ---
> grub-core/mmap/mmap.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/grub-core/mmap/mmap.c b/grub-core/mmap/mmap.c
> index c8c8312c5..ce02d8e66 100644
> --- a/grub-core/mmap/mmap.c
> +++ b/grub-core/mmap/mmap.c
> @@ -396,13 +396,14 @@ badram_iter (grub_uint64_t addr, grub_uint64_t size,
> else
> {
> low = 0;
> - high = ~0ULL;
> + high = var < 64 ? (1ULL << var) - 1ULL : ~0ULL;
Instead of ULL you should use grub_uint64_t cast.
> /* Find starting value. Keep low and high such that
> fill_mask (low) < addr and fill_mask (high) >= addr;
> */
> while (high - low > 1)
>{
> - cur = (low + high) / 2;
> + cur = low / 2 + high / 2 + (low & high & 1ULL);
Ditto...
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] efi/chainloader: Do not print device path of chainloaded file
On Fri, Aug 02, 2024 at 11:52:02AM +0100, Mate Kukri wrote:
> Users have no reason to see this without explictily asking for it, and
Users do not have any option to ask for it. Why do not use grub_dprintf("efi",
...)
instead of grub_printf()?
Daniel
> it can break graphical boot.
>
> Signed-off-by: Mate Kukri
> ---
> grub-core/loader/efi/chainloader.c | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/grub-core/loader/efi/chainloader.c
> b/grub-core/loader/efi/chainloader.c
> index eb833b678..11b64ac1b 100644
> --- a/grub-core/loader/efi/chainloader.c
> +++ b/grub-core/loader/efi/chainloader.c
> @@ -183,7 +183,6 @@ make_file_path (grub_efi_device_path_t *dp, const char
> *filename)
>/* Fill the file path for the directory. */
>d = (grub_efi_device_path_t *) ((char *) file_path
> + ((char *) d - (char *) dp));
> - grub_efi_print_device_path (d);
>if (copy_file_path ((grub_efi_file_path_device_path_t *) d,
> dir_start, dir_end - dir_start) != GRUB_ERR_NONE)
> {
> @@ -270,9 +269,6 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__
> ((unused)),
>file_path = make_file_path (dp, filename);
>if (file_path == NULL)
> goto fail;
> -
> - grub_printf ("file path: ");
> - grub_efi_print_device_path (file_path);
> }
>
>size = grub_file_size (file);
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2] bli: Fix crash in get_part_uuid
On Wed, Jul 17, 2024 at 02:46:46PM +0800, Michael Chang via Grub-devel wrote: > The get_part_uuid() function made an assumption that the target grub > device is a partition device and accessed device->disk->partition > without checking for NULL. There are four situations where this > assumption is problematic: > > 1. The device is a net device instead of a disk. > 2. The device is an abstraction device, like LVM, RAID, or CRYPTO, which >is mostly logical "disk" ((lvmid/) and so on). > 3. Firmware RAID may present the ESP to grub as an EFI disk (hd0) device >if it is contained within a Linux software RAID. > 4. When booting from a cdrom, the ESP is a vfat image indexed by the El >Torito boot catalog. The boot device is set to (cd0), corresponding >to the cdrom image mounted as an iso9660 filesystem. > > As a result, get_part_uuid() could lead to a NULL pointer dereference > and trigger a synchronous exception during boot if the ESP falls into > one of these categories. This patch fixes the problem by adding the > necessary checks to handle cases where the ESP is not a partition > device. > > Additionally, to avoid disrupting the boot process, this patch relaxes > the severity of the errors in this context to non-critical. Errors will > be logged, but they will not prevent the boot process from continuing. > > Fixes: e0fa7dc84 (bli: Add a module for the Boot Loader Interface) > Signed-off-by: Michael Chang > Reviewed-By: Oliver Steffen Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] tests: Let grub_cmd_cryptomount by default operate in /tmp rather than in /
Hi,
i wrote:
> > Further delete each created directory as soon as the command of its
> > test case is finished.
> > [...]
> > mkdir -p "$TMPDIR"
> >
> > output=`"$@" 2>&1` || res=$?
> > +
> > +rmdir "$TMPDIR"
Daniel Kiper wrote:
> s/rmdir/rm -rf/?
This is equivalent to the question whether remaining content shall be
removed silently. In my case the directories were all empty.
The worker in TMPDIR is @builddir@/grub-shell-luks-tester which stems
from tests/util/grub-shell-luks-tester.in . It shows own effort to leave
a clean TMPDIR:
cleanup() {
...
if [ -z "$debug" ] && [ "${RET:-1}" -eq 0 ]; then
rm -rf "$lukstestdir" || :
fi
}
trap cleanup EXIT INT TERM KILL QUIT
...
lukstestdir="`mktemp -d "${TMPDIR:-/tmp}/$(basename "$0").XX"`" ||
exit 20
Seeing the condition before "rm -rf", i guess tests/grub_cmd_cryptomount
should rather not remove the directory if it is not empty. A smarter way
than letting rmdir loudly fail seems appropriate, though.
If you agree to my assessment i will try to propose one in v2.
---
Self criticism:
I have recognized meanwhile that the proposed gesture
> > +: ${TMPDIR:=/tmp}
stems from the gnulib subdirectory and is not tradition in the rest
of the GRUB git repo.
GRUB test tradition seems to be temporary defaulting of TMPDIR like in
line 174 of tests/grub_cmd_cryptomount.in :
csscript=`mktemp "${TMPDIR:-/tmp}/tmp.XX"` || exit 99
Indeed the patch would be less needy of a lengthy comment if i propose
something like this yet untested change instead:
-TMPDIR=$TMPDIR/`echo -n "$(date +%s).$LOGPREFIX" | sed -e 's,[ /],_,g' -e
's,:$,,g'`
+TMPDIR="${TMPDIR:-/tmp}"/`echo -n "$(date +%s).$LOGPREFIX" | sed -e 's,[
/],_,g' -e 's,:$,,g'`
If you agree to my assessment, i will propose this in patch v2.
(Tested, of course.)
Have a nice day :)
Thomas
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] tests: Let grub_cmd_cryptomount by default operate in /tmp rather than in /
Hi, thinking more i believe that the currently used mkdir option -p is inappropriate in tests/grub_cmd_cryptomount.in . It hampers proper cleanup because the script cannot know how many directories in the path to TMPDIR were created and need to be removed. It is unusual because about all other tests in GRUB assume that non-empty TMPDIR or its default "/tmp" lead to an existing directory. (At least my local mktemp(1) refuses on non-existing components in the path.) If i read no objections, i will propose removal of -p in patch v2. Have a nice day :) Thomas ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 0/3] LVM Cachevol and Integrity volumes break entire LVM VG
In an effort to solve
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1061168 for myself,
I implemented basic support for cachevol and integrity volumes in LVM.
This is just an extension of the cachepool support that already
existed, and just like that support, I ignore all of the metadata and
just go for the data. This obviously means that writing to "writeback"
cachepools, and any raidintegrity volumes will cause corruption as
before, but as grub only reads files, I think that should be fine.
Without these patches, a system with /boot on a LV with cachevol or
integrity volumes will fail to boot
These patches are also available attached and at
https://github.com/byteit101/grub2/commits/grub-lvmintegrity/
Patrick Plenefisch (3):
disk/lvm: Make cache_lv more generic as ignored_feature_lv
disk/lvm: Remove unused cache_pool
lvm: Add support for cachevol and integrity lv
grub-core/disk/diskfilter.c | 6 +-
grub-core/disk/lvm.c| 267 ++--
2 files changed, 136 insertions(+), 137 deletions(-)
--
2.39.2
From 4a34bdbac9211d6c01e190b80664e92e183ef697 Mon Sep 17 00:00:00 2001
From: Patrick Plenefisch
Date: Tue, 13 Aug 2024 20:14:18 -0400
Subject: [PATCH 1/3] disk/lvm: Make cache_lv more generic as
ignored_feature_lv
This patch isn't necessary by itself, but when combined with the next patch it enhances readability as ignored_features_lv is then used for multiple types of extra LV's, not just cache LV's
Signed-off-by: Patrick Plenefisch
---
grub-core/disk/lvm.c | 261 +++
1 file changed, 141 insertions(+), 120 deletions(-)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 794248540..3d226fad5 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -34,12 +34,12 @@
GRUB_MOD_LICENSE ("GPLv3+");
-struct cache_lv
+struct ignored_feature_lv
{
struct grub_diskfilter_lv *lv;
char *cache_pool;
char *origin;
- struct cache_lv *next;
+ struct ignored_feature_lv *next;
};
@@ -105,30 +105,30 @@ grub_lvm_check_flag (const char *p, const char *str, const char *flag)
}
static void
-grub_lvm_free_cache_lvs (struct cache_lv *cache_lvs)
+grub_lvm_free_ignored_feature_lvs (struct ignored_feature_lv *ignored_feature_lvs)
{
- struct cache_lv *cache;
+ struct ignored_feature_lv *ignored_feature;
- while ((cache = cache_lvs))
+ while ((ignored_feature = ignored_feature_lvs))
{
- cache_lvs = cache_lvs->next;
+ ignored_feature_lvs = ignored_feature_lvs->next;
- if (cache->lv)
+ if (ignored_feature->lv)
{
unsigned int i;
- for (i = 0; i < cache->lv->segment_count; ++i)
- if (cache->lv->segments)
- grub_free (cache->lv->segments[i].nodes);
- grub_free (cache->lv->segments);
- grub_free (cache->lv->fullname);
- grub_free (cache->lv->idname);
- grub_free (cache->lv->name);
+ for (i = 0; i < ignored_feature->lv->segment_count; ++i)
+ if (ignored_feature->lv->segments)
+ grub_free (ignored_feature->lv->segments[i].nodes);
+ grub_free (ignored_feature->lv->segments);
+ grub_free (ignored_feature->lv->fullname);
+ grub_free (ignored_feature->lv->idname);
+ grub_free (ignored_feature->lv->name);
}
- grub_free (cache->lv);
- grub_free (cache->origin);
- grub_free (cache->cache_pool);
- grub_free (cache);
+ grub_free (ignored_feature->lv);
+ grub_free (ignored_feature->origin);
+ grub_free (ignored_feature->cache_pool);
+ grub_free (ignored_feature);
}
}
@@ -325,7 +325,7 @@ grub_lvm_detect (grub_disk_t disk,
if (! vg)
{
- struct cache_lv *cache_lvs = NULL;
+ struct ignored_feature_lv *ignored_feature_lvs = NULL;
/* First time we see this volume group. We've to create the
whole volume group structure. */
@@ -807,108 +807,122 @@ grub_lvm_detect (grub_disk_t disk,
seg->nodes[seg->node_count - 1].name = tmp;
}
}
+ /* Cache and integrity LVs have extra parts that
+ * we can ignore for our read-only access */
else if (grub_memcmp (p, "cache\"",
- sizeof ("cache\"") - 1) == 0)
+ sizeof ("cache\"") - 1) == 0
+ || grub_memcmp (p, "cache+CACHE_USES_CACHEVOL\"",
+ sizeof ("cache+CACHE_USES_CACHEVOL\"") - 1) == 0
+ || grub_memcmp (p, "integrity\"",
+ sizeof ("integrity\"") - 1) == 0)
{
- struct cache_lv *cache = NULL;
+ struct ignored_feature_lv *ignored_feature = NULL;
char *p2, *p3;
grub_size_t sz;
+#ifdef GRUB_UTIL
+ p2 = grub_strchr (p, '"');
+ if (p2)
+ *p2 = '\0';
+ grub_util_info ("Ignoring extra metadata type '%s' for %s", p, lv->name);
+ if (p2)
+ *p2 ='"';
+#endif
- cache = grub_zalloc (sizeof (*cache));
- if (!cache)
- goto cache_lv_fail;
- cache->lv = grub_zalloc (sizeof (*cache->lv));
- if (!cache->lv)
- goto cache_lv_fail;
- grub_memcpy (cache->lv, lv, sizeof (*cac
[PATCH 1/3] disk/lvm: Make cache_lv more generic as ignored_feature_lv
This patch isn't necessary by itself, but when combined with the next
patch it enhances readability as ignored_features_lv is then used for
multiple types of extra LV's, not just cache LV's
Signed-off-by: Patrick Plenefisch
---
grub-core/disk/lvm.c | 261 +++
1 file changed, 141 insertions(+), 120 deletions(-)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 794248540..3d226fad5 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -34,12 +34,12 @@
GRUB_MOD_LICENSE ("GPLv3+");
-struct cache_lv
+struct ignored_feature_lv
{
struct grub_diskfilter_lv *lv;
char *cache_pool;
char *origin;
- struct cache_lv *next;
+ struct ignored_feature_lv *next;
};
@@ -105,30 +105,30 @@ grub_lvm_check_flag (const char *p, const char
*str, const char *flag)
}
static void
-grub_lvm_free_cache_lvs (struct cache_lv *cache_lvs)
+grub_lvm_free_ignored_feature_lvs (struct ignored_feature_lv
*ignored_feature_lvs)
{
- struct cache_lv *cache;
+ struct ignored_feature_lv *ignored_feature;
- while ((cache = cache_lvs))
+ while ((ignored_feature = ignored_feature_lvs))
{
- cache_lvs = cache_lvs->next;
+ ignored_feature_lvs = ignored_feature_lvs->next;
- if (cache->lv)
+ if (ignored_feature->lv)
{
unsigned int i;
- for (i = 0; i < cache->lv->segment_count; ++i)
-if (cache->lv->segments)
- grub_free (cache->lv->segments[i].nodes);
- grub_free (cache->lv->segments);
- grub_free (cache->lv->fullname);
- grub_free (cache->lv->idname);
- grub_free (cache->lv->name);
+ for (i = 0; i < ignored_feature->lv->segment_count; ++i)
+if (ignored_feature->lv->segments)
+ grub_free (ignored_feature->lv->segments[i].nodes);
+ grub_free (ignored_feature->lv->segments);
+ grub_free (ignored_feature->lv->fullname);
+ grub_free (ignored_feature->lv->idname);
+ grub_free (ignored_feature->lv->name);
}
- grub_free (cache->lv);
- grub_free (cache->origin);
- grub_free (cache->cache_pool);
- grub_free (cache);
+ grub_free (ignored_feature->lv);
+ grub_free (ignored_feature->origin);
+ grub_free (ignored_feature->cache_pool);
+ grub_free (ignored_feature);
}
}
@@ -325,7 +325,7 @@ grub_lvm_detect (grub_disk_t disk,
if (! vg)
{
- struct cache_lv *cache_lvs = NULL;
+ struct ignored_feature_lv *ignored_feature_lvs = NULL;
/* First time we see this volume group. We've to create the
whole volume group structure. */
@@ -807,108 +807,122 @@ grub_lvm_detect (grub_disk_t disk,
seg->nodes[seg->node_count - 1].name = tmp;
}
}
+ /* Cache and integrity LVs have extra parts that
+ * we can ignore for our read-only access */
else if (grub_memcmp (p, "cache\"",
- sizeof ("cache\"") - 1) == 0)
+ sizeof ("cache\"") - 1) == 0
+ || grub_memcmp (p, "cache+CACHE_USES_CACHEVOL\"",
+ sizeof ("cache+CACHE_USES_CACHEVOL\"") - 1) == 0
+ || grub_memcmp (p, "integrity\"",
+ sizeof ("integrity\"") - 1) == 0)
{
- struct cache_lv *cache = NULL;
+ struct ignored_feature_lv *ignored_feature = NULL;
char *p2, *p3;
grub_size_t sz;
+#ifdef GRUB_UTIL
+ p2 = grub_strchr (p, '"');
+ if (p2)
+*p2 = '\0';
+ grub_util_info ("Ignoring extra metadata type '%s' for
%s", p, lv->name);
+ if (p2)
+*p2 ='"';
+#endif
- cache = grub_zalloc (sizeof (*cache));
- if (!cache)
-goto cache_lv_fail;
- cache->lv = grub_zalloc (sizeof (*cache->lv));
- if (!cache->lv)
-goto cache_lv_fail;
- grub_memcpy (cache->lv, lv, sizeof (*cache->lv));
+ ignored_feature = grub_zalloc (sizeof (*ignored_feature));
+ if (!ignored_feature)
+goto ignored_feature_lv_fail;
+ ignored_feature->lv = grub_zalloc (sizeof
(*ignored_feature->lv));
+ if (!ignored_feature->lv)
+goto ignored_feature_lv_fail;
+ grub_memcpy (ignored_feature->lv, lv, sizeof
(*ignored_feature->lv));
if (lv->fullname)
{
- cache->lv->fullname = grub_strdup (lv->fullname);
- if (!cache->lv->fullname)
-goto cache_lv_fail;
+ ignored_feature->lv->fullname = grub_strdup (lv->fullname);
+ if (!ignored_feature->lv->fullname)
+goto ignored_feature_lv_fail;
}
if (lv->idname)
{
- cache->lv->idname = grub_strdup (lv->idname);
- if (!cache->lv->idname)
-goto cache_lv_fail;
+ ignored_feature->lv
[PATCH 2/3] disk/lvm: Remove unused cache_pool
cache_pool is never read or used, remove it
Signed-off-by: Patrick Plenefisch
---
grub-core/disk/lvm.c | 25 -
1 file changed, 25 deletions(-)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 3d226fad5..17e225596 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -37,7 +37,6 @@ GRUB_MOD_LICENSE ("GPLv3+");
struct ignored_feature_lv
{
struct grub_diskfilter_lv *lv;
- char *cache_pool;
char *origin;
struct ignored_feature_lv *next;
};
@@ -127,7 +126,6 @@ grub_lvm_free_ignored_feature_lvs (struct
ignored_feature_lv *ignored_feature_lv
}
grub_free (ignored_feature->lv);
grub_free (ignored_feature->origin);
- grub_free (ignored_feature->cache_pool);
grub_free (ignored_feature);
}
}
@@ -858,28 +856,6 @@ grub_lvm_detect (grub_disk_t disk,
skip_lv = 1;
- p2 = grub_strstr (p, "cache_pool = \"");
- if (!p2)
-goto ignored_feature_lv_fail;
-
- p2 = grub_strchr (p2, '"');
- if (!p2)
-goto ignored_feature_lv_fail;
-
- p3 = ++p2;
- if (p3 == mda_end)
-goto ignored_feature_lv_fail;
- p3 = grub_strchr (p3, '"');
- if (!p3)
-goto ignored_feature_lv_fail;
-
- sz = p3 - p2;
-
- ignored_feature->cache_pool = grub_malloc (sz + 1);
- if (!ignored_feature->cache_pool)
-goto ignored_feature_lv_fail;
- grub_memcpy (ignored_feature->cache_pool, p2, sz);
- ignored_feature->cache_pool[sz] = '\0';
p2 = grub_strstr (p, "origin = \"");
if (!p2)
@@ -912,7 +888,6 @@ grub_lvm_detect (grub_disk_t disk,
if (ignored_feature)
{
grub_free (ignored_feature->origin);
- grub_free (ignored_feature->cache_pool);
if (ignored_feature->lv)
{
grub_free (ignored_feature->lv->fullname);
--
2.39.2
From 17514a1462d6c64e16e6d67133b0759c09746cbb Mon Sep 17 00:00:00 2001
From: Patrick Plenefisch
Date: Tue, 13 Aug 2024 20:15:37 -0400
Subject: [PATCH 2/3] disk/lvm: Remove unused cache_pool
cache_pool is never read or used, remove it
Signed-off-by: Patrick Plenefisch
---
grub-core/disk/lvm.c | 25 -
1 file changed, 25 deletions(-)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 3d226fad5..17e225596 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -37,7 +37,6 @@ GRUB_MOD_LICENSE ("GPLv3+");
struct ignored_feature_lv
{
struct grub_diskfilter_lv *lv;
- char *cache_pool;
char *origin;
struct ignored_feature_lv *next;
};
@@ -127,7 +126,6 @@ grub_lvm_free_ignored_feature_lvs (struct ignored_feature_lv *ignored_feature_lv
}
grub_free (ignored_feature->lv);
grub_free (ignored_feature->origin);
- grub_free (ignored_feature->cache_pool);
grub_free (ignored_feature);
}
}
@@ -858,28 +856,6 @@ grub_lvm_detect (grub_disk_t disk,
skip_lv = 1;
- p2 = grub_strstr (p, "cache_pool = \"");
- if (!p2)
- goto ignored_feature_lv_fail;
-
- p2 = grub_strchr (p2, '"');
- if (!p2)
- goto ignored_feature_lv_fail;
-
- p3 = ++p2;
- if (p3 == mda_end)
- goto ignored_feature_lv_fail;
- p3 = grub_strchr (p3, '"');
- if (!p3)
- goto ignored_feature_lv_fail;
-
- sz = p3 - p2;
-
- ignored_feature->cache_pool = grub_malloc (sz + 1);
- if (!ignored_feature->cache_pool)
- goto ignored_feature_lv_fail;
- grub_memcpy (ignored_feature->cache_pool, p2, sz);
- ignored_feature->cache_pool[sz] = '\0';
p2 = grub_strstr (p, "origin = \"");
if (!p2)
@@ -912,7 +888,6 @@ grub_lvm_detect (grub_disk_t disk,
if (ignored_feature)
{
grub_free (ignored_feature->origin);
- grub_free (ignored_feature->cache_pool);
if (ignored_feature->lv)
{
grub_free (ignored_feature->lv->fullname);
--
2.39.2
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 1/3 v8] disk/lvm: Make cache_lv more generic as ignored_feature_lv
This patch isn't necessary by itself, but when combined with the next
two patchs it enhances readability as ignored_features_lv is then used
for multiple types of extra LV's, not just cache LV's
Signed-off-by: Patrick Plenefisch
---
grub-core/disk/lvm.c | 176 +--
1 file changed, 88 insertions(+), 88 deletions(-)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 794248540..442a9fc96 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -34,12 +34,12 @@
GRUB_MOD_LICENSE ("GPLv3+");
-struct cache_lv
+struct ignored_feature_lv
{
struct grub_diskfilter_lv *lv;
char *cache_pool;
char *origin;
- struct cache_lv *next;
+ struct ignored_feature_lv *next;
};
@@ -105,30 +105,30 @@ grub_lvm_check_flag (const char *p, const char
*str, const char *flag)
}
static void
-grub_lvm_free_cache_lvs (struct cache_lv *cache_lvs)
+grub_lvm_free_ignored_feature_lvs (struct ignored_feature_lv
*ignored_feature_lvs)
{
- struct cache_lv *cache;
+ struct ignored_feature_lv *ignored_feature;
- while ((cache = cache_lvs))
+ while ((ignored_feature = ignored_feature_lvs))
{
- cache_lvs = cache_lvs->next;
+ ignored_feature_lvs = ignored_feature_lvs->next;
- if (cache->lv)
+ if (ignored_feature->lv)
{
unsigned int i;
- for (i = 0; i < cache->lv->segment_count; ++i)
-if (cache->lv->segments)
- grub_free (cache->lv->segments[i].nodes);
- grub_free (cache->lv->segments);
- grub_free (cache->lv->fullname);
- grub_free (cache->lv->idname);
- grub_free (cache->lv->name);
+ for (i = 0; i < ignored_feature->lv->segment_count; ++i)
+if (ignored_feature->lv->segments)
+ grub_free (ignored_feature->lv->segments[i].nodes);
+ grub_free (ignored_feature->lv->segments);
+ grub_free (ignored_feature->lv->fullname);
+ grub_free (ignored_feature->lv->idname);
+ grub_free (ignored_feature->lv->name);
}
- grub_free (cache->lv);
- grub_free (cache->origin);
- grub_free (cache->cache_pool);
- grub_free (cache);
+ grub_free (ignored_feature->lv);
+ grub_free (ignored_feature->origin);
+ grub_free (ignored_feature->cache_pool);
+ grub_free (ignored_feature);
}
}
@@ -325,7 +325,7 @@ grub_lvm_detect (grub_disk_t disk,
if (! vg)
{
- struct cache_lv *cache_lvs = NULL;
+ struct ignored_feature_lv *ignored_feature_lvs = NULL;
/* First time we see this volume group. We've to create the
whole volume group structure. */
@@ -810,105 +810,105 @@ grub_lvm_detect (grub_disk_t disk,
else if (grub_memcmp (p, "cache\"",
sizeof ("cache\"") - 1) == 0)
{
- struct cache_lv *cache = NULL;
+ struct ignored_feature_lv *ignored_feature = NULL;
char *p2, *p3;
grub_size_t sz;
- cache = grub_zalloc (sizeof (*cache));
- if (!cache)
-goto cache_lv_fail;
- cache->lv = grub_zalloc (sizeof (*cache->lv));
- if (!cache->lv)
-goto cache_lv_fail;
- grub_memcpy (cache->lv, lv, sizeof (*cache->lv));
+ ignored_feature = grub_zalloc (sizeof (*ignored_feature));
+ if (!ignored_feature)
+goto ignored_feature_lv_fail;
+ ignored_feature->lv = grub_zalloc (sizeof
(*ignored_feature->lv));
+ if (!ignored_feature->lv)
+goto ignored_feature_lv_fail;
+ grub_memcpy (ignored_feature->lv, lv, sizeof
(*ignored_feature->lv));
if (lv->fullname)
{
- cache->lv->fullname = grub_strdup (lv->fullname);
- if (!cache->lv->fullname)
-goto cache_lv_fail;
+ ignored_feature->lv->fullname = grub_strdup (lv->fullname);
+ if (!ignored_feature->lv->fullname)
+goto ignored_feature_lv_fail;
}
if (lv->idname)
{
- cache->lv->idname = grub_strdup (lv->idname);
- if (!cache->lv->idname)
-goto cache_lv_fail;
+ ignored_feature->lv->idname = grub_strdup (lv->idname);
+ if (!ignored_feature->lv->idname)
+goto ignored_feature_lv_fail;
}
if (lv->name)
{
- cache->lv->name = grub_strdup (lv->name);
- if (!cache->lv->name)
-goto cache_lv_fail;
+ ignored_feature->lv->name = grub_strdup (lv->name);
+ if (!ignored_feature->lv->name)
+goto ignored_feature_lv_fail;
}
skip_lv = 1;
p2 = grub_strstr (p, "cache_pool = \"");
if (!p2)
-goto cache_lv_fail;
+goto ignored_feature_lv_fail;
p2 = grub_strchr (p2, '"');
if (!
[PATCH 2/3 v8] disk/lvm: Remove unused cache_poo
cache_pool is never read or used, remove it
Signed-off-by: Patrick Plenefisch
---
grub-core/disk/lvm.c | 25 -
1 file changed, 25 deletions(-)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 442a9fc96..10bc965a4 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -37,7 +37,6 @@ GRUB_MOD_LICENSE ("GPLv3+");
struct ignored_feature_lv
{
struct grub_diskfilter_lv *lv;
- char *cache_pool;
char *origin;
struct ignored_feature_lv *next;
};
@@ -127,7 +126,6 @@ grub_lvm_free_ignored_feature_lvs (struct
ignored_feature_lv *ignored_feature_lv
}
grub_free (ignored_feature->lv);
grub_free (ignored_feature->origin);
- grub_free (ignored_feature->cache_pool);
grub_free (ignored_feature);
}
}
@@ -844,28 +842,6 @@ grub_lvm_detect (grub_disk_t disk,
skip_lv = 1;
- p2 = grub_strstr (p, "cache_pool = \"");
- if (!p2)
-goto ignored_feature_lv_fail;
-
- p2 = grub_strchr (p2, '"');
- if (!p2)
-goto ignored_feature_lv_fail;
-
- p3 = ++p2;
- if (p3 == mda_end)
-goto ignored_feature_lv_fail;
- p3 = grub_strchr (p3, '"');
- if (!p3)
-goto ignored_feature_lv_fail;
-
- sz = p3 - p2;
-
- ignored_feature->cache_pool = grub_malloc (sz + 1);
- if (!ignored_feature->cache_pool)
-goto ignored_feature_lv_fail;
- grub_memcpy (ignored_feature->cache_pool, p2, sz);
- ignored_feature->cache_pool[sz] = '\0';
p2 = grub_strstr (p, "origin = \"");
if (!p2)
@@ -898,7 +874,6 @@ grub_lvm_detect (grub_disk_t disk,
if (ignored_feature)
{
grub_free (ignored_feature->origin);
- grub_free (ignored_feature->cache_pool);
if (ignored_feature->lv)
{
grub_free (ignored_feature->lv->fullname);
--
2.39.2
From 0ae85fdab57eeea58d275962f9608d9a9fe6da57 Mon Sep 17 00:00:00 2001
From: Patrick Plenefisch
Date: Tue, 13 Aug 2024 20:15:37 -0400
Subject: [PATCH 2/3] disk/lvm: Remove unused cache_pool
cache_pool is never read or used, remove it
Signed-off-by: Patrick Plenefisch
---
grub-core/disk/lvm.c | 25 -
1 file changed, 25 deletions(-)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 442a9fc96..10bc965a4 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -37,7 +37,6 @@ GRUB_MOD_LICENSE ("GPLv3+");
struct ignored_feature_lv
{
struct grub_diskfilter_lv *lv;
- char *cache_pool;
char *origin;
struct ignored_feature_lv *next;
};
@@ -127,7 +126,6 @@ grub_lvm_free_ignored_feature_lvs (struct ignored_feature_lv *ignored_feature_lv
}
grub_free (ignored_feature->lv);
grub_free (ignored_feature->origin);
- grub_free (ignored_feature->cache_pool);
grub_free (ignored_feature);
}
}
@@ -844,28 +842,6 @@ grub_lvm_detect (grub_disk_t disk,
skip_lv = 1;
- p2 = grub_strstr (p, "cache_pool = \"");
- if (!p2)
- goto ignored_feature_lv_fail;
-
- p2 = grub_strchr (p2, '"');
- if (!p2)
- goto ignored_feature_lv_fail;
-
- p3 = ++p2;
- if (p3 == mda_end)
- goto ignored_feature_lv_fail;
- p3 = grub_strchr (p3, '"');
- if (!p3)
- goto ignored_feature_lv_fail;
-
- sz = p3 - p2;
-
- ignored_feature->cache_pool = grub_malloc (sz + 1);
- if (!ignored_feature->cache_pool)
- goto ignored_feature_lv_fail;
- grub_memcpy (ignored_feature->cache_pool, p2, sz);
- ignored_feature->cache_pool[sz] = '\0';
p2 = grub_strstr (p, "origin = \"");
if (!p2)
@@ -898,7 +874,6 @@ grub_lvm_detect (grub_disk_t disk,
if (ignored_feature)
{
grub_free (ignored_feature->origin);
- grub_free (ignored_feature->cache_pool);
if (ignored_feature->lv)
{
grub_free (ignored_feature->lv->fullname);
--
2.39.2
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 0/3 v8] LVM Cachevol and Integrity volumes break entire LVM VG
In an effort to solve
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1061168 for myself,
I implemented basic support for cachevol and integrity volumes in LVM.
This is just an extension of the cachepool support that already
existed, and just like that support, I ignore all of the metadata and
just go for the data. This obviously means that writing to "writeback"
cachepools, and any raidintegrity volumes will cause corruption as
before, but as grub only reads files, I think that should be fine.
Without these patches, a system with /boot on a LV with cachevol or
integrity volumes will fail to boot
These patches are also available attached and at
https://github.com/byteit101/grub2/commits/grub-lvmintegrity/
Please ignore the ones not labeled "v8", as I had a cherry picking
error. How do you edit patches sanely?
Patrick Plenefisch (3):
disk/lvm: Make cache_lv more generic as ignored_feature_lv
disk/lvm: Remove unused cache_pool
lvm: Add support for cachevol and integrity lv
grub-core/disk/diskfilter.c | 6 +-
grub-core/disk/lvm.c| 267 ++--
2 files changed, 136 insertions(+), 137 deletions(-)
--
2.39.2
From 6dc4ffd21693bf1a1b44090be74219a6df444f8d Mon Sep 17 00:00:00 2001
From: Patrick Plenefisch
Date: Tue, 13 Aug 2024 20:40:02 -0400
Subject: [PATCH 1/3] disk/lvm: Make cache_lv more generic as
ignored_feature_lv
This patch isn't necessary by itself, but when combined with the next two patchs it enhances readability as ignored_features_lv is then used for multiple types of extra LV's, not just cache LV's
Signed-off-by: Patrick Plenefisch
---
grub-core/disk/lvm.c | 176 +--
1 file changed, 88 insertions(+), 88 deletions(-)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 794248540..442a9fc96 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -34,12 +34,12 @@
GRUB_MOD_LICENSE ("GPLv3+");
-struct cache_lv
+struct ignored_feature_lv
{
struct grub_diskfilter_lv *lv;
char *cache_pool;
char *origin;
- struct cache_lv *next;
+ struct ignored_feature_lv *next;
};
@@ -105,30 +105,30 @@ grub_lvm_check_flag (const char *p, const char *str, const char *flag)
}
static void
-grub_lvm_free_cache_lvs (struct cache_lv *cache_lvs)
+grub_lvm_free_ignored_feature_lvs (struct ignored_feature_lv *ignored_feature_lvs)
{
- struct cache_lv *cache;
+ struct ignored_feature_lv *ignored_feature;
- while ((cache = cache_lvs))
+ while ((ignored_feature = ignored_feature_lvs))
{
- cache_lvs = cache_lvs->next;
+ ignored_feature_lvs = ignored_feature_lvs->next;
- if (cache->lv)
+ if (ignored_feature->lv)
{
unsigned int i;
- for (i = 0; i < cache->lv->segment_count; ++i)
- if (cache->lv->segments)
- grub_free (cache->lv->segments[i].nodes);
- grub_free (cache->lv->segments);
- grub_free (cache->lv->fullname);
- grub_free (cache->lv->idname);
- grub_free (cache->lv->name);
+ for (i = 0; i < ignored_feature->lv->segment_count; ++i)
+ if (ignored_feature->lv->segments)
+ grub_free (ignored_feature->lv->segments[i].nodes);
+ grub_free (ignored_feature->lv->segments);
+ grub_free (ignored_feature->lv->fullname);
+ grub_free (ignored_feature->lv->idname);
+ grub_free (ignored_feature->lv->name);
}
- grub_free (cache->lv);
- grub_free (cache->origin);
- grub_free (cache->cache_pool);
- grub_free (cache);
+ grub_free (ignored_feature->lv);
+ grub_free (ignored_feature->origin);
+ grub_free (ignored_feature->cache_pool);
+ grub_free (ignored_feature);
}
}
@@ -325,7 +325,7 @@ grub_lvm_detect (grub_disk_t disk,
if (! vg)
{
- struct cache_lv *cache_lvs = NULL;
+ struct ignored_feature_lv *ignored_feature_lvs = NULL;
/* First time we see this volume group. We've to create the
whole volume group structure. */
@@ -810,105 +810,105 @@ grub_lvm_detect (grub_disk_t disk,
else if (grub_memcmp (p, "cache\"",
sizeof ("cache\"") - 1) == 0)
{
- struct cache_lv *cache = NULL;
+ struct ignored_feature_lv *ignored_feature = NULL;
char *p2, *p3;
grub_size_t sz;
- cache = grub_zalloc (sizeof (*cache));
- if (!cache)
- goto cache_lv_fail;
- cache->lv = grub_zalloc (sizeof (*cache->lv));
- if (!cache->lv)
- goto cache_lv_fail;
- grub_memcpy (cache->lv, lv, sizeof (*cache->lv));
+ ignored_feature = grub_zalloc (sizeof (*ignored_feature));
+ if (!ignored_feature)
+ goto ignored_feature_lv_fail;
+ ignored_feature->lv = grub_zalloc (sizeof (*ignored_feature->lv));
+ if (!ignored_feature->lv)
+ goto ignored_feature_lv_fail;
+ grub_memcpy (ignored_feature->lv, lv, sizeof (*ignored_feature->lv));
if (lv->fullname)
{
- cache->lv->fullname = grub_strdup (lv->fullname);
- if (!cache->lv->fullname)
- goto c
[PATCH 3/3 v8] lvm: Add support for cachevol and integrity lv
lv matching must be done after processing the ignored feature
indirections, as integrity volumes & caches may have several levels
of indirection that the segments must be shifted through.
pv matching must be completely finished before validating a
volume, otherwise referenced raid stripes may not have pv
data applied yet
This patch contains a change requested by Daniel Kiper to use
a null character instead of an integer zero to terminate strings
Signed-off-by: Patrick Plenefisch
---
grub-core/disk/diskfilter.c | 6 ++-
grub-core/disk/lvm.c| 88 +++--
2 files changed, 59 insertions(+), 35 deletions(-)
diff --git a/grub-core/disk/diskfilter.c b/grub-core/disk/diskfilter.c
index 21e239511..dc3bd943b 100644
--- a/grub-core/disk/diskfilter.c
+++ b/grub-core/disk/diskfilter.c
@@ -966,8 +966,6 @@ grub_diskfilter_vg_register (struct grub_diskfilter_vg *vg)
for (lv = vg->lvs; lv; lv = lv->next)
{
- grub_err_t err;
-
/* RAID 1 and single-disk RAID 0 don't use a chunksize but code
assumes one so set one. */
for (i = 0; i < lv->segment_count; i++)
@@ -979,6 +977,10 @@ grub_diskfilter_vg_register (struct grub_diskfilter_vg *vg)
&& lv->segments[i].stripe_size == 0)
lv->segments[i].stripe_size = 64;
}
+}
+ for (lv = vg->lvs; lv; lv = lv->next)
+{
+ grub_err_t err;
err = validate_lv(lv);
if (err)
diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 10bc965a4..02208907d 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -805,13 +805,29 @@ grub_lvm_detect (grub_disk_t disk,
seg->nodes[seg->node_count - 1].name = tmp;
}
}
- else if (grub_memcmp (p, "cache\"",
- sizeof ("cache\"") - 1) == 0)
+ /*
+ * Cache and integrity LVs have extra parts that
+ * we can ignore for our read-only access
+ */
+ else if (grub_strncmp (p, "cache\"",
+ sizeof ("cache\"") - 1) == 0
+ || grub_strncmp (p, "cache+CACHE_USES_CACHEVOL\"",
+ sizeof ("cache+CACHE_USES_CACHEVOL\"") - 1) == 0
+ || grub_strncmp (p, "integrity\"",
+ sizeof ("integrity\"") - 1) == 0)
{
struct ignored_feature_lv *ignored_feature = NULL;
char *p2, *p3;
grub_size_t sz;
+#ifdef GRUB_UTIL
+ p2 = grub_strchr (p, '"');
+ if (p2)
+*p2 = '\0';
+ grub_util_info ("Ignoring extra metadata type '%s' for
%s", p, lv->name);
+ if (p2)
+*p2 ='"';
+#endif
ignored_feature = grub_zalloc (sizeof (*ignored_feature));
if (!ignored_feature)
@@ -892,7 +908,7 @@ grub_lvm_detect (grub_disk_t disk,
char *p2;
p2 = grub_strchr (p, '"');
if (p2)
-*p2 = 0;
+*p2 = '\0';
grub_util_info ("unknown LVM type %s", p);
if (p2)
*p2 ='"';
@@ -936,36 +952,6 @@ grub_lvm_detect (grub_disk_t disk,
}
}
- /* Match lvs. */
- {
-struct grub_diskfilter_lv *lv1;
-struct grub_diskfilter_lv *lv2;
-for (lv1 = vg->lvs; lv1; lv1 = lv1->next)
- for (i = 0; i < lv1->segment_count; i++)
-for (j = 0; j < lv1->segments[i].node_count; j++)
- {
-if (vg->pvs)
- for (pv = vg->pvs; pv; pv = pv->next)
-{
- if (! grub_strcmp (pv->name,
- lv1->segments[i].nodes[j].name))
-{
- lv1->segments[i].nodes[j].pv = pv;
- break;
-}
-}
-if (lv1->segments[i].nodes[j].pv == NULL)
- for (lv2 = vg->lvs; lv2; lv2 = lv2->next)
-{
- if (lv1 == lv2)
-continue;
- if (grub_strcmp (lv2->name,
- lv1->segments[i].nodes[j].name) == 0)
-lv1->segments[i].nodes[j].lv = lv2;
-}
- }
-
- }
{
struct ignored_feature_lv *ignored_feature;
@@ -1014,9 +1000,45 @@ grub_lvm_detect (grub_disk_t disk,
ignored_feature->lv = NULL;
}
}
+ else
+ {
+#ifdef GRUB_UTIL
+ grub_util_info ("Couldn't find LVM part of ignored
feature on %s", ignored_feature->origin);
+#endif
+ }
}
}
+ /* Match lvs. Must be done after cache and integrity are found */
+ {
+struct grub_diskfilter_lv *lv1;
+struct grub_diskfilter_lv *lv2;
+for (lv1 = vg->lvs; lv1; lv1 = lv1->next)
+ for (i = 0; i < lv1->segment_count; i++)
+for (j = 0; j < lv1->segments[i].node_count; j++)
+ {
+if (vg->pvs)
+ for (pv = vg->pvs; pv; pv = pv->next)
+{
+ if (! grub_strcmp (pv->name,
+ lv1
