On Thu, Oct 02, 2025 at 02:46:14PM +0800, Michael Chang via Grub-devel wrote: > 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.
s/grub editenv/grub-editenv/ > Signed-off-by: Michael Chang <[email protected]> > Reviewed-by: Neal Gompa <[email protected]> > Reviewed-by: Avnish Chouhan <[email protected]> > --- > include/grub/fs.h | 2 + > util/grub-editenv.c | 155 +++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 156 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) This change does not belong to this patch and should be moved to the patch #7. > #endif /* ! GRUB_FS_HEADER */ > diff --git a/util/grub-editenv.c b/util/grub-editenv.c > index db6f187cc..1f055ca43 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 { > + const char *fs_name; > + off_t offset; > + size_t size; > +}; > +typedef struct fs_envblk_spec *fs_envblk_spec_t; If you drop "*" from typedf then you can use the fs_envblk_spec_t everywhere... > +struct fs_envblk { > + struct fs_envblk_spec *spec; s/struct fs_envblk_spec/fs_envblk_spec_t/ > + const char *dev; > +}; > +typedef struct fs_envblk *fs_envblk_t; > + > +static struct fs_envblk_spec fs_envblk_spec[] = { s/struct fs_envblk_spec/fs_envblk_spec_t/ > + { "btrfs", ENV_BTRFS_OFFSET, GRUB_DISK_SECTOR_SIZE }, Again, this does not belong to this patch... Additionally, I think you should put a comment here and in grub-core/fs/btrfs.c around btrfs_head variable initialization saying that both fs_envblk_spec and btrfs_head should be kept in sync. > + { NULL, 0, 0 } > +}; > + > +static fs_envblk_t fs_envblk = NULL; > + > static grub_envblk_t > open_envblk_file (const char *name) > { > @@ -253,6 +276,133 @@ 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; > + > + 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 (); > + > + 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]); > + > + for (curdev = grub_devices; *curdev; curdev++, ndev++) *curdev != NULL In general please compare with NULL explicitly. Still I can see this issue in many places in this patch set. > + grub_util_pull_device (*curdev); > + > + grub_drives = xcalloc ((ndev + 1), sizeof (grub_drives[0])); > + > + for (curdev = grub_devices, curdrive = grub_drives; *curdev; curdev++, Ditto... > + 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; *curdrive = NULL; > + > + 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); Please add en empty line after variables definitions. This should be fixed in other places too... > + if (dev == NULL) > + continue; > + have_abstraction = is_abstraction (dev); > + grub_device_close (dev); > + } > + > + cleanup: s/cleanup/cleanup_0/ > + if (grub_devices != NULL) > + for (curdev = grub_devices; *curdev; curdev++) > + free (*curdev); > + free (grub_devices); > + free (grub_drives); > + grub_device_close (grub_dev); > + grub_gcry_fini_all (); > + grub_fini_all (); > + grub_util_biosdisk_fini (); > + > + if (grub_fs == NULL) > + { > + free (devname); > + return NULL; > + } goto cleanup_1; > + > + for (p = spec; p->fs_name; p++) p->fs_name != NULL > + { > + if (strcmp (grub_fs->name, p->fs_name) == 0 && !have_abstraction) have_abstraction == false > + { > + fs_envblk = xmalloc (sizeof (fs_envblk_t)); > + fs_envblk->spec = p; > + fs_envblk->dev = devname; > + return fs_envblk; > + } > + } > + cleanup_1: > + free (devname); > + > + return NULL; > +} Daniel _______________________________________________ Grub-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/grub-devel
