Re: [SECURITY PATCH 000/117] Multiple GRUB2 vulnerabilities - 2021/03/02 round

2021-03-09 Thread Neal Gompa
On Tue, Mar 2, 2021 at 4:08 PM Daniel Kiper  wrote:
>
> Hi Adrian,
>
> On Tue, Mar 02, 2021 at 08:37:14PM +0100, John Paul Adrian Glaubitz wrote:
> > Hi Daniel!
> >
> > On 3/2/21 7:00 PM, Daniel Kiper wrote:
> > > The BootHole vulnerability [1][2] announced last year encouraged many 
> > > people to
> > > take a closer look at the security of boot process in general and the GRUB
> > > bootloader in particular. Due to that, during past few months we were 
> > > getting
> > > reports of, and also discovering various security flaws in the GRUB 
> > > ourselves.
> > > You can find the list of most severe ones which got CVEs assigned at the 
> > > end of
> > > this message. The patch bundle fixing all these issues in the upstream 
> > > GRUB
> > > contains 117 patches.
> >
> > Huge thanks and kudos to everyone involved fixing all these vulnerabilities!
> >
> > Given the amount of patches, wouldn't it make sense to push an RC candidate
> > for 2.06 in the near future so that distributions can start shipping the 
> > pre-
> > release and avoiding to carry this large amount of patches?
>
> I am planning to cut 2.06-rc1 in matter of days...
>

Any status update on this? The delta between 2.04 and HEAD is huge,
and I'd rather have a release to work from now...


-- 
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v2 26/30] btdfs: grub2-btrfs-05-grub2-mkconfig

2024-10-12 Thread Neal Gompa
On Fri, Oct 11, 2024 at 6:40 PM Leo Sandoval  wrote:
>
> From: Michael Chang 
>
> Signed-off-by: Michael Chang 


This patch needs a real commit message. I know which series of patches
this comes from, and they *all* need real commit messages. Can you and
Michael work to figure out proper messages for these?

> ---
>  util/grub-mkconfig.in   |  3 ++-
>  util/grub-mkconfig_lib.in   |  4 
>  util/grub.d/00_header.in| 26 +-
>  util/grub.d/10_linux.in |  4 
>  util/grub.d/20_linux_xen.in |  4 
>  5 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
> index 91f761331..ad5b778e3 100644
> --- a/util/grub-mkconfig.in
> +++ b/util/grub-mkconfig.in
> @@ -259,7 +259,8 @@ export GRUB_DEFAULT \
>GRUB_BADRAM \
>GRUB_OS_PROBER_SKIP_LIST \
>GRUB_DISABLE_SUBMENU \
> -  GRUB_DEFAULT_DTB
> +  GRUB_DEFAULT_DTB \
> +  SUSE_BTRFS_SNAPSHOT_BOOTING
>

This needs to drop the SUSE_ prefix for upstreaming. Same for the
instances below.

>  if test "x${grub_cfg}" != "x"; then
>rm -f "${grub_cfg}.new"
> diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
> index 33e1750ae..0ba0e0e1c 100644
> --- a/util/grub-mkconfig_lib.in
> +++ b/util/grub-mkconfig_lib.in
> @@ -49,7 +49,11 @@ grub_warn ()
>
>  make_system_path_relative_to_its_root ()
>  {
> +  if [ "x${SUSE_BTRFS_SNAPSHOT_BOOTING}" = "xtrue" ] ; then
> +  "${grub_mkrelpath}" -r "$1"
> +  else
>"${grub_mkrelpath}" "$1"
> +  fi
>  }
>
>  is_path_readable_by_grub ()
> diff --git a/util/grub.d/00_header.in b/util/grub.d/00_header.in
> index 6a316a5ba..4ed7975a9 100644
> --- a/util/grub.d/00_header.in
> +++ b/util/grub.d/00_header.in
> @@ -27,6 +27,15 @@ export TEXTDOMAINDIR="@localedir@"
>
>  . "$pkgdatadir/grub-mkconfig_lib"
>
> +if [ "x${SUSE_BTRFS_SNAPSHOT_BOOTING}" = "xtrue" ] &&
> +   [ "x${GRUB_FS}" = "xbtrfs" ] ; then
> +cat < +set btrfs_relative_path="y"
> +export btrfs_relative_path
> +EOF
> +fi
> +
> +
>  # Do this as early as possible, since other commands might depend on it.
>  # (e.g. the `loadfont' command might need lvm or raid modules)
>  for i in ${GRUB_PRELOAD_MODULES} ; do
> @@ -43,7 +52,9 @@ if [ "x${GRUB_DEFAULT_BUTTON}" = "xsaved" ] ; then 
> GRUB_DEFAULT_BUTTON='${saved_
>  if [ "x${GRUB_TIMEOUT_BUTTON}" = "x" ] ; then 
> GRUB_TIMEOUT_BUTTON="$GRUB_TIMEOUT" ; fi
>
>  cat << EOF
> -if [ -s \$prefix/grubenv ]; then
> +if [ -f \${config_directory}/grubenv ]; then
> +  load_env -f \${config_directory}/grubenv
> +elif [ -s \$prefix/grubenv ]; then
>load_env
>  fi
>  EOF
> @@ -354,3 +365,16 @@ fi
>  if [ "x${GRUB_BADRAM}" != "x" ] ; then
>echo "badram ${GRUB_BADRAM}"
>  fi
> +
> +if [ "x${SUSE_BTRFS_SNAPSHOT_BOOTING}" = "xtrue" ] &&
> +   [ "x${GRUB_ENABLE_BLSCFG}" = "xtrue" ] &&
> +   [ "x${GRUB_FS}" = "xbtrfs" ] ; then
> +   # Note: No $snapshot_num on *read-only* rollback!  (bsc#901487)
> +   cat < +if [ -n "\$extra_cmdline" ]; then
> +  submenu "Bootable snapshot #\$snapshot_num" {
> +menuentry "If OK, run 'snapper rollback' and reboot." { true; }
> +  }
> +fi
> +EOF
> +fi
> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
> index b94af4a8d..192d7f594 100644
> --- a/util/grub.d/10_linux.in
> +++ b/util/grub.d/10_linux.in
> @@ -68,10 +68,14 @@ fi
>
>  case x"$GRUB_FS" in
>  xbtrfs)
> +  if [ "x${SUSE_BTRFS_SNAPSHOT_BOOTING}" = "xtrue" ]; then
> +  GRUB_CMDLINE_LINUX="${GRUB_CMDLINE_LINUX} \${extra_cmdline}"
> +  else
> rootsubvol="`make_system_path_relative_to_its_root /`"
> rootsubvol="${rootsubvol#/}"
> if [ "x${rootsubvol}" != x ]; then
> GRUB_CMDLINE_LINUX="rootflags=subvol=${rootsubvol} 
> ${GRUB_CMDLINE_LINUX}"
> +  fi
> fi;;
>  xzfs)
> rpool=`${grub_probe} --device ${GRUB_DEVICE} --target=fs_label 
> 2>/dev/null || true`
> diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
> index 94dd8be13..a786c87ac 100644
> --- a/util/grub.d/20_linux_xen.in
> +++ b/util/grub.d/20_linux_xen.in
> @@ -75,10 +75,14 @@ fi
>
>  case x"$GRUB_FS" in
>  xbtrfs)
> +  if [ "x${SUSE_BTRFS_SNAPSHOT_BOOTING}" = "xtrue" ]; then
> +  GRUB_CMDLINE_LINUX="${GRUB_CMDLINE_LINUX} \${extra_cmdline}"
> +  else
> rootsubvol="`make_system_path_relative_to_its_root /`"
> rootsubvol="${rootsubvol#/}"
> if [ "x${rootsubvol}" != x ]; then
> GRUB_CMDLINE_LINUX="rootflags=subvol=${rootsubvol} 
> ${GRUB_CMDLINE_LINUX}"
> +  fi
> fi;;
>  xzfs)
> rpool=`${grub_probe} --device ${GRUB_DEVICE} --target=fs_label 
> 2>/dev/null || true`
> --
> 2.46.2
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v2 25/30] btrfs: grub2-btrfs-03-follow_default

2024-10-12 Thread Neal Gompa
On Fri, Oct 11, 2024 at 6:40 PM Leo Sandoval  wrote:
>
> From: Michael Chang 
>
> Signed-off-by: Michael Chang 
> Signed-off-by: Robbie Harwood 

This needs a real commit message describing what this does. Please
work with Michael to come up with one. :)

> ---
>  grub-core/fs/btrfs.c | 107 ++-
>  1 file changed, 76 insertions(+), 31 deletions(-)
>
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index 14e38a4df..d47f9ab03 100644
> --- a/grub-core/fs/btrfs.c
> +++ b/grub-core/fs/btrfs.c
> @@ -1335,6 +1335,7 @@ grub_btrfs_mount (grub_device_t dev)
>  {
>struct grub_btrfs_data *data;
>grub_err_t err;
> +  const char *relpath = grub_env_get ("btrfs_relative_path");
>
>if (!dev->disk)
>  {
> @@ -1365,11 +1366,14 @@ grub_btrfs_mount (grub_device_t dev)
>data->devices_attached[0].dev = dev;
>data->devices_attached[0].id = data->sblock.this_device.device_id;
>
> -  err = btrfs_handle_subvol (data);
> -  if (err)
> +  if (relpath && (relpath[0] == '1' || relpath[0] == 'y'))
>  {
> -  grub_free (data);
> -  return NULL;
> +  err = btrfs_handle_subvol (data);
> +  if (err)
> +  {
> +grub_free (data);
> +return NULL;
> +  }
>  }
>
>return data;
> @@ -1966,24 +1970,39 @@ find_path (struct grub_btrfs_data *data,
>grub_size_t allocated = 0;
>struct grub_btrfs_dir_item *direl = NULL;
>struct grub_btrfs_key key_out;
> +  int follow_default;
>const char *ctoken;
>grub_size_t ctokenlen;
>char *path_alloc = NULL;
>char *origpath = NULL;
>unsigned symlinks_max = 32;
> +  const char *relpath = grub_env_get ("btrfs_relative_path");
>
> +  follow_default = 0;
>origpath = grub_strdup (path);
>if (!origpath)
>  return grub_errno;
>
> -  if (data->fs_tree)
> +  if (relpath && (relpath[0] == '1' || relpath[0] == 'y'))
>  {
> -  *type = GRUB_BTRFS_DIR_ITEM_TYPE_DIRECTORY;
> -  *tree = data->fs_tree;
> -  /* This is a tree root, so everything starts at objectid 256 */
> -  key->object_id = grub_cpu_to_le64_compile_time 
> (GRUB_BTRFS_OBJECT_ID_CHUNK);
> -  key->type = GRUB_BTRFS_ITEM_TYPE_DIR_ITEM;
> -  key->offset = 0;
> +  if (data->fs_tree)
> +{
> +  *type = GRUB_BTRFS_DIR_ITEM_TYPE_DIRECTORY;
> +  *tree = data->fs_tree;
> +  /* This is a tree root, so everything starts at objectid 256 */
> +  key->object_id = grub_cpu_to_le64_compile_time 
> (GRUB_BTRFS_OBJECT_ID_CHUNK);
> +  key->type = GRUB_BTRFS_ITEM_TYPE_DIR_ITEM;
> +  key->offset = 0;
> +}
> +  else
> +{
> +  *type = GRUB_BTRFS_DIR_ITEM_TYPE_DIRECTORY;
> +  *tree = data->sblock.root_tree;
> +  key->object_id = data->sblock.root_dir_objectid;
> +  key->type = GRUB_BTRFS_ITEM_TYPE_DIR_ITEM;
> +  key->offset = 0;
> +  follow_default = 1;
> +}
>  }
>else
>  {
> @@ -1994,15 +2013,23 @@ find_path (struct grub_btrfs_data *data,
>
>while (1)
>  {
> -  while (path[0] == '/')
> -   path++;
> -  if (!path[0])
> -   break;
> -  slash = grub_strchr (path, '/');
> -  if (!slash)
> -   slash = path + grub_strlen (path);
> -  ctoken = path;
> -  ctokenlen = slash - path;
> +  if (!follow_default)
> +   {
> + while (path[0] == '/')
> +   path++;
> + if (!path[0])
> +   break;
> + slash = grub_strchr (path, '/');
> + if (!slash)
> +   slash = path + grub_strlen (path);
> + ctoken = path;
> + ctokenlen = slash - path;
> +   }
> +  else
> +   {
> + ctoken = "default";
> + ctokenlen = sizeof ("default") - 1;
> +   }
>
>if (*type != GRUB_BTRFS_DIR_ITEM_TYPE_DIRECTORY)
> {
> @@ -2013,7 +2040,9 @@ find_path (struct grub_btrfs_data *data,
>
>if (ctokenlen == 1 && ctoken[0] == '.')
> {
> - path = slash;
> + if (!follow_default)
> +   path = slash;
> + follow_default = 0;
>   continue;
> }
>if (ctokenlen == 2 && ctoken[0] == '.' && ctoken[1] == '.')
> @@ -2044,8 +2073,9 @@ find_path (struct grub_btrfs_data *data,
>   *type = GRUB_BTRFS_DIR_ITEM_TYPE_DIRECTORY;
>   key->object_id = key_out.offset;
>
> - path = slash;
> -
> + if (!follow_default)
> +   path = slash;
> + follow_default = 0;
>   continue;
> }
>
> @@ -2114,7 +2144,9 @@ find_path (struct grub_btrfs_data *data,
>   return err;
> }
>
> -  path = slash;
> +  if (!follow_default)
> +   path = slash;
> +  follow_default = 0;
>if (cdirel->type == GRUB_BTRFS_DIR_ITEM_TYPE_SYMLINK)
> {
>   struct grub_btrfs_inode inode;
> @@ -2164,14 +2196,26 @@ find_path (struct grub_btrfs_data *data,
>   path = path_alloc = tmp;
>   if (path[0] == '/

Re: [PATCH v2 00/30] Second Distro-agnostic series taken from Fedora Rawhide

2024-10-12 Thread Neal Gompa
On Fri, Oct 11, 2024 at 6:39 PM Leo Sandoval  wrote:
>
> This is the second patch series, taken from Fedora Rawhide spec [1] that
> is distro-agnostic. The goal is to merge most of them so all the 
> community/distros
> would benefit.
>
> Changes since v1:
> - delete 0024-efinet-and-bootp-add-support-for-dhcpv6.patch
>   build issue, so for the moment discarting it
>
> - delete 0025-bootp-New-net_bootp6-command.patch
>   patch will be sent separately by mchang
>
> - delete 0027-Make-grub_fatal-also-backtrace.patch
>   not accepted
>
> - delete 0028-Make-our-info-pages-say-grub2-where-appropriate.patch
>   non-distro agnostic
>
> - delete 0034-grub2-btrfs-04-grub2-install.patch
>   patch will be revisited by author (mchang)
>
> - delete 0036-grub2-btrfs-06-subvol-mount.patch
>   not accepted
>
> Andrei Borzenkov (1):
>   btrfs: Fallback to old subvol name scheme to support old snapshot
> config
>
> Jeff Mahoney (1):
>   btrfs: Add ability to boot from subvolumes
>
> Josef Bacik (1):
>   net/tcp: add window scaling support
>
> Michael Chang (4):
>   btrfs: export btrfs_subvol and btrfs_subvolid
>   btrfs: grub2-btrfs-03-follow_default
>   btdfs: grub2-btrfs-05-grub2-mkconfig
>   btrfs: Grub not working correctly with btrfs snapshots (bsc#1026511)
>
> Peter Jones (6):
>   10_linux.in: Make grub2-mkconfig construct titles that look like the
> ones we want elsewhere.
>   grub-get-kernel-settings.in: Add grub-get-kernel-settings and use it
> in 10_linux
>   30_os-prober.in: just build chainloader entries, don't try any xnu
> xnu.
>   btrfs: fix a bad null check
>   efi: Add grub_efi_allocate_pool() and grub_efi_free_pool() wrappers.
>   chainloader: Use grub_efi_...() memory helpers where reasonable.
>
> Robert Marshall (1):
>   grub-set-password.in: Add friendly grub2 password config tool
> (#985962)
>

I appreciate that you're doing this, thank you! I've given some
feedback on the btrfs patches, though alas I missed the v1 round where
some of the patches were debated.



-- 
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v2 28/30] btrfs: Grub not working correctly with btrfs snapshots (bsc#1026511)

2024-10-12 Thread Neal Gompa
On Fri, Oct 11, 2024 at 6:39 PM Leo Sandoval  wrote:
>
> From: Michael Chang 
>
> Signed-off-by: Michael Chang 
> Signed-off-by: Robbie Harwood 

Can we have a description of what this patch does for the commit
message? The bug report[1] is private, so I have no context here.

[1]: https://bugzilla.suse.com/1026511





--
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v1 33/37] btrfs: grub2-btrfs-06-subvol-mount

2024-10-15 Thread Neal Gompa
On Tue, Oct 8, 2024 at 2:59 AM Michael Chang via Grub-devel
 wrote:
>
> On Mon, Oct 07, 2024 at 09:40:40PM GMT, Vladimir 'phcoder' Serbinenko wrote:
> > What do you try to achieve with this that can't be achieved with using full
> > path? We should avoid using hidden state for directory parsing. Solaris
> > GRUB legacy used it for ZFS and it was a mess.
>
> If a path is relative to the default subvolume, it cannot access files
> outside of the default subvolume tree. This patch enables access to
> files in other subvolumes by mounting them to a specific directory
>

This is incredibly useful functionality, especially if you have a
non-hierarchical subvolume setup (sometimes called a flat setup) like
Fedora and a few other distributions do. If you want snapshots to
exist outside of the root subvolume or default subvolume, then this
patch is needed or you cannot access them properly for boot to
snapshot.



-- 
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v3 22/29] btrfs: Add ability to boot from subvolumes

2024-10-18 Thread Neal Gompa
+  if (err)
> +{
> +  r = -err;
> +  break;
> +}
> +buf[elemsize] = 0;
> +
> +  find_pathname(data, ref->dirid, fs_root, ref->name, &p);
> +
> +  if (print)
> +{
> +  if (num_only)
> +grub_printf("ID %"PRIuGRUB_UINT64_T"\n", key_out.offset);
> +  else if (path_only)
> +grub_printf("%s\n", p);
> +  else
> +grub_printf("ID %"PRIuGRUB_UINT64_T" path %s\n", key_out.offset, 
> p);
> +} else {
> +  char *old = output;
> +  if (num_only)
> +output = grub_xasprintf("%s%"PRIuGRUB_UINT64_T"\n",
> +old ?: "", key_out.offset);
> +  else if (path_only)
> +output = grub_xasprintf("%s%s\n", old ?: "", p);
> +  else
> +output = grub_xasprintf("%sID %"PRIuGRUB_UINT64_T" path %s\n",
> +old ?: "", key_out.offset, p);
> +
> +  if (old)
> +grub_free(old);
> +}
> +
> +  r = next(data, &desc, &elemaddr, &elemsize, &key_out);
> +  } while(r > 0);
> +
> +  if (output)
> +grub_env_set(varname, output);
> +
> +out:
> +  free_iterator(&desc);
> +  grub_btrfs_unmount(data);
> +
> +  grub_device_close (dev);
> +
> +  return 0;
> +}
> +
>  static struct grub_fs grub_btrfs_fs = {
>.name = "btrfs",
>.fs_dir = grub_btrfs_dir,
> @@ -2411,12 +2847,88 @@ static struct grub_fs grub_btrfs_fs = {
>  #endif
>  };
>
> +static grub_command_t cmd_info;
> +static grub_extcmd_t cmd_list_subvols;
> +
> +static char *
> +subvolid_set_env (struct grub_env_var *var __attribute__ ((unused)),
> +  const char *val)
> +{
> +  unsigned long long result = 0;
> +
> +  grub_errno = GRUB_ERR_NONE;
> +  if (*val)
> +{
> +  result = grub_strtoull(val, NULL, 10);
> +  if (grub_errno)
> +return NULL;
> +}
> +
> +  grub_free (btrfs_default_subvol);
> +  btrfs_default_subvol = NULL;
> +  btrfs_default_subvolid = result;
> +  return grub_strdup(val);
> +}
> +
> +static const char *
> +subvolid_get_env (struct grub_env_var *var __attribute__ ((unused)),
> +  const char *val __attribute__ ((unused)))
> +{
> +  if (btrfs_default_subvol)
> +return grub_xasprintf("subvol:%s", btrfs_default_subvol);
> +  else if (btrfs_default_subvolid)
> +return grub_xasprintf("%"PRIuGRUB_UINT64_T, btrfs_default_subvolid);
> +  else
> +return "";
> +}
> +
> +static char *
> +subvol_set_env (struct grub_env_var *var __attribute__ ((unused)),
> +const char *val)
> +{
> +  grub_free (btrfs_default_subvol);
> +  btrfs_default_subvol = grub_strdup (val);
> +  btrfs_default_subvolid = 0;
> +  return grub_strdup(val);
> +}
> +
> +static const char *
> +subvol_get_env (struct grub_env_var *var __attribute__ ((unused)),
> +const char *val __attribute__ ((unused)))
> +{
> +  if (btrfs_default_subvol)
> +return btrfs_default_subvol;
> +  else if (btrfs_default_subvolid)
> +return grub_xasprintf("subvolid:%" PRIuGRUB_UINT64_T,
> +  btrfs_default_subvolid);
> +  else
> +return "";
> +}
> +
>  GRUB_MOD_INIT (btrfs)
>  {
>grub_fs_register (&grub_btrfs_fs);
> +  cmd_info = grub_register_command("btrfs-info", grub_cmd_btrfs_info,
> +  "DEVICE",
> +  "Print BtrFS info about DEVICE.");
> +  cmd_list_subvols = grub_register_extcmd("btrfs-list-subvols",
> +grub_cmd_btrfs_list_subvols, 0,
> +"[-p|-n] [-o var] DEVICE",
> +"Print list of BtrFS subvolumes on "
> +"DEVICE.", options);
> +  grub_register_variable_hook ("btrfs_subvol", subvol_get_env,
> +   subvol_set_env);
> +  grub_register_variable_hook ("btrfs_subvolid", subvolid_get_env,
> +   subvolid_set_env);
>  }
>
>  GRUB_MOD_FINI (btrfs)
>  {
> +  grub_register_variable_hook ("btrfs_subvol", NULL, NULL);
> +  grub_register_variable_hook ("btrfs_subvolid", NULL, NULL);
> +  grub_unregister_command (cmd_info);
> +  grub_unregister_extcmd (cmd_list_subvols);
>grub_fs_unregister (&grub_btrfs_fs);
>  }
> +
> +// vim: si et sw=2:
> diff --git a/include/grub/btrfs.h b/include/grub/btrfs.h
> index 9d93fb6c1..234ad9767 100644
> --- a/include/grub/btrfs.h
> +++ b/include/grub/btrfs.h
> @@ -29,6 +29,7 @@ enum
>  GRUB_BTRFS_ITEM_TYPE_ROOT_ITEM = 0x84,
>  GRUB_BTRFS_ITEM_TYPE_ROOT_BACKREF = 0x90,
>  GRUB_BTRFS_ITEM_TYPE_DEVICE = 0xd8,
> +GRUB_BTRFS_ITEM_TYPE_ROOT_REF = 0x9c,
>  GRUB_BTRFS_ITEM_TYPE_CHUNK = 0xe4
>};
>
> --
> 2.46.2
>

LGTM. :)

Reviewed-by: Neal Gompa 



-- 
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v3 23/29] btrfs: fix a bad null check

2024-10-18 Thread Neal Gompa
On Mon, Oct 14, 2024 at 1:10 PM Leo Sandoval  wrote:
>
> From: Peter Jones 
>
> current gcc complains:
>
>   grub-core/fs/btrfs.c: In function ‘grub_cmd_btrfs_info’:
>   grub-core/fs/btrfs.c:2745:7: error: the comparison will always evaluate as 
> ‘true’ for the address of ‘label’ will never be NULL [-Werror=address]
>2745 |   if (data->sblock.label)
> |   ^~~~
>   grub-core/fs/btrfs.c:92:8: note: ‘label’ declared here
>  92 |   char label[0x100];
> |^
>   cc1: all warnings being treated as errors
>
> Obviously this check should be on the first data byte instead of the
> symbol itself.
>
> Signed-off-by: Peter Jones 
> ---
>  grub-core/fs/btrfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index f14fe9c1b..8e2b1e9f7 100644
> --- a/grub-core/fs/btrfs.c
> +++ b/grub-core/fs/btrfs.c
> @@ -2625,7 +2625,7 @@ grub_cmd_btrfs_info (grub_command_t cmd __attribute__ 
> ((unused)), int argc,
>return grub_error (GRUB_ERR_BAD_ARGUMENT, "failed to open fs");
>  }
>
> -  if (data->sblock.label)
> +  if (data->sblock.label[0])
>  grub_printf("Label: '%s' ", data->sblock.label);
>else
>  grub_printf("Label: none ");
> --
> 2.46.2
>

Straightforward and reasonable.

Reviewed-by: Neal Gompa 



-- 
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v3 24/29] btrfs: export btrfs_subvol and btrfs_subvolid

2024-10-18 Thread Neal Gompa
On Mon, Oct 14, 2024 at 1:10 PM Leo Sandoval  wrote:
>
> From: Michael Chang 
>
> We should export btrfs_subvol and btrfs_subvolid to have both visible
> to subsidiary configuration files loaded using configfile.
>
> Signed-off-by: Michael Chang 
> ---
>  grub-core/fs/btrfs.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index 8e2b1e9f7..14e38a4df 100644
> --- a/grub-core/fs/btrfs.c
> +++ b/grub-core/fs/btrfs.c
> @@ -2920,6 +2920,8 @@ GRUB_MOD_INIT (btrfs)
> subvol_set_env);
>grub_register_variable_hook ("btrfs_subvolid", subvolid_get_env,
> subvolid_set_env);
> +  grub_env_export ("btrfs_subvol");
> +  grub_env_export ("btrfs_subvolid");
>  }
>
>  GRUB_MOD_FINI (btrfs)
> --
> 2.46.2
>

LGTM. :)

Reviewed-by: Neal Gompa 



-- 
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v3 26/29] grub-mkconfig: Add GRUB_BTRFS_SNAPSHOT_BOOTING simple configuration

2024-10-18 Thread Neal Gompa
On Mon, Oct 14, 2024 at 1:10 PM Leo Sandoval  wrote:
>
> From: Michael Chang 
>
> To support btrfs rollback and snapshot booting, a simple configuration setting
> GRUB_BTRFS_SNAPSHOT_BOOTING is introduced to enable this at the configuration
> level. When set to "yes," grub-mkconfig will set btrfs_relative_path=y, and 
> all
> paths produced by grub-mkrelpath will be relative to the default subvolume.
>
> Signed-off-by: Michael Chang 
> ---
>  util/grub-mkconfig.in   |  4 +++-
>  util/grub-mkconfig_lib.in   |  4 
>  util/grub.d/00_header.in| 25 -
>  util/grub.d/10_linux.in |  4 
>  util/grub.d/20_linux_xen.in |  4 
>  5 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
> index 91f761331..b9932147d 100644
> --- a/util/grub-mkconfig.in
> +++ b/util/grub-mkconfig.in
> @@ -259,7 +259,9 @@ export GRUB_DEFAULT \
>GRUB_BADRAM \
>GRUB_OS_PROBER_SKIP_LIST \
>GRUB_DISABLE_SUBMENU \
> -  GRUB_DEFAULT_DTB
> +  GRUB_DEFAULT_DTB \
> +  GRUB_BTRFS_SNAPSHOT_BOOTING
> +
>
>  if test "x${grub_cfg}" != "x"; then
>rm -f "${grub_cfg}.new"
> diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
> index 33e1750ae..71e75f0bc 100644
> --- a/util/grub-mkconfig_lib.in
> +++ b/util/grub-mkconfig_lib.in
> @@ -49,7 +49,11 @@ grub_warn ()
>
>  make_system_path_relative_to_its_root ()
>  {
> +  if [ "x${GRUB_BTRFS_SNAPSHOT_BOOTING}" = "xtrue" ] ; then
> +  "${grub_mkrelpath}" -r "$1"
> +  else
>"${grub_mkrelpath}" "$1"
> +  fi
>  }
>
>  is_path_readable_by_grub ()
> diff --git a/util/grub.d/00_header.in b/util/grub.d/00_header.in
> index 6a316a5ba..2e94a4c95 100644
> --- a/util/grub.d/00_header.in
> +++ b/util/grub.d/00_header.in
> @@ -27,6 +27,14 @@ export TEXTDOMAINDIR="@localedir@"
>
>  . "$pkgdatadir/grub-mkconfig_lib"
>
> +if [ "x${GRUB_BTRFS_SNAPSHOT_BOOTING}" = "xtrue" ] &&
> +   [ "x${GRUB_FS}" = "xbtrfs" ] ; then
> +cat < +set btrfs_relative_path="y"
> +export btrfs_relative_path
> +EOF
> +fi
> +
>  # Do this as early as possible, since other commands might depend on it.
>  # (e.g. the `loadfont' command might need lvm or raid modules)
>  for i in ${GRUB_PRELOAD_MODULES} ; do
> @@ -43,7 +51,9 @@ if [ "x${GRUB_DEFAULT_BUTTON}" = "xsaved" ] ; then 
> GRUB_DEFAULT_BUTTON='${saved_
>  if [ "x${GRUB_TIMEOUT_BUTTON}" = "x" ] ; then 
> GRUB_TIMEOUT_BUTTON="$GRUB_TIMEOUT" ; fi
>
>  cat << EOF
> -if [ -s \$prefix/grubenv ]; then
> +if [ -f \${config_directory}/grubenv ]; then
> +  load_env -f \${config_directory}/grubenv
> +elif [ -s \$prefix/grubenv ]; then
>load_env
>  fi
>  EOF
> @@ -354,3 +364,16 @@ fi
>  if [ "x${GRUB_BADRAM}" != "x" ] ; then
>echo "badram ${GRUB_BADRAM}"
>  fi
> +
> +if [ "x${GRUB_BTRFS_SNAPSHOT_BOOTING}" = "xtrue" ] &&
> +   [ "x${GRUB_ENABLE_BLSCFG}" = "xtrue" ] &&
> +   [ "x${GRUB_FS}" = "xbtrfs" ] ; then
> +# Note: No $snapshot_num on *read-only* rollback!  (bsc#901487)
> +cat < +if [ -n "\$extra_cmdline" ]; then
> +  submenu "Bootable snapshot #\$snapshot_num" {
> +menuentry "If OK, run 'snapper rollback' and reboot." { true; }
> +  }
> +fi
> +EOF
> +fi
> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
> index b94af4a8d..c7a691051 100644
> --- a/util/grub.d/10_linux.in
> +++ b/util/grub.d/10_linux.in
> @@ -68,10 +68,14 @@ fi
>
>  case x"$GRUB_FS" in
>  xbtrfs)
> +   if [ "x${GRUB_BTRFS_SNAPSHOT_BOOTING}" = "xtrue" ]; then
> +   GRUB_CMDLINE_LINUX="${GRUB_CMDLINE_LINUX} \${extra_cmdline}"
> +   else
> rootsubvol="`make_system_path_relative_to_its_root /`"
> rootsubvol="${rootsubvol#/}"
> if [ "x${rootsubvol}" != x ]; then
> GRUB_CMDLINE_LINUX="rootflags=subvol=${rootsubvol} 
> ${GRUB_CMDLINE_LINUX}"
> +   fi
> fi;;
>  xzfs)
> rpool=`${grub_probe} --device ${GRUB_DEVICE} --target=fs_label 
> 2>/dev/null || true`
> diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
> index 94dd8be13..d6573bd8f 100644
> --- a/util/grub.d/20_linux_xen.in
> +++ b/util/grub.d/20_linux_xen.in
> @@ -75,10 +75,14 @@ fi
>
>  case x"$GRUB_FS" in
>  xbtrfs)
> +   if [ "x${GRUB_BTRFS_SNAPSHOT_BOOTING}" = "xtrue" ]; then
> +   GRUB_CMDLINE_LINUX="${GRUB_CMDLINE_LINUX} \${extra_cmdline}"
> +   else
> rootsubvol="`make_system_path_relative_to_its_root /`"
> rootsubvol="${rootsubvol#/}"
> if [ "x${rootsubvol}" != x ]; then
> GRUB_CMDLINE_LINUX="rootflags=subvol=${rootsubvol} 
> ${GRUB_CMDLINE_LINUX}"
> +   fi
> fi;;
>  xzfs)
> rpool=`${grub_probe} --device ${GRUB_DEVICE} --target=fs_label 
> 2>/dev/null || true`
> --
> 2.46.2
>


LGTM. :)

Reviewed-by: Neal Gompa 



-- 
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v3 25/29] btrfs: Allow specifying btrfs_relative_path to follow the default subvolume

2024-10-18 Thread Neal Gompa
+ ctoken = "default";
> + ctokenlen = sizeof ("default") - 1;
> +   }
>
>if (*type != GRUB_BTRFS_DIR_ITEM_TYPE_DIRECTORY)
> {
> @@ -2013,7 +2040,9 @@ find_path (struct grub_btrfs_data *data,
>
>if (ctokenlen == 1 && ctoken[0] == '.')
> {
> - path = slash;
> + if (!follow_default)
> +   path = slash;
> + follow_default = 0;
>   continue;
> }
>if (ctokenlen == 2 && ctoken[0] == '.' && ctoken[1] == '.')
> @@ -2044,8 +2073,9 @@ find_path (struct grub_btrfs_data *data,
>   *type = GRUB_BTRFS_DIR_ITEM_TYPE_DIRECTORY;
>   key->object_id = key_out.offset;
>
> - path = slash;
> -
> + if (!follow_default)
> +   path = slash;
> + follow_default = 0;
>   continue;
> }
>
> @@ -2114,7 +2144,9 @@ find_path (struct grub_btrfs_data *data,
>   return err;
> }
>
> -  path = slash;
> +  if (!follow_default)
> +   path = slash;
> +  follow_default = 0;
>if (cdirel->type == GRUB_BTRFS_DIR_ITEM_TYPE_SYMLINK)
> {
>   struct grub_btrfs_inode inode;
> @@ -2164,14 +2196,26 @@ find_path (struct grub_btrfs_data *data,
>   path = path_alloc = tmp;
>   if (path[0] == '/')
> {
> - if (data->fs_tree)
> +  if (relpath && (relpath[0] == '1' || relpath[0] == 'y'))
> {
> - *type = GRUB_BTRFS_DIR_ITEM_TYPE_DIRECTORY;
> - *tree = data->fs_tree;
> - /* This is a tree root, so everything starts at objectid 
> 256 */
> - key->object_id = grub_cpu_to_le64_compile_time 
> (GRUB_BTRFS_OBJECT_ID_CHUNK);
> - key->type = GRUB_BTRFS_ITEM_TYPE_DIR_ITEM;
> - key->offset = 0;
> + if (data->fs_tree)
> +   {
> + *type = GRUB_BTRFS_DIR_ITEM_TYPE_DIRECTORY;
> + *tree = data->fs_tree;
> + /* This is a tree root, so everything starts at 
> objectid 256 */
> + key->object_id = grub_cpu_to_le64_compile_time 
> (GRUB_BTRFS_OBJECT_ID_CHUNK);
> +     key->type = GRUB_BTRFS_ITEM_TYPE_DIR_ITEM;
> + key->offset = 0;
> +   }
> + else
> +   {
> + *type = GRUB_BTRFS_DIR_ITEM_TYPE_DIRECTORY;
> + *tree = data->sblock.root_tree;
> + key->object_id = data->sblock.root_dir_objectid;
> + key->type = GRUB_BTRFS_ITEM_TYPE_DIR_ITEM;
> + key->offset = 0;
> + follow_default = 1;
> +   }
> }
>   else
> {
> @@ -2922,6 +2966,7 @@ GRUB_MOD_INIT (btrfs)
> subvolid_set_env);
>grub_env_export ("btrfs_subvol");
>grub_env_export ("btrfs_subvolid");
> +  grub_env_export ("btrfs_relative_path");
>  }
>
>  GRUB_MOD_FINI (btrfs)
> --
> 2.46.2
>

LGTM. :)

Reviewed-by: Neal Gompa 



-- 
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v3 27/29] btrfs: add btrfs-get-default-subvol command

2024-10-18 Thread Neal Gompa
data->sblock.root_dir_objectid;
> +  key.type = GRUB_BTRFS_ITEM_TYPE_DIR_ITEM;
> +  key.offset = grub_cpu_to_le64 (~grub_getcrc32c (1, ctoken, ctokenlen));
> +  err = lower_bound (data, &key, &key_out, data->sblock.root_tree, 
> &elemaddr, &elemsize,
> +NULL, 0);
> +  if (err)
> +return err;
> +
> +  if (key_cmp (&key, &key_out) != 0)
> +return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("file not found"));
> +
> +  struct grub_btrfs_dir_item *cdirel;
> +  direl = grub_malloc (elemsize + 1);
> +  err = grub_btrfs_read_logical (data, elemaddr, direl, elemsize, 0);
> +  if (err)
> +{
> +  grub_free (direl);
> +  return err;
> +}
> +  for (cdirel = direl;
> +   (grub_uint8_t *) cdirel - (grub_uint8_t *) direl
> +   < (grub_ssize_t) elemsize;
> +   cdirel = (void *) ((grub_uint8_t *) (direl + 1)
> +   + grub_le_to_cpu16 (cdirel->n)
> +   + grub_le_to_cpu16 (cdirel->m)))
> +{
> +  if (ctokenlen == grub_le_to_cpu16 (cdirel->n)
> +&& grub_memcmp (cdirel->name, ctoken, ctokenlen) == 0)
> +  break;
> +}
> +  if ((grub_uint8_t *) cdirel - (grub_uint8_t *) direl
> +  >= (grub_ssize_t) elemsize)
> +{
> +  grub_free (direl);
> +  err = grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("file not found"));
> +  return err;
> +}
> +
> +  if (cdirel->key.type != GRUB_BTRFS_ITEM_TYPE_ROOT_ITEM)
> +{
> +  grub_free (direl);
> +  err = grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("file not found"));
> +  return err;
> +}
> +
> +  *id = grub_le_to_cpu64 (cdirel->key.object_id);
> +  return GRUB_ERR_NONE;
> +}
> +
> +static grub_err_t
> +grub_cmd_btrfs_get_default_subvol (struct grub_extcmd_context *ctxt,
> +int argc, char **argv)
> +{
> +  char *devname;
> +  grub_device_t dev;
> +  struct grub_btrfs_data *data;
> +  grub_err_t err;
> +  grub_uint64_t id;
> +  char *subvol = NULL;
> +  grub_uint64_t subvolid = 0;
> +  char *varname = NULL;
> +  char *output = NULL;
> +  int path_only = ctxt->state[1].set;
> +  int num_only = ctxt->state[2].set;
> +
> +  if (ctxt->state[0].set)
> +varname = ctxt->state[0].arg;
> +
> +  if (argc < 1)
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name required");
> +
> +  devname = grub_file_get_device_name(argv[0]);
> +  if (!devname)
> +return grub_errno;
> +
> +  dev = grub_device_open (devname);
> +  grub_free (devname);
> +  if (!dev)
> +return grub_errno;
> +
> +  data = grub_btrfs_mount(dev);
> +  if (!data)
> +{
> +  grub_device_close (dev);
> +  grub_dprintf ("btrfs", "failed to open fs\n");
> +  grub_errno = GRUB_ERR_NONE;
> +  return 0;
> +}
> +
> +  err = grub_btrfs_get_default_subvolume_id (data, &subvolid);
> +  if (err)
> +{
> +  grub_btrfs_unmount (data);
> +  grub_device_close (dev);
> +  return err;
> +}
> +
> +  id = subvolid;
> +  while (id != GRUB_BTRFS_ROOT_VOL_OBJECTID)
> +{
> +  grub_uint64_t parent_id;
> +  char *path_out;
> +
> +  err = grub_btrfs_get_parent_subvol_path (data, grub_cpu_to_le64 (id), 
> subvol, &parent_id, &path_out);
> +  if (err)
> +   {
> + grub_btrfs_unmount (data);
> + grub_device_close (dev);
> + return err;
> +   }
> +
> +  if (subvol)
> +grub_free (subvol);
> +  subvol = path_out;
> +  id = parent_id;
> +}
> +
> +  if (num_only && path_only)
> +  output = grub_xasprintf ("%"PRIuGRUB_UINT64_T" /%s", subvolid, subvol);
> +  else if (num_only)
> +  output = grub_xasprintf ("%"PRIuGRUB_UINT64_T, subvolid);
> +  else
> +  output = grub_xasprintf ("/%s", subvol);
> +
> +  if (varname)
> +grub_env_set(varname, output);
> +  else
> +grub_printf ("%s\n", output);
> +
> +  grub_free (output);
> +  grub_free (subvol);
> +
> +  grub_btrfs_unmount (data);
> +  grub_device_close (dev);
> +
> +  return GRUB_ERR_NONE;
> +}
> +
>  static struct grub_fs grub_btrfs_fs = {
>.name = "btrfs",
>.fs_dir = grub_btrfs_dir,
> @@ -2893,6 +3125,7 @@ static struct grub_fs grub_btrfs_fs = {
>
>  static grub_command_t cmd_info;
>  static grub_extcmd_t cmd_list_subvols;
> +static grub_extcmd_t cmd_get_default_subvol;
>
>  static char *
>  subvolid_set_env (struct grub_env_var *var __attribute__ ((unused)),
> @@ -2960,6 +3193,11 @@ GRUB_MOD_INIT (btrfs)
>  "[-p|-n] [-o var] DEVICE",
>  "Print list of BtrFS subvolumes on "
>  "DEVICE.", options);
> +  cmd_get_default_subvol = grub_register_extcmd("btrfs-get-default-subvol",
> +grub_cmd_btrfs_get_default_subvol, 0,
> +"[-p|-n] [-o var] DEVICE",
> +"Print default BtrFS subvolume on "
> +"DEVICE.", options);
>grub_register_variable_hook ("btrfs_subvol", subvol_get_env,
> subvol_set_env);
>grub_register_variable_hook ("btrfs_subvolid", subvolid_get_env,
> --
> 2.46.2
>

LGTM. :)

Reviewed-by: Neal Gompa 



-- 
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v3 22/29] btrfs: Add ability to boot from subvolumes

2024-10-18 Thread Neal Gompa
On Fri, Oct 18, 2024 at 1:39 PM Vladimir 'phcoder' Serbinenko
 wrote:
>
>
>
> Le lun. 14 oct. 2024, 20:10, Leo Sandoval  a écrit :
>>
>> From: Jeff Mahoney 
>>
>> This patch adds the ability to specify a different root on a btrfs
>> filesystem too boot from other than the default one.
>
> Does it make available some files that are currentl y unavailable? Do you 
> have an example?

This is currently in use by openSUSE's boot to snapshot method, and
people also do this on Fedora manually as we provide all the tools to
do it.


-- 
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v3 22/29] btrfs: Add ability to boot from subvolumes

2024-10-22 Thread Neal Gompa
On Mon, Oct 21, 2024 at 4:29 AM Vladimir 'phcoder' Serbinenko
 wrote:
>
>
>
> Le lun. 21 oct. 2024, 06:49, Michael Chang via Grub-devel 
>  a écrit :
>>
>> On Fri, Oct 18, 2024 at 08:39:01PM GMT, Vladimir 'phcoder' Serbinenko wrote:
>> > Le lun. 14 oct. 2024, 20:10, Leo Sandoval  a écrit :
>> >
>> > > From: Jeff Mahoney 
>> > >
>> > > This patch adds the ability to specify a different root on a btrfs
>> > > filesystem too boot from other than the default one.
>> > >
>> > Does it make available some files that are currentl y unavailable? Do you
>> > have an example?
>>
>> Say, if we want to boot snapshot "foo" via it's grub.cfg, the config can
>> be written as:
>>
>>   menuentry "boot snapshot foo" {
>> saved_subvol=$btrfs_subvol
>> btrfs_subvol=foo
>> configfile /boot/grub/grub.cfg
>> btrfs_subvol=$saved_subvol
>>   }
>>
>> In combination with relative path, the grub.cfg executed from foo
>> snapshot is also set to use files from foo snapshots explicitly. rather
>> than default or other sources.
>
> It's not the question I asked

I'm sorry I don't understand what you're asking. Perhaps this explanation helps?

The point of this patch is to specifically enable split access: some
files for the bootloader are on the default volume, but we need to
access an alternative hierarchy for booting the system. This is a
foundational requirement for boot-to-snapshot, especially in
openSUSE's setup where the boot files are a nested subvolume in the
default subvolume, but the snapshots may be anywhere, including an
alternative hierarchy that isn't rooted in the default volume. In
Fedora, we have no nested volumes, and we do not restrict a user on
how they want their boot-to-snapshot setup to work. A simplified case
of this is that you might want to boot fully encapsulated system roots
which have completely different files and setups.




--
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v1 11/15] grub-install: disable support for EFI platforms

2024-11-03 Thread Neal Gompa
On Thu, Oct 31, 2024 at 3:43 PM Leo Sandoval  wrote:
>
> From: Jan Hlavac 
>
> For each platform, GRUB is shipped as a kernel image and a set of
> modules. These files are then used by the grub-install utility to
> install GRUB on a specific device. However, in order to support UEFI
> Secure Boot, the resulting EFI binary must be signed by a recognized
> private key. For this reason, for EFI platforms, most distributions also
> ship prebuilt EFI binaries signed by a distribution-specific private
> key. In this case, however, the grub-install utility should not be used
> because it would overwrite the signed EFI binary.
>
> The current fix is suboptimal because it preserves all EFI-related code.
> A better solution could be to modularize the code and provide a
> build-time option.
>
> Resolves: rhbz#1737444
>
> Signed-off-by: Jan Hlavac 
> [rharwood: drop man page]
> ---
>  docs/grub.texi  |  7 +++
>  util/grub-install.c | 41 +++--
>  2 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index a225f9a88..bc9791794 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -7080,6 +7080,13 @@ grub-install @var{install_device}
>  The device name @var{install_device} is an OS device name or a GRUB
>  device name.
>
> +In order to support UEFI Secure Boot, the resulting GRUB EFI binary must
> +be signed by a recognized private key. For this reason, for EFI
> +platforms, most distributions also ship prebuilt GRUB EFI binaries
> +signed by a distribution-specific private key. In this case, however,
> +@command{grub2-install} should not be used because it would overwrite
> +the signed EFI binary.
> +
>  @command{grub-install} accepts the following options:
>
>  @table @option
> diff --git a/util/grub-install.c b/util/grub-install.c
> index c245d9359..ee61b042b 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -910,6 +910,25 @@ main (int argc, char *argv[])
>
>platform = grub_install_get_target (grub_install_source_directory);
>
> +  switch (platform)
> +{
> +case GRUB_INSTALL_PLATFORM_ARM_EFI:
> +case GRUB_INSTALL_PLATFORM_ARM64_EFI:
> +case GRUB_INSTALL_PLATFORM_I386_EFI:
> +case GRUB_INSTALL_PLATFORM_IA64_EFI:
> +case GRUB_INSTALL_PLATFORM_LOONGARCH64_EFI:
> +case GRUB_INSTALL_PLATFORM_RISCV32_EFI:
> +case GRUB_INSTALL_PLATFORM_RISCV64_EFI:
> +case GRUB_INSTALL_PLATFORM_X86_64_EFI:
> +  is_efi = 1;
> +  grub_util_error (_("this utility cannot be used for EFI platforms"
> + " because it does not support UEFI Secure Boot"));
> +  break;
> +default:
> +  is_efi = 0;
> +  break;
> +}
> +
>{
>  char *platname = grub_install_get_platform_name (platform);
>  fprintf (stderr, _("Installing for %s platform.\n"), platname);
> @@ -1024,27 +1043,6 @@ main (int argc, char *argv[])
>grub_hostfs_init ();
>grub_host_init ();
>
> -  switch (platform)
> -{
> -case GRUB_INSTALL_PLATFORM_I386_EFI:
> -case GRUB_INSTALL_PLATFORM_X86_64_EFI:
> -case GRUB_INSTALL_PLATFORM_ARM_EFI:
> -case GRUB_INSTALL_PLATFORM_ARM64_EFI:
> -case GRUB_INSTALL_PLATFORM_LOONGARCH64_EFI:
> -case GRUB_INSTALL_PLATFORM_RISCV32_EFI:
> -case GRUB_INSTALL_PLATFORM_RISCV64_EFI:
> -case GRUB_INSTALL_PLATFORM_IA64_EFI:
> -  is_efi = 1;
> -  break;
> -default:
> -  is_efi = 0;
> -  break;
> -
> -  /* pacify warning.  */
> -case GRUB_INSTALL_PLATFORM_MAX:
> -  break;
> -}
> -
>switch (platform)
>  {
>  case GRUB_INSTALL_PLATFORM_I386_IEEE1275:
> @@ -1060,7 +1058,6 @@ main (int argc, char *argv[])
>  }
>
>/* Find the EFI System Partition.  */
> -
>if (is_efi)
>  {
>grub_fs_t fs;
> --
> 2.46.2

This patch should not be upstreamed as-is, since it completely breaks
the ability to generate grub-efi binaries on the system with the
assumption that pre-existing ones have been shipped by a distributor.

The rework mentioned in the commit message body needs to happen here,
because it's unacceptable to just completely break stuff.



-- 
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v1 14/15] grub-install: install on EFI if forced

2024-11-03 Thread Neal Gompa
On Thu, Oct 31, 2024 at 3:43 PM Leo Sandoval  wrote:
>
> From: Marta Lewandowska 
>
> UEFI Secure Boot requires signed grub binaries to work, so grub-
> install should not be used. However, users who have Secure Boot
> disabled and wish to use the command should not be prevented from
> doing so if they invoke --force.
>
> fixes bz#1917213 / bz#2240994
>
> Signed-off-by: Marta Lewandowska 
> ---
>  util/grub-install.c | 38 +++---
>  1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/util/grub-install.c b/util/grub-install.c
> index ee61b042b..b924c2d34 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -910,25 +910,6 @@ main (int argc, char *argv[])
>
>platform = grub_install_get_target (grub_install_source_directory);
>
> -  switch (platform)
> -{
> -case GRUB_INSTALL_PLATFORM_ARM_EFI:
> -case GRUB_INSTALL_PLATFORM_ARM64_EFI:
> -case GRUB_INSTALL_PLATFORM_I386_EFI:
> -case GRUB_INSTALL_PLATFORM_IA64_EFI:
> -case GRUB_INSTALL_PLATFORM_LOONGARCH64_EFI:
> -case GRUB_INSTALL_PLATFORM_RISCV32_EFI:
> -case GRUB_INSTALL_PLATFORM_RISCV64_EFI:
> -case GRUB_INSTALL_PLATFORM_X86_64_EFI:
> -  is_efi = 1;
> -  grub_util_error (_("this utility cannot be used for EFI platforms"
> - " because it does not support UEFI Secure Boot"));
> -  break;
> -default:
> -  is_efi = 0;
> -  break;
> -}
> -
>{
>  char *platname = grub_install_get_platform_name (platform);
>  fprintf (stderr, _("Installing for %s platform.\n"), platname);
> @@ -1045,6 +1026,22 @@ main (int argc, char *argv[])
>
>switch (platform)
>  {
> +case GRUB_INSTALL_PLATFORM_ARM_EFI:
> +case GRUB_INSTALL_PLATFORM_ARM64_EFI:
> +case GRUB_INSTALL_PLATFORM_I386_EFI:
> +case GRUB_INSTALL_PLATFORM_IA64_EFI:
> +case GRUB_INSTALL_PLATFORM_LOONGARCH64_EFI:
> +case GRUB_INSTALL_PLATFORM_RISCV32_EFI:
> +case GRUB_INSTALL_PLATFORM_RISCV64_EFI:
> +case GRUB_INSTALL_PLATFORM_X86_64_EFI:
> +  is_efi = 1;
> +  if (!force)
> +grub_util_error (_("This utility should not be used for EFI 
> platforms"
> +  " because it does not support UEFI Secure Boot."
> +  " If you really wish to proceed, invoke the 
> --force"
> +  " option.\nMake sure Secure Boot is disabled 
> before"
> +  " proceeding"));
> +  break;
>  case GRUB_INSTALL_PLATFORM_I386_IEEE1275:
>  case GRUB_INSTALL_PLATFORM_POWERPC_IEEE1275:
>  #ifdef __linux__
> @@ -1053,6 +1050,9 @@ main (int argc, char *argv[])
>  try_open ("/dev/nvram");
>  #endif
>break;
> +  /* pacify warning.  */
> +case GRUB_INSTALL_PLATFORM_MAX:
> +  break;
>  default:
>break;
>  }
> --
> 2.46.2
>

This should be merged with the patch that breaks grub-install for EFI
and re-sent as one *new* patch. It's not okay to break it and then fix
it in the same patch series, since we don't want broken functionality
in a commit applied to git.



-- 
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v1 0/5] Fedora Rawhide BTRFS patches

2024-11-03 Thread Neal Gompa
On Thu, Oct 31, 2024 at 3:43 PM Leo Sandoval  wrote:
>
> This series contains BTRFS patches taken from Fedora Rawhide [1]. Some of
> these patches have already being sent [2][3] however, as suggested
> by Daniel Kiper, to make the review easier, I took only BTRFS related patches
> from [1][2][3] and create the series.
>
> [1] https://src.fedoraproject.org/rpms/grub2
> [2] https://lists.gnu.org/archive/html/grub-devel/2024-10/msg00145.html
> [3] https://lists.gnu.org/archive/html/grub-devel/2024-10/msg00192.html
>
> Jeff Mahoney (1):
>   btrfs: Add ability to boot from subvolumes
>
> Michael Chang (2):
>   btrfs: export btrfs_subvol and btrfs_subvolid
>   btrfs: Allow specifying btrfs_relative_path to follow the default
> subvolume
>
> Peter Jones (2):
>   btrfs: fix a bad null check
>   btrfs: Fixup for newer compiler
>
>  grub-core/fs/btrfs.c | 629 ---
>  include/grub/btrfs.h |   1 +
>  2 files changed, 595 insertions(+), 35 deletions(-)
>
> --
> 2.46.2
>

Series LGTM.

Reviewed-by: Neal Gompa 




--
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v3 0/3] Add commands to load BLS and UKI files

2025-04-17 Thread Neal Gompa
On Fri, Apr 11, 2025 at 11:55 PM Alec Brown via Grub-devel
 wrote:
>
> v3:
>  - Added --enable-fallback option to check the default directory if the --path
>option isn't able to find entries.
>  - Added the function blsuki_set_find_entry_info() to help set the path and
>device of BLS and UKI entries.
>  - Removed grub_strchr uses in uki_read_osrel().
>  - Converted the static variable "cmd_type" into a parameter and added enums.
>  - Fixed improper handling of the output of
>grub_make_system_path_relative_to_its_root().
>
> This patch set is introducing BootLoaderSpec support to upstream GRUB from
> Fedora GRUB. I've also added a uki command to load Unified Kernel Images since
> it shares similar code to loading BLS config files.
>
> Alec Brown
>
> Alec Brown (1):
>   blsuki: Add uki command to load Unified Kernel Image entries
>
> Peter Jones (1):
>   blsuki: Add blscfg command to parse Boot Loader Specification snippets
>
> Robbie Harwood (1):
>   blsuki: Check for mounted /boot in emu
>

I noticed in your patch set that you haven't done the required "From:"
overrides to maintain authorship when these are applied to git. Could
you fix that up? Also with patches with multiple secondary authors or
if you've done substantial cleanup, could you please add the
appropriate "Co-authored-by:" stanzas?



-- 
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH v3 0/3] Add commands to load BLS and UKI files

2025-04-21 Thread Neal Gompa
On Thu, Apr 17, 2025 at 2:43 PM Daniel Kiper  wrote:
>
> On Thu, Apr 17, 2025 at 07:37:13AM -0400, Neal Gompa wrote:
> > On Fri, Apr 11, 2025 at 11:55 PM Alec Brown via Grub-devel
> >  wrote:
> > >
> > > v3:
> > >  - Added --enable-fallback option to check the default directory if the 
> > > --path
> > >option isn't able to find entries.
> > >  - Added the function blsuki_set_find_entry_info() to help set the path 
> > > and
> > >device of BLS and UKI entries.
> > >  - Removed grub_strchr uses in uki_read_osrel().
> > >  - Converted the static variable "cmd_type" into a parameter and added 
> > > enums.
> > >  - Fixed improper handling of the output of
> > >grub_make_system_path_relative_to_its_root().
> > >
> > > This patch set is introducing BootLoaderSpec support to upstream GRUB from
> > > Fedora GRUB. I've also added a uki command to load Unified Kernel Images 
> > > since
> > > it shares similar code to loading BLS config files.
> > >
> > > Alec Brown
> > >
> > > Alec Brown (1):
> > >   blsuki: Add uki command to load Unified Kernel Image entries
> > >
> > > Peter Jones (1):
> > >   blsuki: Add blscfg command to parse Boot Loader Specification 
> > > snippets
> > >
> > > Robbie Harwood (1):
> > >   blsuki: Check for mounted /boot in emu
> >
> > I noticed in your patch set that you haven't done the required "From:"
> > overrides to maintain authorship when these are applied to git. Could
> > you fix that up? Also with patches with multiple secondary authors or
>
> Yeah, the "From:" thing has to be fixed.
>
> > if you've done substantial cleanup, could you please add the
> > appropriate "Co-authored-by:" stanzas?
>
> We use SOB only now.
>

Why? That makes it difficult to specifically query for patches made by
multiple people. Many tools and Git forges process that tag
*specifically* to be able to give credit to multiple authors.

"Signed-off-by" says absolutely nothing about authorship.




--
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH] Skip tests if required tools are not available

2025-06-14 Thread Neal Gompa
On Mon, Jun 9, 2025 at 1:26 PM Leo Sandoval via Grub-devel
 wrote:
>
> There is no reason to fail a test if the required testing tool is not
> present on the system, so skip the test instead of failing it.
>
> Signed-off-by: Leo Sandoval 

LGTM.

Reviewed-by: Neal Gompa 

-- 
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [RFC Discussion] Proposing an Unstable Repository for GRUB Development

2025-06-14 Thread Neal Gompa
On Tue, May 27, 2025 at 3:57 PM Vladimir 'phcoder' Serbinenko
 wrote:
>
> I appreciate your concern about pace of development and share them. However 
> maintaining what is effectively 2 versions is twice as much effort and would 
> only slow down both versions.
> We are already discussing among maintainers how we can improve the state of 
> affairs but don't have anything ready for announcement.
>

Is there somewhere the rest of us stakeholders can participate in this
discussion? As a developer of image build tools (kiwi, livecd-tools,
etc.) that need to interface with GRUB, I am highly interested in
this.




--
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH] Create a Containerfile with required grub packages for development

2025-06-14 Thread Neal Gompa
On Mon, Jun 9, 2025 at 3:22 PM Leo Sandoval via Grub-devel
 wrote:
>
> Containers bring the ability to have ready-to-use environments and in
> this case a complete Fedora container file is described containing
> all required packages for building and testing grub2.
>
> Once users manually build it, they can run the container jump into a
> setup ready for development. On the other hand, if users
> prefer to use bare metal instead of a containerized environment, it is
> still useful to have a file explicitly indicating the required packages.

This looks great to me.

Reviewed-by: Neal Gompa 


-- 
真実はいつも一つ!/ Always, there's only one truth!

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


Re: [PATCH] Support environment block in btrfs reserved area

2025-07-30 Thread Neal Gompa
On Thu, Jul 10, 2025 at 9:21 PM Michael Chang  wrote:
>
> On Btrfs, GRUB cannot update the environment block (grubenv) because
> file based writes via blocklists are incompatible with Btrfs COW (Copy
> On Write) design. Although GRUB’s filesystem drivers are read only,
> environment updates rely on raw block access to fixed locations, which
> is not safe on Btrfs due to its dynamic block relocation.
>
> To address this, we introduce support for storing the GRUB environment
> block in a fixed block within Btrfs reserved bootloader area, an unused
> region in the device header intended for bootloader use.
>
> This patch adds fs_envblk helpers to grub-editenv for accessing the
> reserved area directly. Variables that require runtime write access
> during boot, such as next_entry, will be mirrored into this external
> block. Other variables will remain stored in the file based grubenv so
> they can keep per snapshot.
>
> The embedding logic is also updated to mark the reserved area as used to
> avoid conflicts with embedded core images.
>
> This enables support for runtime environment updates on Btrfs root
> volumes, allowing tools like grub-reboot to boot an entry once.
>
> Signed-off-by: Michael Chang 
> ---

Looks good to me and seems to work.

Reviewed-by: Neal Gompa 


-- 
真実はいつも一つ!/ Always, there's only one truth!

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