On 2025-09-15 14:39, [email protected] wrote:
Message: 2
Date: Mon, 15 Sep 2025 17:08:40 +0800
From: Michael Chang <[email protected]>
To: The development of GNU GRUB <[email protected]>
Cc: Neal Gompa <[email protected]>, Marta Lewandowska
<[email protected]>
Subject: [PATCH v2 1/9] util/grub-editenv: add basic structures and
probe call for external envblk
Message-ID: <[email protected]>
This patch prepares for using an environment block stored in a reserved
area of the filesystem. It adds a constant ENV_BTRFS_OFFSET at 256 KiB
in the Btrfs header. It also introduces the fs_envblk_spec and
fs_envblk
structures together with the probe_fs_envblk function to identify the
root filesystem and to avoid configurations that involve diskfilter or
cryptodisk.
The probe is only invoked when grub-editenv is working on the default
environment file path. This restriction ensures that probing and
possible raw device access are not triggered for arbitrary user
supplied
paths, but only for the standard grubenv file. In that case the code
checks if the filename equals DEFAULT_ENVBLK_PATH and then calls
probe_fs_envblk with fs_envblk_spec. The result is stored in the global
fs_envblk handle. At this stage the external environment block is only
detected and recorded, and the behavior of grub editenv is unchanged.
Signed-off-by: Michael Chang <[email protected]>
---
include/grub/fs.h | 2 +
util/grub-editenv.c | 153 +++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 154 insertions(+), 1 deletion(-)
diff --git a/include/grub/fs.h b/include/grub/fs.h
index df4c93b16..1be26dfba 100644
--- a/include/grub/fs.h
+++ b/include/grub/fs.h
@@ -132,4 +132,6 @@ grub_fs_unregister (grub_fs_t fs)
grub_fs_t EXPORT_FUNC(grub_fs_probe) (grub_device_t device);
+#define ENV_BTRFS_OFFSET (256 * 1024)
+
#endif /* ! GRUB_FS_HEADER */
diff --git a/util/grub-editenv.c b/util/grub-editenv.c
index db6f187cc..2302a6acf 100644
--- a/util/grub-editenv.c
+++ b/util/grub-editenv.c
@@ -23,8 +23,11 @@
#include <grub/util/misc.h>
#include <grub/lib/envblk.h>
#include <grub/i18n.h>
-#include <grub/emu/hostfile.h>
+#include <grub/emu/hostdisk.h>
#include <grub/util/install.h>
+#include <grub/emu/getroot.h>
+#include <grub/fs.h>
+#include <grub/crypto.h>
#include <stdio.h>
#include <unistd.h>
@@ -120,6 +123,26 @@ block, use `rm %s'."),
NULL, help_filter, NULL
};
+struct fs_envblk_spec {
Hi Michael,
This '{' seems to be in a new line!
+ const char *fs_name;
+ off_t offset;
+ size_t size;
+};
+typedef struct fs_envblk_spec *fs_envblk_spec_t;
+
+struct fs_envblk {
Same as above!
+ struct fs_envblk_spec *spec;
+ const char *dev;
+};
+typedef struct fs_envblk *fs_envblk_t;
+
+static struct fs_envblk_spec fs_envblk_spec[] = {
+ { "btrfs", ENV_BTRFS_OFFSET, GRUB_DISK_SECTOR_SIZE },
+ { NULL, 0, 0 }
+};
+
+static fs_envblk_t fs_envblk = NULL;
+
static grub_envblk_t
open_envblk_file (const char *name)
{
@@ -253,6 +276,131 @@ unset_variables (const char *name, int argc, char
*argv[])
grub_envblk_close (envblk);
}
+static bool
+is_abstraction (grub_device_t dev)
+{
+ if (dev == NULL || dev->disk == NULL)
+ return false;
+
+ if (dev->disk->dev->id == GRUB_DISK_DEVICE_DISKFILTER_ID ||
+ dev->disk->dev->id == GRUB_DISK_DEVICE_CRYPTODISK_ID)
+ return true;
Adding an empty line would be better here!
+ return false;
+}
+
+static fs_envblk_t
+probe_fs_envblk (fs_envblk_spec_t spec)
+{
+ char **grub_devices = NULL;
+ char **curdev, **curdrive;
+ size_t ndev = 0;
+ char **grub_drives = NULL;
+ grub_device_t grub_dev = NULL;
+ grub_fs_t grub_fs = NULL;
+ char *devname = NULL;
+ fs_envblk_spec_t p;
+ bool have_abstraction = false;
+
+ grub_util_biosdisk_init (DEFAULT_DEVICE_MAP);
+ grub_init_all ();
+ grub_gcry_init_all ();
+
+ grub_lvm_fini ();
+ grub_mdraid09_fini ();
+ grub_mdraid1x_fini ();
+ grub_diskfilter_fini ();
+ grub_diskfilter_init ();
+ grub_mdraid09_init ();
+ grub_mdraid1x_init ();
+ grub_lvm_init ();
We aren't checking any failure in these initializes? Are we good without
the checks here.. I'm not sure though!
+
+ grub_devices = grub_guess_root_devices (DEFAULT_DIRECTORY);
+
+ if (grub_devices == NULL || grub_devices[0] == NULL)
+ {
+ grub_util_warn (_("cannot find a device for %s (is /dev
mounted?)"), DEFAULT_DIRECTORY);
+ goto cleanup;
+ }
+
+ devname = xstrdup (grub_devices[0]);
Using grub_strdup would be better. We can check for NULL return in case
of failure!
+
+ for (curdev = grub_devices; *curdev; curdev++, ndev++)
+ grub_util_pull_device (*curdev);
+
+ grub_drives = xcalloc ((ndev + 1), sizeof (grub_drives[0]));
Same as above. grub_calloc...
+
+ for (curdev = grub_devices, curdrive = grub_drives; *curdev;
curdev++,
+ curdrive++)
+ {
+ *curdrive = grub_util_get_grub_dev (*curdev);
+ if (*curdrive == NULL)
+ {
+ grub_util_warn (_("cannot find a GRUB drive for %s. Check your
device.map"),
+ *curdev);
+ goto cleanup;
+ }
+ }
+ *curdrive = 0;
+
+ grub_dev = grub_device_open (grub_drives[0]);
+ if (grub_dev == NULL)
+ {
+ grub_util_warn (_("cannot open device %s: %s"), grub_drives[0],
grub_errmsg);
+ grub_errno = GRUB_ERR_NONE;
+ goto cleanup;
+ }
+
+ grub_fs = grub_fs_probe (grub_dev);
+ if (grub_fs == NULL)
+ {
+ grub_util_warn (_("cannot probe fs for %s: %s"),
grub_drives[0], grub_errmsg);
+ grub_errno = GRUB_ERR_NONE;
+ goto cleanup;
+ }
+
+ have_abstraction = is_abstraction (grub_dev);
+ for (curdrive = grub_drives + 1; *curdrive && have_abstraction ==
false; curdrive++)
+ {
+ grub_device_t dev = grub_device_open (*curdrive);
+ if (dev == NULL)
+ continue;
+ have_abstraction = is_abstraction (dev);
+ grub_device_close (dev);
+ }
+
+ cleanup:
+ if (grub_devices != NULL)
+ for (curdev = grub_devices; *curdev; curdev++)
+ free (*curdev);
+ free (grub_devices);
+ free (grub_drives);
grub_free() instead of free().
+ grub_device_close (grub_dev);
+ grub_gcry_fini_all ();
+ grub_fini_all ();
+ grub_util_biosdisk_fini ();
+
+ if (grub_fs == NULL)
+ {
+ free (devname);
grub_free() instead of free().
+ return NULL;
+ }
+
+ for (p = spec; p->fs_name; p++)
+ {
+ if (strcmp (grub_fs->name, p->fs_name) == 0 &&
!have_abstraction)
Same as mentioned above. grub_strcmp()...
+ {
+ fs_envblk = xmalloc (sizeof (fs_envblk_t));
Same as above. grub_malloc()...
+ fs_envblk->spec = p;
+ fs_envblk->dev = devname;
+ return fs_envblk;
+ }
+ }
+
+ free (devname);
grub_free() instead of free(). And adding a new line here would be good!
+ return NULL;
+}
+
+
int
main (int argc, char *argv[])
{
@@ -284,6 +432,9 @@ main (int argc, char *argv[])
command = argv[curindex++];
}
+ if (strcmp (filename, DEFAULT_ENVBLK_PATH) == 0)
Same as mentioned above. grub_strcmp()...
Thank you!
Regards,
Avnish Chouhan
+ fs_envblk = probe_fs_envblk (fs_envblk_spec);
+
if (strcmp (command, "create") == 0)
grub_util_create_envblk_file (filename);
else if (strcmp (command, "list") == 0)
--
2.51.0
_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
