Re: [PATCH v1 22/37] bootp: New net_bootp6 command

[Vladimir 'phcoder' Serbinenko]

Le mar. 8 oct. 2024, 10:23, Michael Chang via Grub-devel 
a écrit :

> On Mon, Oct 07, 2024 at 12:20:55PM GMT, Leo Sandoval wrote:
> > From: Michael Chang 
> >
> > Implement new net_bootp6 command for IPv6 network auto configuration via
> the
> > DHCPv6 protocol (RFC3315).
>
> This would have marked the fourth attempt to upstream the patch by
> different people.
>
> https://lists.gnu.org/archive/html/grub-devel/2016-08/msg0.html
> https://lists.gnu.org/archive/html/grub-devel/2023-01/msg00012.html
> https://www.mail-archive.com/grub-devel@gnu.org/msg30432.html
>
> If everything is still desirable, I could help by reposting the series
> in a separate thread to ease the review,

Please do

> but I’m not really sure because
> it seems to me that upstream isn't interested in the feature.
>
We're. Just our throughput is limited and patches fall through the cracks

>
> Thanks,
> Michael
>
> >
> > Signed-off-by: Michael Chang 
> > Signed-off-by: Ken Lin 
> > [pjones: Put back our code to add a local route]
> > Signed-off-by: Peter Jones 
> > ---
> >  grub-core/net/bootp.c  | 1059 +++-
> >  grub-core/net/drivers/efi/efinet.c |   18 +-
> >  grub-core/net/ip.c |   39 +
> >  include/grub/efi/api.h |2 +-
> >  include/grub/net.h |   91 ++-
> >  5 files changed, 1001 insertions(+), 208 deletions(-)
> >
> > diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
> > index 02d2c2614..e0aec2523 100644
> > --- a/grub-core/net/bootp.c
> > +++ b/grub-core/net/bootp.c
> > @@ -24,6 +24,98 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> > +
> > +static int
> > +dissect_url (const char *url, char **proto, char **host, char **path)
> > +{
> > +  const char *p, *ps;
> > +  grub_size_t l;
> > +
> > +  *proto = *host = *path = NULL;
> > +  ps = p = url;
> > +
> > +  while ((p = grub_strchr (p, ':')))
> > +{
> > +  if (grub_strlen (p) < sizeof ("://") - 1)
> > + break;
> > +  if (grub_memcmp (p, "://", sizeof ("://") - 1) == 0)
> > + {
> > +   l = p - ps;
> > +   *proto = grub_malloc (l + 1);
> > +   if (!*proto)
> > + {
> > +   grub_print_error ();
> > +   return 0;
> > + }
> > +
> > +   grub_memcpy (*proto, ps, l);
> > +   (*proto)[l] = '\0';
> > +   p +=  sizeof ("://") - 1;
> > +   break;
> > + }
> > +  ++p;
> > +}
> > +
> > +  if (!*proto)
> > +{
> > +  grub_dprintf ("bootp", "url: %s is not valid, protocol not
> found\n", url);
> > +  return 0;
> > +}
> > +
> > +  ps = p;
> > +  p = grub_strchr (p, '/');
> > +
> > +  if (!p)
> > +{
> > +  grub_dprintf ("bootp", "url: %s is not valid, host/path not
> found\n", url);
> > +  grub_free (*proto);
> > +  *proto = NULL;
> > +  return 0;
> > +}
> > +
> > +  l = p - ps;
> > +
> > +  if (l > 2 && ps[0] == '[' && ps[l - 1] == ']')
> > +{
> > +  *host = grub_malloc (l - 1);
> > +  if (!*host)
> > + {
> > +   grub_print_error ();
> > +   grub_free (*proto);
> > +   *proto = NULL;
> > +   return 0;
> > + }
> > +  grub_memcpy (*host, ps + 1, l - 2);
> > +  (*host)[l - 2] = 0;
> > +}
> > +  else
> > +{
> > +  *host = grub_malloc (l + 1);
> > +  if (!*host)
> > + {
> > +   grub_print_error ();
> > +   grub_free (*proto);
> > +   *proto = NULL;
> > +   return 0;
> > + }
> > +  grub_memcpy (*host, ps, l);
> > +  (*host)[l] = 0;
> > +}
> > +
> > +  *path = grub_strdup (p);
> > +  if (!*path)
> > +{
> > +  grub_print_error ();
> > +  grub_free (*host);
> > +  grub_free (*proto);
> > +  *host = NULL;
> > +  *proto = NULL;
> > +  return 0;
> > +}
> > +  return 1;
> > +}
> >
> >  struct grub_dhcp_discover_options
> >  {
> > @@ -610,6 +702,584 @@ out:
> >return err;
> >  }
> >
> > +/* The default netbuff size for sending DHCPv6 packets which should be
> > +   large enough to hold the information */
> > +#define GRUB_DHCP6_DEFAULT_NETBUFF_ALLOC_SIZE 512
> > +
> > +struct grub_dhcp6_options
> > +{
> > +  grub_uint8_t *client_duid;
> > +  grub_uint16_t client_duid_len;
> > +  grub_uint8_t *server_duid;
> > +  grub_uint16_t server_duid_len;
> > +  grub_uint32_t iaid;
> > +  grub_uint32_t t1;
> > +  grub_uint32_t t2;
> > +  grub_net_network_level_address_t *ia_addr;
> > +  grub_uint32_t preferred_lifetime;
> > +  grub_uint32_t valid_lifetime;
> > +  grub_net_network_level_address_t *dns_server_addrs;
> > +  grub_uint16_t num_dns_server;
> > +  char *boot_file_proto;
> > +  char *boot_file_server_ip;
> > +  char *boot_file_path;
> > +};
> > +
> > +typedef struct grub_dhcp6_options *grub_dhcp6_options_t;
> > +
> > +struct grub_dhcp6_session
> > +{
> > +  struct grub_dhcp6_session *next;
> > +  struct grub_dhcp6_session **prev;
> > +  grub_uint32_t iaid;
> > +  grub_uint32_t transaction_id:24;
> > +  grub_uint64_t start_time;
> > +

Re: [PATCH v1 31/37] btrfs: grub2-btrfs-04-grub2-install

[Vladimir 'phcoder' Serbinenko]

Le mar. 8 oct. 2024, 09:53, Michael Chang via Grub-devel 
a écrit :

> On Tue, Oct 08, 2024 at 08:07:17AM GMT, Vladimir 'phcoder' Serbinenko
> wrote:
> > Again, what do you try to achieve? Why aren't absolute paths enough?
>
> The absolute path does not align with the default subvolume. As a
> result, after running btrfs set-default , the system does not
> boot the new default subvolume as expected. Instead, it continues to
> boot from a fixed subvolume.
>
Yes, this is the intended behavior. When you change mount points or mount
options they are not propagated either. We can have a command to retrieve
default subvolume:
btrfs_default_subvolume --set=subvol /
linux /$subvol/boot/vmlinuz
This way you can still e.g. do operations with different subvols like:
if cmp /$subvol/grub.cfg /vol2/grub.cfg; then
...
fi

>
> Thanks,
> Michael
>
> >
> > Le lun. 7 oct. 2024, 21:23, Leo Sandoval  a écrit :
> >
> > > From: Michael Chang 
> > >
> > > Signed-off-by: Michael Chang 
> > > Signed-off-by: Robbie Harwood 
> > > ---
> > >  grub-core/osdep/linux/getroot.c |  7 +++
> > >  grub-core/osdep/unix/config.c   | 17 +++--
> > >  include/grub/emu/config.h   |  1 +
> > >  util/config.c   | 10 ++
> > >  util/grub-install.c | 14 ++
> > >  util/grub-mkrelpath.c   |  6 ++
> > >  6 files changed, 53 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/grub-core/osdep/linux/getroot.c
> > > b/grub-core/osdep/linux/getroot.c
> > > index 7dd775d2a..7c29b3523 100644
> > > --- a/grub-core/osdep/linux/getroot.c
> > > +++ b/grub-core/osdep/linux/getroot.c
> > > @@ -373,6 +373,7 @@ get_btrfs_fs_prefix (const char *mount_path)
> > >return NULL;
> > >  }
> > >
> > > +int use_relative_path_on_btrfs = 0;
> > >
> > >  char **
> > >  grub_find_root_devices_from_mountinfo (const char *dir, char
> **relroot)
> > > @@ -516,6 +517,12 @@ again:
> > > {
> > >   ret = grub_find_root_devices_from_btrfs (dir);
> > >   fs_prefix = get_btrfs_fs_prefix (entries[i].enc_path);
> > > + if (use_relative_path_on_btrfs)
> > > +   {
> > > + if (fs_prefix)
> > > +   free (fs_prefix);
> > > + fs_prefix = xstrdup ("/");
> > > +   }
> > > }
> > >else if (!retry && grub_strcmp (entries[i].fstype, "autofs") ==
> 0)
> > > {
> > > diff --git a/grub-core/osdep/unix/config.c
> b/grub-core/osdep/unix/config.c
> > > index 0b1f7618d..0ce0e309a 100644
> > > --- a/grub-core/osdep/unix/config.c
> > > +++ b/grub-core/osdep/unix/config.c
> > > @@ -82,6 +82,19 @@ grub_util_load_config (struct grub_util_config *cfg)
> > >if (v)
> > >  cfg->grub_distributor = xstrdup (v);
> > >
> > > +  v = getenv ("SUSE_BTRFS_SNAPSHOT_BOOTING");
> > > +  if (v)
> > > +{
> > > +  if (grub_strncmp(v, "true", sizeof ("true") - 1) == 0)
> > > +{
> > > +  cfg->is_suse_btrfs_snapshot_enabled = 1;
> > > +}
> > > +  else
> > > +{
> > > +  cfg->is_suse_btrfs_snapshot_enabled = 0;
> > > +}
> > > +}
> > > +
> > >cfgfile = grub_util_get_config_filename ();
> > >if (!grub_util_is_regular (cfgfile))
> > >  return;
> > > @@ -105,8 +118,8 @@ grub_util_load_config (struct grub_util_config
> *cfg)
> > >*ptr++ = *iptr;
> > >  }
> > >
> > > -  strcpy (ptr, "'; printf
> > > \"GRUB_ENABLE_CRYPTODISK=%s\\nGRUB_DISTRIBUTOR=%s\\n\" "
> > > - "\"$GRUB_ENABLE_CRYPTODISK\" \"$GRUB_DISTRIBUTOR\"");
> > > +  strcpy (ptr, "'; printf
> > >
> \"GRUB_ENABLE_CRYPTODISK=%s\\nGRUB_DISTRIBUTOR=%s\\nSUSE_BTRFS_SNAPSHOT_BOOTING=%s\\n\"
> > > "
> > > + "\"$GRUB_ENABLE_CRYPTODISK\" \"$GRUB_DISTRIBUTOR\"
> > > \"$SUSE_BTRFS_SNAPSHOT_BOOTING\"");
> > >
> > >argv[2] = script;
> > >argv[3] = '\0';
> > > diff --git a/include/grub/emu/config.h b/include/grub/emu/config.h
> > > index 875d5896c..c9a7e5f4a 100644
> > 

Re: [PATCH v2 03/17] ieee1275: Disable GRUB video support for IBM power machines

[Vladimir 'phcoder' Serbinenko]

Can you add a short comment in cmain why it's needed? It should mention
32-bit address truncation. The story in the commit message, on the other
hand, is way too long. It needs to be summarized with a link to full story.

Le lun. 7 oct. 2024, 21:19, Leo Sandoval  a écrit :

> From: Paulo Flabiano Smorigo 
>
> Disable GRUB Video Support from IBM Power Machines. This patch fixes what
> is describe above and should fix
> https://bugzilla.redhat.com/show_bug.cgi?id=973205.
> C&P  the bz
> problem's description:
>
> Grub crashes if it tries to run in video mode.
>
> I'm currently testing with grub rpm from f18 in a lpar inside a Power 7R2
> machine using a matrox video card attached.
>
> To reproduce you need to redirect the console to the video output.
>
> This will happens:
>
> IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM
> IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM
> IBM IBM IBM IBM IBM IBM STARTING SOFTWARE   IBM IBM IBM IBM IBM IBM
> IBM IBM IBM IBM IBM IBMPLEASE WAIT...   IBM IBM IBM IBM IBM IBM
> IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM
> IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM
> IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM
> IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM
> IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM
> IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM
> IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM
> IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM
> IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM
> IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM
> IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM
> IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM IBM
>
> Welcome to GRUB
>
> DEFAULT CATCH!, exception-handler=fff00300
> at   %SRR0: 0020ac80   %SRR1: 3002
> Open Firmware exception handler entered from non-OF code
>
> Client's Fix Pt Regs:
>  00 00780c38 01adfc40  1800
>  04 00780850 0004afff 001d2bb0 
>  08 1800 fffb5000 1804b000 01adfc50
>  0c 0008  00800050 0266
>  10 0265 0270 026e 0267
>  14 0263 0001 001a 0002
>  18 0002  00780dce 8080
>  1c 0067 001d3de0 001d2b30 0002
> Special Regs:
> %IV: 0300 %CR: 22002084%XER: 2000  %DSISR: 4200
>   %SRR0: 0020ac80   %SRR1: 3002
> %LR: 6584%CTR: 0004b000
>%DAR: 1804afff
> Virtual PID = 0
>  ok
> 0 >
>
> In Power, GRUB has video support and works fine with PowerMAC.
>
> Theorically it should works with IBM machines as well but, using a Matrox
> videocard and grub.cfg with the video option enabled, GRUB crashes as we
> saw above.
>
> Using GRUB debug we saw that the crash happens because GRUB tries to
> access the video card address:
>
> video/ieee1275.c:236: IEEE1275: keeping current mode 640x480
> video/ieee1275.c:266: IEEE1275: initialising FB @ 0x1800 640x480x8
>
> GRUB gets the address from the display card node from openfirmware. The
> property is called "address":
>
> 0 > dev /pci@800200d/pci@0/display@0  ok
> 0 > .properties
> ibm,loc-codeU78AB.001.WZSHS9P-P1-C7-T1
> vendor-id   102b
> device-id   2527
> revision-id 0001
> class-code  0003
> interrupts  0001
> min-grant   0010
> max-latency 0020
> subsystem-vendor-id 102b
> subsystem-id2300
> devsel-speed0001
> fast-back-to-back
> built-in
> namedisplay
> compatible  MTRX,G550
> pci102b,2527
> pciclass,03
> display
> reg 0099     
> 02990030     0001
> 02990010     0400
> 02990014     0080
> 02990018     0080
> fcode-rom-offset9000
> device_type display
> character-set   ISO8859-1
> iso6429-1983-colors
> power-consumption     007270e0 007270e0 
>  007b98a0 0

Re: [PATCH v2 11/17] 00_header.in: Enable pager by default. (#985860)

[Vladimir 'phcoder' Serbinenko]

This has a danger of rendering headless systems unbootable while waiting
for user input

Le lun. 7 oct. 2024, 21:19, Leo Sandoval  a écrit :

> From: Peter Jones 
>
> Signed-off-by: Peter Jones 
> ---
>  util/grub.d/00_header.in | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/util/grub.d/00_header.in b/util/grub.d/00_header.in
> index 6a316a5ba..c2d8b0937 100644
> --- a/util/grub.d/00_header.in
> +++ b/util/grub.d/00_header.in
> @@ -43,6 +43,8 @@ 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
> +set pager=1
> +
>  if [ -s \$prefix/grubenv ]; then
>load_env
>  fi
> --
> 2.46.2
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v1 24/37] backtrace: Make grub_fatal() also backtrace

[Vladimir 'phcoder' Serbinenko]

This increases the core size on i386-pc needlessly. And will increase even
more when we improve backtrace command. This needs to be a hook and loaded
together with normal. Also it needs to be cpu-agnostic

Le lun. 7 oct. 2024, 21:22, Leo Sandoval  a écrit :

> From: Peter Jones 
>
> ---
>  grub-core/Makefile.core.def |  3 ++
>  grub-core/kern/misc.c   |  6 
>  grub-core/lib/arm64/backtrace.c | 62 +
>  grub-core/lib/backtrace.c   |  2 ++
>  grub-core/lib/i386/backtrace.c  | 14 +++-
>  5 files changed, 86 insertions(+), 1 deletion(-)
>  create mode 100644 grub-core/lib/arm64/backtrace.c
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 0bffbfea9..17b581220 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -199,6 +199,9 @@ kernel = {
>
>softdiv = lib/division.c;
>
> +  x86 = lib/i386/backtrace.c;
> +  x86 = lib/backtrace.c;
> +
>i386 = kern/i386/dl.c;
>i386_xen = kern/i386/dl.c;
>i386_xen_pvh = kern/i386/dl.c;
> diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
> index 11037dc02..465a8e74e 100644
> --- a/grub-core/kern/misc.c
> +++ b/grub-core/kern/misc.c
> @@ -26,6 +26,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  union printf_arg
>  {
> @@ -1301,6 +1302,11 @@ grub_printf_fmt_check (const char *fmt, const char
> *fmt_expected)
>  void __attribute__ ((noreturn))
>  grub_abort (void)
>  {
> +#ifndef GRUB_UTIL
> +#if defined(__i386__) || defined(__x86_64__)
> +  grub_backtrace();
> +#endif
> +#endif
>grub_printf ("\nAborted.");
>
>  #ifndef GRUB_UTIL
> diff --git a/grub-core/lib/arm64/backtrace.c
> b/grub-core/lib/arm64/backtrace.c
> new file mode 100644
> index 0..1079b5380
> --- /dev/null
> +++ b/grub-core/lib/arm64/backtrace.c
> @@ -0,0 +1,62 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2009  Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define MAX_STACK_FRAME 102400
> +
> +void
> +grub_backtrace_pointer (int frame)
> +{
> +  while (1)
> +{
> +  void *lp = __builtin_return_address (frame);
> +  if (!lp)
> +   break;
> +
> +  lp = __builtin_extract_return_addr (lp);
> +
> +  grub_printf ("%p: ", lp);
> +  grub_backtrace_print_address (lp);
> +  grub_printf (" (");
> +  for (i = 0; i < 2; i++)
> +   grub_printf ("%p,", ((void **)ptr) [i + 2]);
> +  grub_printf ("%p)\n", ((void **)ptr) [i + 2]);
> +  nptr = *(void **)ptr;
> +  if (nptr < ptr || (void **) nptr - (void **) ptr > MAX_STACK_FRAME
> + || nptr == ptr)
> +   {
> + grub_printf ("Invalid stack frame at %p (%p)\n", ptr, nptr);
> + break;
> +   }
> +  ptr = nptr;
> +}
> +}
> +
> +void
> +grub_backtrace (void)
> +{
> +  grub_backtrace_pointer (1);
> +}
> +
> diff --git a/grub-core/lib/backtrace.c b/grub-core/lib/backtrace.c
> index 825a8800e..c0ad6ab8b 100644
> --- a/grub-core/lib/backtrace.c
> +++ b/grub-core/lib/backtrace.c
> @@ -29,6 +29,7 @@ GRUB_MOD_LICENSE ("GPLv3+");
>  void
>  grub_backtrace_print_address (void *addr)
>  {
> +#ifndef GRUB_UTIL
>grub_dl_t mod;
>
>FOR_DL_MODULES (mod)
> @@ -44,6 +45,7 @@ grub_backtrace_print_address (void *addr)
> }
>}
>
> +#endif
>grub_printf ("%p", addr);
>  }
>
> diff --git a/grub-core/lib/i386/backtrace.c
> b/grub-core/lib/i386/backtrace.c
> index c3e03c727..c67273db3 100644
> --- a/grub-core/lib/i386/backtrace.c
> +++ b/grub-core/lib/i386/backtrace.c
> @@ -15,11 +15,23 @@
>   *  You should have received a copy of the GNU General Public License
>   *  along with GRUB.  If not, see .
>   */
> +#include 
> +#ifdef GRUB_UTIL
> +#define REALLY_GRUB_UTIL GRUB_UTIL
> +#undef GRUB_UTIL
> +#endif
> +
> +#include 
> +#include 
> +
> +#ifdef REALLY_GRUB_UTIL
> +#define GRUB_UTIL REALLY_GRUB_UTIL
> +#undef REALLY_GRUB_UTIL
> +#endif
>
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> --
> 2.46.2
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
__

Re: [PATCH v1 28/37] btrfs: fix a bad null check

[Vladimir 'phcoder' Serbinenko]

LGTM
Reviewed-by: Vladimir Serbinenko phco...@gmail.com


Le lun. 7 oct. 2024, 21:23, Leo Sandoval  a écrit :

> 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
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v1 31/37] btrfs: grub2-btrfs-04-grub2-install

[Vladimir 'phcoder' Serbinenko]

Again, what do you try to achieve? Why aren't absolute paths enough?

Le lun. 7 oct. 2024, 21:23, Leo Sandoval  a écrit :

> From: Michael Chang 
>
> Signed-off-by: Michael Chang 
> Signed-off-by: Robbie Harwood 
> ---
>  grub-core/osdep/linux/getroot.c |  7 +++
>  grub-core/osdep/unix/config.c   | 17 +++--
>  include/grub/emu/config.h   |  1 +
>  util/config.c   | 10 ++
>  util/grub-install.c | 14 ++
>  util/grub-mkrelpath.c   |  6 ++
>  6 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/grub-core/osdep/linux/getroot.c
> b/grub-core/osdep/linux/getroot.c
> index 7dd775d2a..7c29b3523 100644
> --- a/grub-core/osdep/linux/getroot.c
> +++ b/grub-core/osdep/linux/getroot.c
> @@ -373,6 +373,7 @@ get_btrfs_fs_prefix (const char *mount_path)
>return NULL;
>  }
>
> +int use_relative_path_on_btrfs = 0;
>
>  char **
>  grub_find_root_devices_from_mountinfo (const char *dir, char **relroot)
> @@ -516,6 +517,12 @@ again:
> {
>   ret = grub_find_root_devices_from_btrfs (dir);
>   fs_prefix = get_btrfs_fs_prefix (entries[i].enc_path);
> + if (use_relative_path_on_btrfs)
> +   {
> + if (fs_prefix)
> +   free (fs_prefix);
> + fs_prefix = xstrdup ("/");
> +   }
> }
>else if (!retry && grub_strcmp (entries[i].fstype, "autofs") == 0)
> {
> diff --git a/grub-core/osdep/unix/config.c b/grub-core/osdep/unix/config.c
> index 0b1f7618d..0ce0e309a 100644
> --- a/grub-core/osdep/unix/config.c
> +++ b/grub-core/osdep/unix/config.c
> @@ -82,6 +82,19 @@ grub_util_load_config (struct grub_util_config *cfg)
>if (v)
>  cfg->grub_distributor = xstrdup (v);
>
> +  v = getenv ("SUSE_BTRFS_SNAPSHOT_BOOTING");
> +  if (v)
> +{
> +  if (grub_strncmp(v, "true", sizeof ("true") - 1) == 0)
> +{
> +  cfg->is_suse_btrfs_snapshot_enabled = 1;
> +}
> +  else
> +{
> +  cfg->is_suse_btrfs_snapshot_enabled = 0;
> +}
> +}
> +
>cfgfile = grub_util_get_config_filename ();
>if (!grub_util_is_regular (cfgfile))
>  return;
> @@ -105,8 +118,8 @@ grub_util_load_config (struct grub_util_config *cfg)
>*ptr++ = *iptr;
>  }
>
> -  strcpy (ptr, "'; printf
> \"GRUB_ENABLE_CRYPTODISK=%s\\nGRUB_DISTRIBUTOR=%s\\n\" "
> - "\"$GRUB_ENABLE_CRYPTODISK\" \"$GRUB_DISTRIBUTOR\"");
> +  strcpy (ptr, "'; printf
> \"GRUB_ENABLE_CRYPTODISK=%s\\nGRUB_DISTRIBUTOR=%s\\nSUSE_BTRFS_SNAPSHOT_BOOTING=%s\\n\"
> "
> + "\"$GRUB_ENABLE_CRYPTODISK\" \"$GRUB_DISTRIBUTOR\"
> \"$SUSE_BTRFS_SNAPSHOT_BOOTING\"");
>
>argv[2] = script;
>argv[3] = '\0';
> diff --git a/include/grub/emu/config.h b/include/grub/emu/config.h
> index 875d5896c..c9a7e5f4a 100644
> --- a/include/grub/emu/config.h
> +++ b/include/grub/emu/config.h
> @@ -37,6 +37,7 @@ struct grub_util_config
>  {
>int is_cryptodisk_enabled;
>char *grub_distributor;
> +  int is_suse_btrfs_snapshot_enabled;
>  };
>
>  void
> diff --git a/util/config.c b/util/config.c
> index ebcdd8f5e..f044a880a 100644
> --- a/util/config.c
> +++ b/util/config.c
> @@ -42,6 +42,16 @@ grub_util_parse_config (FILE *f, struct
> grub_util_config *cfg, int simple)
> cfg->is_cryptodisk_enabled = 1;
>   continue;
> }
> +  if (grub_strncmp (ptr, "SUSE_BTRFS_SNAPSHOT_BOOTING=",
> +   sizeof ("SUSE_BTRFS_SNAPSHOT_BOOTING=") - 1) == 0)
> +   {
> + ptr += sizeof ("SUSE_BTRFS_SNAPSHOT_BOOTING=") - 1;
> + if (*ptr == '"' || *ptr == '\'')
> +   ptr++;
> + if (grub_strncmp(ptr, "true", sizeof ("true") - 1) == 0)
> +   cfg->is_suse_btrfs_snapshot_enabled = 1;
> + continue;
> +   }
>if (grub_strncmp (ptr, "GRUB_DISTRIBUTOR=",
> sizeof ("GRUB_DISTRIBUTOR=") - 1) == 0)
> {
> diff --git a/util/grub-install.c b/util/grub-install.c
> index 7dc5657bb..ec3ed4967 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -843,6 +843,8 @@ try_open (const char *path)
>  }
>  #endif
>
> +extern int use_relative_path_on_btrfs;
> +
>  int
>  main (int argc, char *argv[])
>  {
> @@ -876,6 +878,9 @@ main (int argc, char *argv[])
>
>grub_util_load_config (&config);
>
> +  if (config.is_suse_btrfs_snapshot_enabled)
> +use_relative_path_on_btrfs = 1;
> +
>if (!bootloader_id && config.grub_distributor)
>  {
>char *ptr;
> @@ -1366,6 +1371,15 @@ main (int argc, char *argv[])
>relative_grubdir = xstrdup ("/");
>  }
>
> +  if (config.is_suse_btrfs_snapshot_enabled
> +  && grub_strncmp(grub_fs->name, "btrfs", sizeof ("btrfs") - 1) == 0)
> +{
> +  if (!load_cfg_f)
> +load_cfg_f = grub_util_fopen (load_cfg, "wb");
> +  have_load_cfg = 1;
> +  fprintf (load_cfg_f, "set btrfs_relative_path='y'\n");
> +}
> +
>char *prefix_drive = NULL;
>char *install

Re: [PATCH v1 25/37] grub.texi: Make our info pages say "grub2" where appropriate.

[Vladimir 'phcoder' Serbinenko]

Upstream uses "grub" for these commands. Rename to "grub2" is
distro-specific

Le lun. 7 oct. 2024, 21:23, Leo Sandoval  a écrit :

> From: Peter Jones 
>
> This needs to be hooked up to --program-transform=, but I haven't had
> time.
>
> Signed-off-by: Peter Jones 
> ---
>  docs/grub-dev.texi|   4 +-
>  docs/grub.texi| 359 ++
>  grub-core/kern/misc.c |   2 +-
>  3 files changed, 191 insertions(+), 174 deletions(-)
>
> diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi
> index 1276c5930..04c6678cb 100644
> --- a/docs/grub-dev.texi
> +++ b/docs/grub-dev.texi
> @@ -1,7 +1,7 @@
>  \input texinfo
>  @c -*-texinfo-*-
>  @c %**start of header
> -@setfilename grub-dev.info
> +@setfilename grub2-dev.info
>  @include version-dev.texi
>  @settitle GNU GRUB Developers Manual @value{VERSION}
>  @c Unify all our little indices for now.
> @@ -32,7 +32,7 @@ Invariant Sections.
>
>  @dircategory Kernel
>  @direntry
> -* grub-dev: (grub-dev). The GRand Unified Bootloader Dev
> +* grub2-dev: (grub2-dev). The GRand Unified Bootloader Dev
>  @end direntry
>
>  @setchapternewpage odd
> diff --git a/docs/grub.texi b/docs/grub.texi
> index a225f9a88..cecf14d55 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -1,7 +1,7 @@
>  \input texinfo
>  @c -*-texinfo-*-
>  @c %**start of header
> -@setfilename grub.info
> +@setfilename grub2.info
>  @include version.texi
>  @settitle GNU GRUB Manual @value{VERSION}
>  @c Unify all our little indices for now.
> @@ -32,15 +32,15 @@ Invariant Sections.
>
>  @dircategory Kernel
>  @direntry
> -* GRUB: (grub). The GRand Unified Bootloader
> -* grub-install: (grub)Invoking grub-install.Install GRUB on your drive
> -* grub-mkconfig: (grub)Invoking grub-mkconfig.  Generate GRUB
> configuration
> -* grub-mkpasswd-pbkdf2: (grub)Invoking grub-mkpasswd-pbkdf2.
> -* grub-mkrelpath: (grub)Invoking grub-mkrelpath.
> -* grub-mkrescue: (grub)Invoking grub-mkrescue.  Make a GRUB rescue image
> -* grub-mount: (grub)Invoking grub-mount.Mount a file system using
> GRUB
> -* grub-probe: (grub)Invoking grub-probe.Probe device information
> -* grub-script-check: (grub)Invoking grub-script-check.
> +* GRUB2: (grub2). The GRand Unified Bootloader
> +* grub2-install: (grub2)Invoking grub2-install.Install GRUB on your
> drive
> +* grub2-mkconfig: (grub2)Invoking grub2-mkconfig.  Generate GRUB
> configuration
> +* grub2-mkpasswd-pbkdf2: (grub2)Invoking grub2-mkpasswd-pbkdf2.
> +* grub2-mkrelpath: (grub2)Invoking grub2-mkrelpath.
> +* grub2-mkrescue: (grub2)Invoking grub2-mkrescue.  Make a GRUB rescue
> image
> +* grub2-mount: (grub2)Invoking grub2-mount.Mount a file system
> using GRUB
> +* grub2-probe: (grub2)Invoking grub2-probe.Probe device
> information
> +* grub2-script-check: (grub2)Invoking grub2-script-check.
>  @end direntry
>
>  @setchapternewpage odd
> @@ -223,7 +223,7 @@ surprising.
>
>  @item
>  @file{grub.cfg} is typically automatically generated by
> -@command{grub-mkconfig} (@pxref{Simple configuration}).  This makes it
> +@command{grub2-mkconfig} (@pxref{Simple configuration}).  This makes it
>  easier to handle versioned kernel upgrades.
>
>  @item
> @@ -237,7 +237,7 @@ scripting language: variables, conditionals, and loops
> are available.
>  @item
>  A small amount of persistent storage is available across reboots, using
> the
>  @command{save_env} and @command{load_env} commands in GRUB and the
> -@command{grub-editenv} utility.  This is not available in all
> configurations
> +@command{grub2-editenv} utility.  This is not available in all
> configurations
>  (@pxref{Environment block}).
>
>  @item
> @@ -542,7 +542,7 @@ On OS which have device nodes similar to Unix-like OS
> GRUB tools use the
>  OS name. E.g. for GNU/Linux:
>
>  @example
> -# @kbd{grub-install /dev/sda}
> +# @kbd{grub2-install /dev/sda}
>  @end example
>
>  On AROS we use another syntax. For volumes:
> @@ -565,7 +565,7 @@ For disks we use syntax:
>  E.g.
>
>  @example
> -# @kbd{grub-install //:ata.device/0/0}
> +# @kbd{grub2-install //:ata.device/0/0}
>  @end example
>
>  On Windows we use UNC path. For volumes it's typically
> @@ -592,7 +592,7 @@ For disks it's
>  E.g.
>
>  @example
> -# @kbd{grub-install \\?\PhysicalDrive0}
> +# @kbd{grub2-install \\?\PhysicalDrive0}
>  @end example
>
>  Beware that you may need to further escape the backslashes depending on
> your
> @@ -602,7 +602,7 @@ When compiled with cygwin support then cygwin drive
> names are automatically
>  when needed. E.g.
>
>  @example
> -# @kbd{grub-install /dev/sda}
> +# @kbd{grub2-install /dev/sda}
>  @end example
>
>  @node Installation
> @@ -615,7 +615,7 @@ from the source tarball, or as a package for your OS.
>
>  After you have done that, you need to install the boot loader on a
>  drive (floppy or hard disk) by using the utility
> -@command{grub-install} (@pxref{Invoking grub-install}) on a UNIX

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

[Vladimir 'phcoder' Serbinenko]

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.

Le lun. 7 oct. 2024, 21:22, Leo Sandoval  a écrit :

> From: Michael Chang 
>
> Signed-off-by: Michael Chang 
> Signed-off-by: Robbie Harwood 
> ---
>  grub-core/fs/btrfs.c| 195 +++-
>  grub-core/osdep/linux/getroot.c | 148 +++-
>  include/grub/emu/getroot.h  |   5 +
>  util/grub-install.c |  49 
>  4 files changed, 392 insertions(+), 5 deletions(-)
>
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index d47f9ab03..d44a1c73b 100644
> --- a/grub-core/fs/btrfs.c
> +++ b/grub-core/fs/btrfs.c
> @@ -41,6 +41,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -266,6 +267,12 @@ static grub_err_t
>  grub_btrfs_read_logical (struct grub_btrfs_data *data,
>  grub_disk_addr_t addr, void *buf, grub_size_t
> size,
>  int recursion_depth);
> +static grub_err_t
> +get_root (struct grub_btrfs_data *data, struct grub_btrfs_key *key,
> + grub_uint64_t *tree, grub_uint8_t *type);
> +
> +grub_uint64_t
> +find_mtab_subvol_tree (const char *path, char **path_in_subvol);
>
>  static grub_err_t
>  read_sblock (grub_disk_t disk, struct grub_btrfs_superblock *sb)
> @@ -1302,9 +1309,26 @@ lookup_root_by_name(struct grub_btrfs_data *data,
> const char *path)
>grub_err_t err;
>grub_uint64_t tree = 0;
>grub_uint8_t type;
> +  grub_uint64_t saved_tree;
>struct grub_btrfs_key key;
>
> +  if (path[0] == '\0')
> +{
> +  data->fs_tree = 0;
> +  return GRUB_ERR_NONE;
> +}
> +
> +  err = get_root (data, &key, &tree, &type);
> +  if (err)
> +return err;
> +
> +  saved_tree = data->fs_tree;
> +  data->fs_tree = tree;
> +
>err = find_path (data, path, &key, &tree, &type);
> +
> +  data->fs_tree = saved_tree;
> +
>if (err)
>return grub_error(GRUB_ERR_FILE_NOT_FOUND, "couldn't locate %s\n",
> path);
>
> @@ -2316,11 +2340,20 @@ grub_btrfs_dir (grub_device_t device, const char
> *path,
>grub_uint64_t tree;
>grub_uint8_t type;
>grub_size_t est_size = 0;
> +  char *new_path = NULL;
>
>if (!data)
>  return grub_errno;
>
> -  err = find_path (data, path, &key_in, &tree, &type);
> +  tree = find_mtab_subvol_tree (path, &new_path);
> +
> +  if (tree)
> +data->fs_tree = tree;
> +
> +  err = find_path (data, new_path ? new_path : path, &key_in, &tree,
> &type);
> +  if (new_path)
> +grub_free (new_path);
> +
>if (err)
>  {
>grub_btrfs_unmount (data);
> @@ -2447,11 +2480,21 @@ grub_btrfs_open (struct grub_file *file, const
> char *name)
>struct grub_btrfs_inode inode;
>grub_uint8_t type;
>struct grub_btrfs_key key_in;
> +  grub_uint64_t tree;
> +  char *new_path = NULL;
>
>if (!data)
>  return grub_errno;
>
> -  err = find_path (data, name, &key_in, &data->tree, &type);
> +  tree = find_mtab_subvol_tree (name, &new_path);
> +
> +  if (tree)
> +data->fs_tree = tree;
> +
> +  err = find_path (data, new_path ? new_path : name, &key_in,
> &data->tree, &type);
> +  if (new_path)
> +grub_free (new_path);
> +
>if (err)
>  {
>grub_btrfs_unmount (data);
> @@ -2686,6 +2729,150 @@ grub_cmd_btrfs_info (grub_command_t cmd
> __attribute__ ((unused)), int argc,
>return 0;
>  }
>
> +struct grub_btrfs_mtab
> +{
> +  struct grub_btrfs_mtab *next;
> +  struct grub_btrfs_mtab **prev;
> +  char *path;
> +  char *subvol;
> +  grub_uint64_t tree;
> +};
> +
> +typedef struct grub_btrfs_mtab* grub_btrfs_mtab_t;
> +
> +static struct grub_btrfs_mtab *btrfs_mtab;
> +
> +#define FOR_GRUB_MTAB(var) FOR_LIST_ELEMENTS (var, btrfs_mtab)
> +#define FOR_GRUB_MTAB_SAFE(var, next) FOR_LIST_ELEMENTS_SAFE((var),
> (next), btrfs_mtab)
> +
> +static void
> +add_mountpoint (const char *path, const char *subvol, grub_uint64_t tree)
> +{
> +  grub_btrfs_mtab_t m = grub_malloc (sizeof (*m));
> +
> +  m->path = grub_strdup (path);
> +  m->subvol = grub_strdup (subvol);
> +  m->tree = tree;
> +  grub_list_push (GRUB_AS_LIST_P (&btrfs_mtab), GRUB_AS_LIST (m));
> +}
> +
> +static grub_err_t
> +grub_cmd_btrfs_mount_subvol (grub_command_t cmd __attribute__ ((unused)),
> int argc,
> +char **argv)
> +{
> +  char *devname, *dirname, *subvol;
> +  struct grub_btrfs_key key_in;
> +  grub_uint8_t type;
> +  grub_uint64_t tree;
> +  grub_uint64_t saved_tree;
> +  grub_err_t err;
> +  struct grub_btrfs_data *data = NULL;
> +  grub_device_t dev = NULL;
> +
> +  if (argc < 3)
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, "required   and
> ");
> +
> +  devname = grub_file_get_device_name(argv[0]);
> +  dev = grub_device_open (devname);
> +  grub_free (devname);
> +
> +  if (!dev)
> +{
> +  err = grub_errno;
> +  goto err_out;
> +}
> +
> +  dirname 

Re: [PATCH 08/20] 20_ppc_terminfo.in: Migrate PPC from Yaboot to Grub2

[Vladimir 'phcoder' Serbinenko]

Looking at the patch: it's specific to ieee1275 not PPC. The reason is that
it operates on ofconsole. But then it means that the patch description
doesn't really match the intent. And neither does the name of the variable
as it's specific to ofconsole

Le mer. 2 oct. 2024, 20:12, Leo Sandoval  a écrit :

>
>
> On Mon, Sep 30, 2024 at 12:57 PM Vladimir 'phcoder' Serbinenko <
> phco...@gmail.com> wrote:
>
>> Why is it ppc-specific?
>>
>
> IBM wrote this patch so this is why this is ppc specific, but you are
> right, it can apply to all archs. I will send a v2.
>
>
>
>>
>> Le lun. 30 sept. 2024, 20:49, Leo Sandoval  a
>> écrit :
>>
>>> From: Mark Hamzy 
>>>
>>> Add configuration support for serial terminal consoles.  This will set
>>> the maximum screen size so that text is not overwritten.
>>>
>>> Signed-off-by: Mark Hamzy 
>>> Signed-off-by: Robbie Harwood 
>>> ---
>>>  Makefile.util.def  |   7 ++
>>>  util/grub.d/20_ppc_terminfo.in | 114 +
>>>  2 files changed, 121 insertions(+)
>>>  create mode 100644 util/grub.d/20_ppc_terminfo.in
>>>
>>> diff --git a/Makefile.util.def b/Makefile.util.def
>>> index 9432365a9..09bfcadd9 100644
>>> --- a/Makefile.util.def
>>> +++ b/Makefile.util.def
>>> @@ -517,6 +517,13 @@ script = {
>>>installdir = grubconf;
>>>  };
>>>
>>> +script = {
>>> +  name = '20_ppc_terminfo';
>>> +  common = util/grub.d/20_ppc_terminfo.in;
>>> +  installdir = grubconf;
>>> +  condition = COND_HOST_LINUX;
>>> +};
>>> +
>>>  script = {
>>>name = '30_os-prober';
>>>common = util/grub.d/30_os-prober.in;
>>> diff --git a/util/grub.d/20_ppc_terminfo.in b/util/grub.d/
>>> 20_ppc_terminfo.in
>>> new file mode 100644
>>> index 0..10d665868
>>> --- /dev/null
>>> +++ b/util/grub.d/20_ppc_terminfo.in
>>> @@ -0,0 +1,114 @@
>>> +#! /bin/sh
>>> +set -e
>>> +
>>> +# grub-mkconfig helper script.
>>> +# Copyright (C) 2006,2007,2008,2009,2010  Free Software Foundation, Inc.
>>> +#
>>> +# GRUB is free software: you can redistribute it and/or modify
>>> +# it under the terms of the GNU General Public License as published by
>>> +# the Free Software Foundation, either version 3 of the License, or
>>> +# (at your option) any later version.
>>> +#
>>> +# GRUB is distributed in the hope that it will be useful,
>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +# GNU General Public License for more details.
>>> +#
>>> +# You should have received a copy of the GNU General Public License
>>> +# along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
>>> +
>>> +prefix=@prefix@
>>> +exec_prefix=@exec_prefix@
>>> +bindir=@bindir@
>>> +libdir=@libdir@
>>> +. "@datadir@/@PACKAGE@/grub-mkconfig_lib"
>>> +
>>> +export TEXTDOMAIN=@PACKAGE@
>>> +export TEXTDOMAINDIR=@localedir@
>>> +
>>> +X=80
>>> +Y=24
>>> +TERMINAL=ofconsole
>>> +
>>> +argument () {
>>> +  opt=$1
>>> +  shift
>>> +
>>> +  if test $# -eq 0; then
>>> +  echo "$0: option requires an argument -- '$opt'" 1>&2
>>> +  exit 1
>>> +  fi
>>> +  echo $1
>>> +}
>>> +
>>> +check_terminfo () {
>>> +
>>> +  while test $# -gt 0
>>> +  do
>>> +option=$1
>>> +shift
>>> +
>>> +case "$option" in
>>> +terminfo | TERMINFO)
>>> +;;
>>> +
>>> +-g)
>>> +NEWXY=`argument $option "$@"`
>>> +NEWX=`echo $NEWXY | cut -d x -f 1`
>>> +NEWY=`echo $NEWXY | cut -d x -f 2`
>>> +
>>> +if [ ${NEWX} -ge 80 ] ; then
>>> +  X=${NEWX}
>>> +else
>>> +  echo "Warning: ${NEWX} is less than the minimum size of 80"
>>> +fi
>>> +
>>> +if [ ${NEWY} -ge 24 ] ; then
>>> +  Y=${NEWY}
>>> +else
>>> +  echo "Warning: ${NEWY} is less than the minimum size of 24"
>&

Re: [PATCH 14/20] 10_linux.in 20_linux_xen.in: Don't say "GNU/Linux" in generated menus.

[Vladimir 'phcoder' Serbinenko]

Official stand of FSF and GNU project is that the correct name is
GNU/Linux. The only case where this could be justified is for non-GNU
variants like BusyBox distros or Android

Le lun. 30 sept. 2024, 20:48, Leo Sandoval  a écrit :

> From: Peter Jones 
>
> [rharwood: say it even less]
> ---
>  grub-core/normal/main.c | 2 +-
>  tests/util/grub-shell-tester.in | 2 +-
>  tests/util/grub-shell.in| 2 +-
>  util/grub.d/10_linux.in | 4 ++--
>  util/grub.d/20_linux_xen.in | 4 ++--
>  5 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/grub-core/normal/main.c b/grub-core/normal/main.c
> index 1317279c0..568c2adfa 100644
> --- a/grub-core/normal/main.c
> +++ b/grub-core/normal/main.c
> @@ -218,7 +218,7 @@ grub_normal_init_page (struct grub_term_output *term,
>
>grub_term_cls (term);
>
> -  msg_formatted = grub_xasprintf (_("GNU GRUB  version %s"),
> PACKAGE_VERSION);
> +  msg_formatted = grub_xasprintf (_("GRUB version %s"), PACKAGE_VERSION);
>if (!msg_formatted)
>  return;
>
> diff --git a/tests/util/grub-shell-tester.in b/tests/util/
> grub-shell-tester.in
> index 8a87109b1..9a4319d4f 100644
> --- a/tests/util/grub-shell-tester.in
> +++ b/tests/util/grub-shell-tester.in
> @@ -56,7 +56,7 @@ for option in "$@"; do
> usage
> exit 0 ;;
>  -v | --version)
> -   echo "$0 (GNU GRUB ${PACKAGE_VERSION})"
> +   echo "$0 (GRUB ${PACKAGE_VERSION})"
> exit 0 ;;
>  --modules=*)
> ms=`echo "$option" | sed -e 's/--modules=//'`
> diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
> index 496e1bab3..e0570c88e 100644
> --- a/tests/util/grub-shell.in
> +++ b/tests/util/grub-shell.in
> @@ -243,7 +243,7 @@ for option in "$@"; do
> usage
> exit 0 ;;
>  -v | --version)
> -   echo "$0 (GNU GRUB ${PACKAGE_VERSION})"
> +   echo "$0 (GRUB ${PACKAGE_VERSION})"
> exit 0 ;;
>  --trim)
> trim=1 ;;
> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
> index 00d4b220c..901745707 100644
> --- a/util/grub.d/10_linux.in
> +++ b/util/grub.d/10_linux.in
> @@ -29,9 +29,9 @@ export TEXTDOMAINDIR="@localedir@"
>  CLASS="--class gnu-linux --class gnu --class os"
>
>  if [ "x${GRUB_DISTRIBUTOR}" = "x" ] ; then
> -  OS=GNU/Linux
> +  OS="$(sed 's, release .*$,,g' /etc/system-release)"
>  else
> -  OS="${GRUB_DISTRIBUTOR} GNU/Linux"
> +  OS="${GRUB_DISTRIBUTOR}"
>CLASS="--class $(echo ${GRUB_DISTRIBUTOR} | tr 'A-Z' 'a-z' | cut -d' '
> -f1|LC_ALL=C sed 's,[^[:alnum:]_],_,g') ${CLASS}"
>  fi
>
> diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
> index 94dd8be13..98ee5bc58 100644
> --- a/util/grub.d/20_linux_xen.in
> +++ b/util/grub.d/20_linux_xen.in
> @@ -29,9 +29,9 @@ export TEXTDOMAINDIR="@localedir@"
>  CLASS="--class gnu-linux --class gnu --class os --class xen"
>
>  if [ "x${GRUB_DISTRIBUTOR}" = "x" ] ; then
> -  OS=GNU/Linux
> +  OS="$(sed 's, release .*$,,g' /etc/system-release)"
>  else
> -  OS="${GRUB_DISTRIBUTOR} GNU/Linux"
> +  OS="${GRUB_DISTRIBUTOR}"
>CLASS="--class $(echo ${GRUB_DISTRIBUTOR} | tr 'A-Z' 'a-z' | cut -d' '
> -f1|LC_ALL=C sed 's,[^[:alnum:]_],_,g') ${CLASS}"
>  fi
>
> --
> 2.46.1
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 05/20] normal/menu: Allow "fallback" to include entries by title, not just number.

[Vladimir 'phcoder' Serbinenko]

Using titles is broken concept and the only reason we support it in default
is backwards compatibility. Maybe it's better to restrict it to just IDs

Le lun. 30 sept. 2024, 20:48, Leo Sandoval  a écrit :

> From: Peter Jones 
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1026084
>
> Signed-off-by: Peter Jones 
> ---
>  grub-core/normal/menu.c | 85 -
>  1 file changed, 58 insertions(+), 27 deletions(-)
>
> diff --git a/grub-core/normal/menu.c b/grub-core/normal/menu.c
> index 6a90e091f..6444ee6f9 100644
> --- a/grub-core/normal/menu.c
> +++ b/grub-core/normal/menu.c
> @@ -163,15 +163,40 @@ grub_menu_set_timeout (int timeout)
>  }
>  }
>
> +static int
> +menuentry_eq (const char *id, const char *spec)
> +{
> +  const char *ptr1, *ptr2;
> +  ptr1 = id;
> +  ptr2 = spec;
> +  while (1)
> +{
> +  if (*ptr2 == '>' && ptr2[1] != '>' && *ptr1 == 0)
> +   return ptr2 - spec;
> +  if (*ptr2 == '>' && ptr2[1] != '>')
> +   return 0;
> +  if (*ptr2 == '>')
> +   ptr2++;
> +  if (*ptr1 != *ptr2)
> +   return 0;
> +  if (*ptr1 == 0)
> +   return ptr1 - id;
> +  ptr1++;
> +  ptr2++;
> +}
> +  return 0;
> +}
> +
>  /* Get the first entry number from the value of the environment variable
> NAME,
> which is a space-separated list of non-negative integers.  The entry
> number
> which is returned is stripped from the value of NAME.  If no entry
> number
> can be found, -1 is returned.  */
>  static int
> -get_and_remove_first_entry_number (const char *name)
> +get_and_remove_first_entry_number (grub_menu_t menu, const char *name)
>  {
>const char *val, *tail;
>int entry;
> +  int sz = 0;
>
>val = grub_env_get (name);
>if (! val)
> @@ -181,9 +206,39 @@ get_and_remove_first_entry_number (const char *name)
>
>entry = (int) grub_strtoul (val, &tail, 0);
>
> +  if (grub_errno == GRUB_ERR_BAD_NUMBER)
> +{
> +  /* See if the variable matches the title of a menu entry.  */
> +  grub_menu_entry_t e = menu->entry_list;
> +  int i;
> +
> +  for (i = 0; e; i++)
> +   {
> + sz = menuentry_eq (e->title, val);
> + if (sz < 1)
> +   sz = menuentry_eq (e->id, val);
> +
> + if (sz >= 1)
> +   {
> + entry = i;
> + break;
> +   }
> + e = e->next;
> +   }
> +
> +  if (sz > 0)
> +   grub_errno = GRUB_ERR_NONE;
> +
> +  if (! e)
> +   entry = -1;
> +}
> +
>if (grub_errno == GRUB_ERR_NONE)
>  {
> -  /* Skip whitespace to find the next digit.  */
> +  if (sz > 0)
> +   tail += sz;
> +
> +  /* Skip whitespace to find the next entry.  */
>while (*tail && grub_isspace (*tail))
> tail++;
>grub_env_set (name, tail);
> @@ -346,7 +401,7 @@ grub_menu_execute_with_fallback (grub_menu_t menu,
>grub_menu_execute_entry (entry, 1);
>
>/* Deal with fallback entries.  */
> -  while ((fallback_entry = get_and_remove_first_entry_number ("fallback"))
> +  while ((fallback_entry = get_and_remove_first_entry_number (menu,
> "fallback"))
>  >= 0)
>  {
>grub_print_error ();
> @@ -464,30 +519,6 @@ grub_menu_register_viewer (struct grub_menu_viewer
> *viewer)
>viewers = viewer;
>  }
>
> -static int
> -menuentry_eq (const char *id, const char *spec)
> -{
> -  const char *ptr1, *ptr2;
> -  ptr1 = id;
> -  ptr2 = spec;
> -  while (1)
> -{
> -  if (*ptr2 == '>' && ptr2[1] != '>' && *ptr1 == 0)
> -   return 1;
> -  if (*ptr2 == '>' && ptr2[1] != '>')
> -   return 0;
> -  if (*ptr2 == '>')
> -   ptr2++;
> -  if (*ptr1 != *ptr2)
> -   return 0;
> -  if (*ptr1 == 0)
> -   return 1;
> -  ptr1++;
> -  ptr2++;
> -}
> -}
> -
> -
>  /* Get the entry number from the variable NAME.  */
>  static int
>  get_entry_number (grub_menu_t menu, const char *name)
> --
> 2.46.1
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 11/20] blscfg: add blscfg module to parse Boot Loader Specification snippets

[Vladimir 'phcoder' Serbinenko]

This is already being reviewed in separate thread

Le lun. 30 sept. 2024, 20:46, Leo Sandoval  a écrit :

> From: Peter Jones 
>
> The BootLoaderSpec (BLS) defines a scheme where different bootloaders can
> share a format for boot items and a configuration directory that accepts
> these common configurations as drop-in files.
>
> Signed-off-by: Peter Jones 
> Signed-off-by: Javier Martinez Canillas 
> [wjt: some cleanups and fixes]
> Signed-off-by: Will Thompson 
> ---
>  grub-core/Makefile.core.def|   11 +
>  grub-core/commands/blscfg.c| 1177 
>  grub-core/commands/legacycfg.c |5 +-
>  grub-core/commands/loadenv.c   |   77 +--
>  grub-core/commands/loadenv.h   |   93 +++
>  grub-core/commands/menuentry.c |   20 +-
>  grub-core/normal/main.c|6 +
>  include/grub/compiler.h|2 +
>  include/grub/menu.h|   13 +
>  include/grub/normal.h  |2 +-
>  10 files changed, 1324 insertions(+), 82 deletions(-)
>  create mode 100644 grub-core/commands/blscfg.c
>  create mode 100644 grub-core/commands/loadenv.h
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 0bffbfea9..47c0fc755 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -842,6 +842,16 @@ module = {
>common = commands/blocklist.c;
>  };
>
> +module = {
> +  name = blscfg;
> +  common = commands/blscfg.c;
> +  common = commands/loadenv.h;
> +  enable = powerpc_ieee1275;
> +  enable = efi;
> +  enable = i386_pc;
> +  enable = emu;
> +};
> +
>  module = {
>name = boot;
>common = commands/boot.c;
> @@ -1009,6 +1019,7 @@ module = {
>  module = {
>name = loadenv;
>common = commands/loadenv.c;
> +  common = commands/loadenv.h;
>common = lib/envblk.c;
>  };
>
> diff --git a/grub-core/commands/blscfg.c b/grub-core/commands/blscfg.c
> new file mode 100644
> index 0..e907a6a5d
> --- /dev/null
> +++ b/grub-core/commands/blscfg.c
> @@ -0,0 +1,1177 @@
> +/*-*- Mode: C; c-basic-offset: 2; indent-tabs-mode: t -*-*/
> +
> +/* bls.c - implementation of the boot loader spec */
> +
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +#include "loadenv.h"
> +
> +#define GRUB_BLS_CONFIG_PATH "/loader/entries/"
> +#ifdef GRUB_MACHINE_EMU
> +#define GRUB_BOOT_DEVICE "/boot"
> +#else
> +#define GRUB_BOOT_DEVICE "($root)"
> +#endif
> +
> +struct keyval
> +{
> +  const char *key;
> +  char *val;
> +};
> +
> +static struct bls_entry *entries = NULL;
> +
> +#define FOR_BLS_ENTRIES(var) FOR_LIST_ELEMENTS (var, entries)
> +
> +static int bls_add_keyval(struct bls_entry *entry, char *key, char *val)
> +{
> +  char *k, *v;
> +  struct keyval **kvs, *kv;
> +  int new_n = entry->nkeyvals + 1;
> +
> +  kvs = grub_realloc (entry->keyvals, new_n * sizeof (struct keyval *));
> +  if (!kvs)
> +return grub_error (GRUB_ERR_OUT_OF_MEMORY,
> +  "couldn't find space for BLS entry");
> +  entry->keyvals = kvs;
> +
> +  kv = grub_malloc (sizeof (struct keyval));
> +  if (!kv)
> +return grub_error (GRUB_ERR_OUT_OF_MEMORY,
> +  "couldn't find space for BLS entry");
> +
> +  k = grub_strdup (key);
> +  if (!k)
> +{
> +  grub_free (kv);
> +  return grub_error (GRUB_ERR_OUT_OF_MEMORY,
> +"couldn't find space for BLS entry");
> +}
> +
> +  v = grub_strdup (val);
> +  if (!v)
> +{
> +  grub_free (k);
> +  grub_free (kv);
> +  return grub_error (GRUB_ERR_OUT_OF_MEMORY,
> +"couldn't find space for BLS entry");
> +}
> +
> +  kv->key = k;
> +  kv->val = v;
> +
> +  entry->keyvals[entry->nkeyvals] = kv;
> +  grub_dprintf("blscfg", "new keyval at %p:%s:%s\n",
> entry->keyvals[entry->nkeyvals], k, v);
> +  entry->nkeyvals = new_n;
> +
> +  return 0;
> +}
> +
> +/* Find they value of the key named by keyname.  If there are allowed to
> be
> + * more than one, pass a pointer to an int set to -1 the first time, and
> pass
> + * the same pointer through each time after, and it'll return them in
> sorted
> + * order as def

Re: [PATCH] LVM cachevol support

[Vladimir 'phcoder' Serbinenko]

You seem to have forgotten the patch

Le lun. 30 sept. 2024, 22:07, Scottie Shore  a
écrit :

> The +suffix format is described as a status flag here:
> https://gitlab.com/lvmteam/lvm2/-/blob/main/lib/format_text/flags.c#L238
>
> I've kept the match simple and in the style of the code around it. There's
> a need for dedicated functions to iterate through the LVM MDA, but that's a
> much larger scope.
>
> I've compiled and tested this change. Before, grub's "ls" does not show my
> LV with cachevol, and grub-probe detects it as an "unknown LVM type".
> After, I'm able to boot with grub's root on an LV with cachevol.
>
> I'd like to acknowledge Patrick Plenefisch, who submitted a similar patch
> with a broader scope, and issues with formatting and unrelated edits.
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 03/20] ieee1275: Disable GRUB video support for IBM power machines

[Vladimir 'phcoder' Serbinenko]

This needs a detailed comment in quirks code as to why we disable video
mode.

Le lun. 30 sept. 2024, 20:47, Leo Sandoval  a écrit :

> From: Paulo Flabiano Smorigo 
>
> Should fix the problem in bugzilla:
> https://bugzilla.redhat.com/show_bug.cgi?id=973205
>
> Signed-off-by: Paulo Flabiano Smorigo 
> Signed-off-by: Robbie Harwood 
> ---
>  grub-core/kern/ieee1275/cmain.c  | 5 -
>  grub-core/video/ieee1275.c   | 9 ++---
>  include/grub/ieee1275/ieee1275.h | 2 ++
>  3 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/grub-core/kern/ieee1275/cmain.c
> b/grub-core/kern/ieee1275/cmain.c
> index e74de3248..810a089a9 100644
> --- a/grub-core/kern/ieee1275/cmain.c
> +++ b/grub-core/kern/ieee1275/cmain.c
> @@ -89,7 +89,10 @@ grub_ieee1275_find_options (void)
>}
>
>if (rc >= 0 && grub_strncmp (tmp, "IBM", 3) == 0)
> -grub_ieee1275_set_flag
> (GRUB_IEEE1275_FLAG_NO_TREE_SCANNING_FOR_DISKS);
> +{
> +  grub_ieee1275_set_flag
> (GRUB_IEEE1275_FLAG_NO_TREE_SCANNING_FOR_DISKS);
> +  grub_ieee1275_set_flag (GRUB_IEEE1275_FLAG_DISABLE_VIDEO_SUPPORT);
> +}
>
>/* Old Macs have no key repeat, newer ones have fully working one.
>   The ones inbetween when repeated key generates an escaoe sequence
> diff --git a/grub-core/video/ieee1275.c b/grub-core/video/ieee1275.c
> index ca3d3c3b2..5592e4bb7 100644
> --- a/grub-core/video/ieee1275.c
> +++ b/grub-core/video/ieee1275.c
> @@ -351,9 +351,12 @@ static struct grub_video_adapter
> grub_video_ieee1275_adapter =
>
>  GRUB_MOD_INIT(ieee1275_fb)
>  {
> -  find_display ();
> -  if (display)
> -grub_video_register (&grub_video_ieee1275_adapter);
> +  if (! grub_ieee1275_test_flag
> (GRUB_IEEE1275_FLAG_DISABLE_VIDEO_SUPPORT))
> +{
> +  find_display ();
> +  if (display)
> +grub_video_register (&grub_video_ieee1275_adapter);
> +}
>  }
>
>  GRUB_MOD_FINI(ieee1275_fb)
> diff --git a/include/grub/ieee1275/ieee1275.h
> b/include/grub/ieee1275/ieee1275.h
> index 4f6e6aaa0..db0ec5f4c 100644
> --- a/include/grub/ieee1275/ieee1275.h
> +++ b/include/grub/ieee1275/ieee1275.h
> @@ -145,6 +145,8 @@ enum grub_ieee1275_flag
>GRUB_IEEE1275_FLAG_POWER_VM,
>
>GRUB_IEEE1275_FLAG_POWER_KVM,
> +
> +  GRUB_IEEE1275_FLAG_DISABLE_VIDEO_SUPPORT
>  };
>
>  extern int EXPORT_FUNC(grub_ieee1275_test_flag) (enum grub_ieee1275_flag
> flag);
> --
> 2.46.1
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 08/20] 20_ppc_terminfo.in: Migrate PPC from Yaboot to Grub2

[Vladimir 'phcoder' Serbinenko]

Why is it ppc-specific?

Le lun. 30 sept. 2024, 20:49, Leo Sandoval  a écrit :

> From: Mark Hamzy 
>
> Add configuration support for serial terminal consoles.  This will set
> the maximum screen size so that text is not overwritten.
>
> Signed-off-by: Mark Hamzy 
> Signed-off-by: Robbie Harwood 
> ---
>  Makefile.util.def  |   7 ++
>  util/grub.d/20_ppc_terminfo.in | 114 +
>  2 files changed, 121 insertions(+)
>  create mode 100644 util/grub.d/20_ppc_terminfo.in
>
> diff --git a/Makefile.util.def b/Makefile.util.def
> index 9432365a9..09bfcadd9 100644
> --- a/Makefile.util.def
> +++ b/Makefile.util.def
> @@ -517,6 +517,13 @@ script = {
>installdir = grubconf;
>  };
>
> +script = {
> +  name = '20_ppc_terminfo';
> +  common = util/grub.d/20_ppc_terminfo.in;
> +  installdir = grubconf;
> +  condition = COND_HOST_LINUX;
> +};
> +
>  script = {
>name = '30_os-prober';
>common = util/grub.d/30_os-prober.in;
> diff --git a/util/grub.d/20_ppc_terminfo.in b/util/grub.d/
> 20_ppc_terminfo.in
> new file mode 100644
> index 0..10d665868
> --- /dev/null
> +++ b/util/grub.d/20_ppc_terminfo.in
> @@ -0,0 +1,114 @@
> +#! /bin/sh
> +set -e
> +
> +# grub-mkconfig helper script.
> +# Copyright (C) 2006,2007,2008,2009,2010  Free Software Foundation, Inc.
> +#
> +# GRUB is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# GRUB is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with GRUB.  If not, see .
> +
> +prefix=@prefix@
> +exec_prefix=@exec_prefix@
> +bindir=@bindir@
> +libdir=@libdir@
> +. "@datadir@/@PACKAGE@/grub-mkconfig_lib"
> +
> +export TEXTDOMAIN=@PACKAGE@
> +export TEXTDOMAINDIR=@localedir@
> +
> +X=80
> +Y=24
> +TERMINAL=ofconsole
> +
> +argument () {
> +  opt=$1
> +  shift
> +
> +  if test $# -eq 0; then
> +  echo "$0: option requires an argument -- '$opt'" 1>&2
> +  exit 1
> +  fi
> +  echo $1
> +}
> +
> +check_terminfo () {
> +
> +  while test $# -gt 0
> +  do
> +option=$1
> +shift
> +
> +case "$option" in
> +terminfo | TERMINFO)
> +;;
> +
> +-g)
> +NEWXY=`argument $option "$@"`
> +NEWX=`echo $NEWXY | cut -d x -f 1`
> +NEWY=`echo $NEWXY | cut -d x -f 2`
> +
> +if [ ${NEWX} -ge 80 ] ; then
> +  X=${NEWX}
> +else
> +  echo "Warning: ${NEWX} is less than the minimum size of 80"
> +fi
> +
> +if [ ${NEWY} -ge 24 ] ; then
> +  Y=${NEWY}
> +else
> +  echo "Warning: ${NEWY} is less than the minimum size of 24"
> +fi
> +
> +shift
> +;;
> +
> +*)
> +#   # accept console or ofconsole
> +#   if [ "$option" != "console" -a "$option" != "ofconsole" ] ; then
> +# echo "Error: GRUB_TERMINFO unknown console: $option"
> +# exit 1
> +#   fi
> +#   # perfer console
> +#   TERMINAL=console
> +# accept ofconsole
> +if [ "$option" != "ofconsole" ] ; then
> +  echo "Error: GRUB_TERMINFO unknown console: $option"
> +  exit 1
> +fi
> +# perfer console
> +TERMINAL=ofconsole
> +;;
> +esac
> +
> +  done
> +
> +}
> +
> +if ! uname -m | grep -q ppc ; then
> +  exit 0
> +fi
> +
> +if [ "x${GRUB_TERMINFO}" != "x" ] ; then
> +  F1=`echo ${GRUB_TERMINFO} | cut -d " " -f 1`
> +
> +  if [ "${F1}" != "terminfo" ] ; then
> +echo "Error: GRUB_TERMINFO is set to \"${GRUB_TERMINFO}\" The first
> word should be terminfo."
> +exit 1
> +  fi
> +
> +  check_terminfo ${GRUB_TERMINFO}
> +fi
> +
> +cat << EOF
> +  terminfo -g ${X}x${Y} ${TERMINAL}
> +EOF
> --
> 2.46.1
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] kern/fs: honour file->read_hook in grub_fs_blocklist_read()

[Vladimir 'phcoder' Serbinenko]

Reviewed-by: phco...@gmail.com

Le jeu. 29 août 2024, 14:07, Rasmus Villemoes via Grub-devel <
grub-devel@gnu.org> a écrit :

> Unlike files accessed via a normal file system, the file->read_hook is
> not honoured when using blocklist notation.
>
> This means that when trying to use a dedicated, 1KiB, raw partition
> for the environment block and hence does something like
>
>   save_env --file=(hd0,gpt9)0+2 X Y Z
>
> this fails with "sparse file not allowed", which is rather unexpected,
> as I've explicitly said exactly which blocks should be used. Adding a
> little debugging reveals that grub_file_size (file) is 1024 as
> expected, but total_length is 0, simply because the callback was never
> invoked, so blocklists is an empty list.
>
> Fix that by honouring the ->read_hook set by the caller, also when a
> "file" is specified with blocklist notation.
>
> Signed-off-by: Rasmus Villemoes 
> ---
> I've tried to keep the patch at a minimum, but since the
> disk->read_hook needs to be cleared on all return paths, it seemed a
> little easier to drop the 'return -1' and instead set ret and break.
>
> For readability, it seemed best to introduce the disk local variable to
> hold file->device->disk, and then there's no point in not also using
> that in the grub_disk_read() call.
>
>  grub-core/kern/fs.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/grub-core/kern/fs.c b/grub-core/kern/fs.c
> index 7ad0aaf4e..79f1a797c 100644
> --- a/grub-core/kern/fs.c
> +++ b/grub-core/kern/fs.c
> @@ -215,12 +215,15 @@ grub_fs_blocklist_read (grub_file_t file, char *buf,
> grub_size_t len)
>grub_disk_addr_t sector;
>grub_off_t offset;
>grub_ssize_t ret = 0;
> +  grub_disk_t disk = file->device->disk;
>
>if (len > file->size - file->offset)
>  len = file->size - file->offset;
>
>sector = (file->offset >> GRUB_DISK_SECTOR_BITS);
>offset = (file->offset & (GRUB_DISK_SECTOR_SIZE - 1));
> +  disk->read_hook = file->read_hook;
> +  disk->read_hook_data = file->read_hook_data;
>for (p = file->data; p->length && len > 0; p++)
>  {
>if (sector < p->length)
> @@ -232,9 +235,12 @@ grub_fs_blocklist_read (grub_file_t file, char *buf,
> grub_size_t len)
>>> GRUB_DISK_SECTOR_BITS) > p->length - sector)
> size = ((p->length - sector) << GRUB_DISK_SECTOR_BITS) -
> offset;
>
> - if (grub_disk_read (file->device->disk, p->offset + sector,
> offset,
> + if (grub_disk_read (disk, p->offset + sector, offset,
>   size, buf) != GRUB_ERR_NONE)
> -   return -1;
> +   {
> + ret = -1;
> + break;
> +   }
>
>   ret += size;
>   len -= size;
> @@ -244,6 +250,7 @@ grub_fs_blocklist_read (grub_file_t file, char *buf,
> grub_size_t len)
>else
> sector -= p->length;
>  }
> +  disk->read_hook = 0;
>
>return ret;
>  }
> --
> 2.46.0
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: How does Grub find itself being booted from UEFI CD ElTorito image ?

[Vladimir 'phcoder' Serbinenko]

The code for auto detect of root device on EFI is
https://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/kern/efi/init.c#n130
. It will have no problem detecting right device even if there are similar
devices. The only case I know of, where it's unreliable is if partition
module for partition in question is missing but the disk is always detected
correctly unless firmware lies or we have a crazy bug.

Le dim. 15 sept. 2024, 14:07, adrian15sgd  a écrit :

> I actually have an specific question for Grub developers / maintainers /
> contributors so if you are one of them you can skip to: (2) .
>
> 1) INTRODUCTION
>
> 1.1) I am working on liveid ( https://github.com/rescatux/liveid ), this
> is sort of an earmark for live cds.
> I am currently trying to push some changes onto the live-build and
> live-boot packages from Debian GNU/Linux distribution.
>
> I was quite convinced I knew how Grub booted from a Debian's ElTorito
> image bundled into a Debian Live CD but as it turns out I was wrong.
>
>
> 1.2) The current way this is implemented from a Debian point of view
> (based on GRUB 2.06) can be described in two steps.
> 1.2.1) The first step is creating a gcdx64.efi file that has an embedded
> memdisk with a custom grub.cfg.
>
> So... the gcdx64.efi image is created thanks to a custom Debian script
> named `build-efi-images` .
>
> A memdisk image is being created first:
> (
>
> https://salsa.debian.org/grub-team/grub/-/blob/c5fc69927aefefb713d43a3753a7e7062c20ffa9/debian/build-efi-images#L57-76
> )
> and then it's being used on the final gcdx64.efi file:
> (
>
> https://salsa.debian.org/grub-team/grub/-/blob/c5fc69927aefefb713d43a3753a7e7062c20ffa9/debian/build-efi-images#L217-227
> )
> .
>
> Please note that the embedded grub.cfg contents are:
> ```
> if [ -z "\$prefix" -o ! -e "\$prefix" ]; then
>  if ! search --file --set=root /.disk/info; then
>  search --file --set=root /.disk/mini-info
>  fi
>  set prefix=(\$root)/boot/grub
> fi
> if [ -e \$prefix/$platform/grub.cfg ]; then
>  source \$prefix/$platform/grub.cfg
> elif [ -e \$prefix/grub.cfg ]; then
>  source \$prefix/grub.cfg
> else
>  source \$cmdpath/grub.cfg
> fi
> ```
> .
>
> So, as you can see it tries to find the kernel and initrd thanks to info
> and mini-info files.
>
> 1.2.2) The second step is that the Debian live cd is going to have an
> ElTorito image inside of it ( boot/grub/efi.img ) which it's essentially
> a FAT partition.
> You can find the specific code for that creation on:
>
> https://salsa.debian.org/live-team/live-build/-/blob/ccf1f49bb9d185b2bb426ae1d4e84f4c4e4547ff/scripts/build/binary_grub-efi#L287-296
>
> Basically a FAT partition image is created reusing some SecureBoot
> signed EFI images (if available, otherwise you would be using non-signed
> EFI images).
> In the end what the FAT partition image contents are is somewhat similar
> to:
>
> ```
> EFI
> EFI/boot
> EFI/boot/bootx64.efi
> EFI/boot/grubx64.efi
> boot
> boot/grub
> boot/grub/grub.cfg
> ```
>
> The grubx64.efi file that you see above is actually a copy of the
> gcdx64.efi file created above in (1.2.1).
>
> 1.3) How does Debian image boot thanks to this ElTorito image from an
> UEFI CD boot?
>
> UEFI firmware reads whatever defines ElTorito image and finds the
> contents of boot/grub/efi.img .
> UEFI firmware only loads into memory EFI/boot/bootx64.efi on the
> ElTorito image.
> This is usually shim which will boot onto grub.
> But let's suppose that this is grub itself for the sake of simplicity.
>
> When EFI/boot/grubx64.efi ( which it's actually a copy of gcdx64.efi )
> is loaded into RAM as it has an embedded memdisk, its prefix gets set to
> the memdisk image and then the snippet mentioned above that searches for
> `/.disk/info` and `/.disk/mini-info` files is run.
>
> Please note that whatever `boot/grub/grub.cfg` file was inside of the
> ElTorito image gets **ignored**.
> Also please note that we are not reading information about the CD unless
> the CD has such files and it is found first in the Grub devices search.
> In other words if you have a `/.disk/info` file in your internal hard
> disk or another device this might get messed up.
>
> So, usually what happens is that the `/.disk/info` is found on the (cd0)
> device and then everything boots correctly.
>
> 1.4) Building a barebones ElTorito image.
>
> So I have modified both Debian live-build's package and Debian grub
> `build-efi-images` script.
>
> 1.4.1) From the point of view of ElTorito image you only get:
>
> ```
> EFI
> EFI/boot
> EFI/boot/bootx64.efi
> ```
>
> with the modified `gcdx64.efi` contents.
>
> 1.4.2) And what about `gcdx64.efi`, well, it's no longer created with a
> embedded memdisk so it just boots and that's it.
>
> 1.4.3) My only test suggests that somehow Grub knows that its root is
> `(cd0)` and prefix is changed onto the usual `(cd0)/boot/grub` .
>
> So that's very great because you can run the `/boot/grub/grub.cfg` file
> in the cdrom. That's w

Re: [PATCH v5 1/5] Import libgcrypt 1.10.3

[Vladimir 'phcoder' Serbinenko]

On Thu, Sep 5, 2024 at 3:53 PM Daniel Kiper  wrote:
>
> On Tue, Sep 03, 2024 at 08:29:30PM +0300, Vladimir Serbinenko wrote:
> > We currently use an old version of libcrypt which
> > results in us having fewer ciphers and missing on many
> > other improvements.
> >
> > Signed-off-by: Vladimir Serbinenko 
>
> Now at least it builds but...
>
> Could you update libgcrypt to the latest version, 1.11.0?
>
Done
> When I apply the patches git complains in the following way:
> I am OK with warnings from original libgcrypt import but our patches
> should be clean.
>
Warnings come from the fact that spaces are needed in .patch files.
Otherwise they're broken.

> When I run bootstrap I can see this:
>
>   Importing libgcrypt...
>   WARNING: C file isn't a module: blake2.c
>   WARNING: C file isn't a module: ecc-curves.c
>   WARNING: C file isn't a module: ecc-ecdh.c
>   WARNING: C file isn't a module: ecc-ecdsa.c
>   WARNING: C file isn't a module: ecc-eddsa.c
>   WARNING: C file isn't a module: ecc-gost.c
>   WARNING: C file isn't a module: ecc-misc.c
>   WARNING: C file isn't a module: ecc-sm2.c
>   WARNING: C file isn't a module: gost-s-box.c
>   WARNING: C file isn't a module: poly1305.c
>   WARNING: C file isn't a module: primegen.c
>   patching file grub-core/lib/libgcrypt-grub/cipher/md.c
>   patching file grub-core/lib/libgcrypt-grub/cipher/keccak.c
>   Hunk #1 succeeded at 253 (offset 2 lines).
>
> Please fix the warnings and hunk complaint.
I silenced the warnings but they do refer to a real problem: we don't
import some of ciphers. Changed to TODO.

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

Re: [PATCH] Change "efi" to "EFI" in grub-mkrescue for secure boot

[Vladimir 'phcoder' Serbinenko]

>
>
>
> And main argument: I'm aware of one example when "EFI" is hard-coded in
> signed secure boot GRUB binary. Debian has 4 signed GRUB binaries. All they
> are generated here:
> https://sources.debian.org/src/grub2/2.12-5/debian/build-efi-images/ .
> One of them, grubx64.efi is created using "grub-mkimage ... -p /EFI/debian".


Why do they use /EFI if it doesn't even work? The question is who needs to
change this.
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Change "efi" to "EFI" in grub-mkrescue for secure boot

[Vladimir 'phcoder' Serbinenko]

Le sam. 7 sept. 2024, 00:51, Pascal Hambourg  a
écrit :

> On 06/09/2024 at 15:15, Askar Safin wrote:
> >
> > While testing this script I noticed this: if I boot from disk (i. e.
> > using "-drive" Qemu option), then "grub.cfg" is read from ESP, i. e.
> > FAT. But if I boot from CD (i. e. using "-cdrom" Qemu option), then
> > "grub.cfg" is read from main .iso partition, i. e. iso9660. I don't know
> > whether this is expected behavior or just a bug.
>
It's intended. ESP in El Torito is just an artifact of the spec. We always
use entire iso9660 to store our files on all the platforms

>
> I do not know either, but it matches my previous observation. When
> booting from a EFI partition on a regular disk, then $root is set to the
> EFI partition (hdX,Y). When booting from a EFI El Torito image on a CD,
> then $root is set to the whole CD (cdX), and the El Torito image is not
> visible.
>
> > FAT is case insensitive from GRUB point of view. But iso9660 is case
> > sensitive. So, the file (in case of CD boot) must be named exactly
> > /EFI/debian/grub.cfg. Name /efi/debian/grub.cfg will not work,
> I faced a similar issue when trying to use $cmdpath instead of the
> hardcoded prefix /EFI/debian; the case in $cmdpath is variable and
> depends on the UEFI firmware and boot method (from EFI boot entry,
> select boot device, select EFI file). I have seen "/EFI/BOOT" most often
> but also "/EFI/Boot" and "/efi/boot".
>
> On 06/09/2024 at 20:09, Thomas Schmitt wrote:
> >
> > I can confirm that many distro ISOs have a copy of the file tree of their
> > EFI system partition as file tree "/EFI/boot" in the ISO 9660 filesystem.
> > Nobody seems to complain about the mixed case.
> > This copy normally plays no role in booting
>
> It plays a role in Debian ISOhybrid images when booting from CD with a
> monolithic signed GRUB image which fetches an early grub.cfg in the EFI
> partition, because GRUB does not see the El Torito image and can read
> the early grub.cfg only from the ISO 9660 filesystem.
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] docs: Fix incorrect and potentially confusing language and minor formatting

[Vladimir 'phcoder' Serbinenko]

Reviewed-by: phco...@gmail.com

Le ven. 6 sept. 2024, 04:38, Glenn Washburn  a
écrit :

> Signed-off-by: Glenn Washburn 
> ---
>  docs/grub.texi | 15 +++
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 63e796a3a738..e6e5efa05e6c 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -4538,11 +4538,12 @@ files.
>
>  @node emunet_module
>  @section emunet
> -This module provides support for an emulated network card in GRUB.
> +This module provides support for networking in GRUB on the emu platform.
>
>  @node emupci_module
>  @section emupci
> -This module provides support for an emulated PCI bus in GRUB.
> +This module provides support for accessing the PCI bus in GRUB on the emu
> +platform.
>
>  @node erofs_module
>  @section erofs
> @@ -4784,8 +4785,8 @@ image support, and icon support.
>
>  @node gfxterm_module
>  @section gfxterm
> -This module provides support for displaying a graphical terminal
> interface from
> -GRUB.
> +This module provides support for displaying a terminal and menu interface
> from
> +GRUB using graphics mode.
>
>  @node gfxterm_background_module
>  @section gfxterm_background
> @@ -5105,8 +5106,7 @@ at the time of writing.
>  @node memdisk_module
>  @section memdisk
>  This module provides support for a memdisk device. A memdisk is a memory
> mapped
> -emulated disk likely only possible in legacy environment such as with a
> legacy
> -BIOS operating in 16-bit mode.
> +emulated disk.
>
>  @node memrw_module
>  @section memrw
> @@ -5475,8 +5475,7 @@ like interface to some GRUB internal data.
>
>  @node progress_module
>  @section progress
> -This module provides support for showing file loading progress to the
> -terminal.
> +This module provides support for showing file loading progress to the
> terminal.
>
>  @node pxe_module
>  @section pxe
> --
> 2.34.1
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v2 0/7] Fix Yeeloong 2F support

[Vladimir 'phcoder' Serbinenko]

v2: Changed how I fix module search: instead of matching alignment,
compute the module position explicitly

-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH] loader/efi: Update comment

[Vladimir 'phcoder' Serbinenko]

Reviewed-by: phco...@gmail.com

Le mar. 3 sept. 2024, 11:02, Frediano Ziglio via Grub-devel <
grub-devel@gnu.org> a écrit :

> The function called is grub_utf8_to_utf16.
>
> Signed-off-by: Frediano Ziglio 
> ---
>  grub-core/loader/efi/linux.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/grub-core/loader/efi/linux.c b/grub-core/loader/efi/linux.c
> index bfbd95aee..803d2541d 100644
> --- a/grub-core/loader/efi/linux.c
> +++ b/grub-core/loader/efi/linux.c
> @@ -216,7 +216,7 @@ grub_arch_efi_linux_boot_image (grub_addr_t addr,
> grub_size_t size, char *args)
>
>grub_dprintf ("linux", "linux command line: '%s'\n", args);
>
> -  /* Convert command line to UCS-2 */
> +  /* Convert command line to UTF-16 */
>loaded_image = grub_efi_get_loaded_image (image_handle);
>if (loaded_image == NULL)
>  {
> --
> 2.46.0
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] relocator: Use .quad instead of .long

[Vladimir 'phcoder' Serbinenko]

Reviewed-by: phco...@gmail.com

Le mar. 3 sept. 2024, 11:02, Frediano Ziglio via Grub-devel <
grub-devel@gnu.org> a écrit :

> Used also in other assembly files.
> They are single 64-bit values.
>
> Signed-off-by: Frediano Ziglio 
> ---
>  grub-core/lib/x86_64/relocator_asm.S | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/grub-core/lib/x86_64/relocator_asm.S
> b/grub-core/lib/x86_64/relocator_asm.S
> index fd9b2b44e..12728d8e1 100644
> --- a/grub-core/lib/x86_64/relocator_asm.S
> +++ b/grub-core/lib/x86_64/relocator_asm.S
> @@ -26,21 +26,21 @@ VARIABLE(grub_relocator_backward_start)
> .byte   0x48
> .byte   0xb8
>  VARIABLE(grub_relocator_backward_dest)
> -   .long   0, 0
> +   .quad   0
> movq%rax, %rdi
>
> /* mov imm64, %rax */
> .byte   0x48
> .byte   0xb8
>  VARIABLE(grub_relocator_backward_src)
> -   .long   0, 0
> +   .quad   0
> movq%rax, %rsi
>
> /* mov imm64, %rcx */
> .byte   0x48
> .byte   0xb9
>  VARIABLE(grub_relocator_backward_chunk_size)
> -   .long   0, 0
> +   .quad   0
>
> add %rcx, %rsi
> add %rcx, %rdi
> @@ -62,21 +62,21 @@ VARIABLE(grub_relocator_forward_start)
> .byte   0x48
> .byte   0xb8
>  VARIABLE(grub_relocator_forward_dest)
> -   .long   0, 0
> +   .quad   0
> movq%rax, %rdi
>
> /* mov imm64, %rax */
> .byte   0x48
> .byte   0xb8
>  VARIABLE(grub_relocator_forward_src)
> -   .long   0, 0
> +   .quad   0
> movq%rax, %rsi
>
> /* mov imm64, %rcx */
> .byte   0x48
> .byte   0xb9
>  VARIABLE(grub_relocator_forward_chunk_size)
> -   .long   0, 0
> +   .quad   0
>
> /* Forward copy.  */
> cld
> --
> 2.46.0
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] loader/efi: Reuse len variable

[Vladimir 'phcoder' Serbinenko]

This also sets a variable at it's end. Can you adjust the commit message?
Or even split it?

Le mar. 3 sept. 2024, 11:02, Frediano Ziglio via Grub-devel <
grub-devel@gnu.org> a écrit :

> Signed-off-by: Frediano Ziglio 
> ---
>  grub-core/loader/efi/linux.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/grub-core/loader/efi/linux.c b/grub-core/loader/efi/linux.c
> index 803d2541d..1ffbcf9ce 100644
> --- a/grub-core/loader/efi/linux.c
> +++ b/grub-core/loader/efi/linux.c
> @@ -226,7 +226,7 @@ grub_arch_efi_linux_boot_image (grub_addr_t addr,
> grub_size_t size, char *args)
>loaded_image->load_options_size = len =
>  (grub_strlen (args) + 1) * sizeof (grub_efi_char16_t);
>loaded_image->load_options =
> -grub_efi_allocate_any_pages (GRUB_EFI_BYTES_TO_PAGES
> (loaded_image->load_options_size));
> +grub_efi_allocate_any_pages (GRUB_EFI_BYTES_TO_PAGES (len));
>if (!loaded_image->load_options)
>  return grub_errno;
>
> @@ -240,7 +240,8 @@ grub_arch_efi_linux_boot_image (grub_addr_t addr,
> grub_size_t size, char *args)
>/* When successful, not reached */
>grub_error (GRUB_ERR_BAD_OS, "start_image() returned 0x%"
> PRIxGRUB_EFI_UINTN_T, status);
>grub_efi_free_pages ((grub_addr_t) loaded_image->load_options,
> -  GRUB_EFI_BYTES_TO_PAGES
> (loaded_image->load_options_size));
> +  GRUB_EFI_BYTES_TO_PAGES (len));
> +  loaded_image->load_options = NULL;
>  unload:
>b->unload_image (image_handle);
>
> --
> 2.46.0
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH 0/6] Fix Yeeloong 2F support

[Vladimir 'phcoder' Serbinenko]

This series of patches fixes compilation and running on Yeeloong 2F
with new GCC.
Clang is not supported as it fails to compile for mips3 architecture

-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: Feedback Request: Bug #63894 (2038 issue) Proposed Change

[Vladimir 'phcoder' Serbinenko]

I have adjusted grub functions to extend the range, I'll send then later
today

Le sam. 17 août 2024, 17:20, Andrew Hamilton  a écrit :

> Thank you, I see now there are some functions I may be able to use
> already in the GRUB gnulib, I will investigate that.
>
> Thank you for the quick feedback!
>
> - Andrew
>
> On Sat, Aug 17, 2024 at 9:03 AM Vladimir 'phcoder' Serbinenko
>  wrote:
> >
> > No, it's license-incompatible. Please don't use that code.
> >
> > Le sam. 17 août 2024, 16:44, Andrew Hamilton  a
> écrit :
> >>
> >> Hello,
> >>
> >> I'm thinking to try to address bug 63894 (grub_datetime2unixtime()
> >> still has the year 2038 problem):
> >> https://savannah.gnu.org/bugs/?63894
> >>
> >> I confirmed the issue is still partially present in
> >> include/grub/datetime.h... the following checks are still present and
> >> the issue mentioned local variable limitations are also still an
> >> issue:
> >>
> >> grub_datetime2unixtime (const struct grub_datetime *datetime,
> grub_int64_t *nix)
> >> {
> >>   grub_int32_t ret;
> >> ...
> >>   if (datetime->year > 2038 || datetime->year < 1901)
> >> return 0;
> >> ...
> >>
> >> Simply changing the locals to 64-bit and removing the 2038 limit does
> >> not completely solve the issue, at some point past 2038 the result of
> >> this algorithm produces incorrect results.
> >>
> >> To completely solve the issue, I wanted to use the algorithm
> >> implemented by the Linux kernel in mktime64 (time.c around line 449):
> >>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/time/time.c?h=v6.11-rc3
> >>
> >> Would it be acceptable to base the GRUB implementation on the Linux
> >> Kernel algorithm above?
> >>
> >> Thanks,
> >> Andrew
> >>
> >> ___
> >> Grub-devel mailing list
> >> Grub-devel@gnu.org
> >> https://lists.gnu.org/mailman/listinfo/grub-devel
> >
> > ___
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Feedback Request: Bug #63894 (2038 issue) Proposed Change

[Vladimir 'phcoder' Serbinenko]

No, it's license-incompatible. Please don't use that code.

Le sam. 17 août 2024, 16:44, Andrew Hamilton  a écrit :

> Hello,
>
> I'm thinking to try to address bug 63894 (grub_datetime2unixtime()
> still has the year 2038 problem):
> https://savannah.gnu.org/bugs/?63894
>
> I confirmed the issue is still partially present in
> include/grub/datetime.h... the following checks are still present and
> the issue mentioned local variable limitations are also still an
> issue:
>
> grub_datetime2unixtime (const struct grub_datetime *datetime, grub_int64_t
> *nix)
> {
>   grub_int32_t ret;
> ...
>   if (datetime->year > 2038 || datetime->year < 1901)
> return 0;
> ...
>
> Simply changing the locals to 64-bit and removing the 2038 limit does
> not completely solve the issue, at some point past 2038 the result of
> this algorithm produces incorrect results.
>
> To completely solve the issue, I wanted to use the algorithm
> implemented by the Linux kernel in mktime64 (time.c around line 449):
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/time/time.c?h=v6.11-rc3
>
> Would it be acceptable to base the GRUB implementation on the Linux
> Kernel algorithm above?
>
> Thanks,
> Andrew
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: How to test grub_iso9660_uuid from userland ?

[Vladimir 'phcoder' Serbinenko]

Le lun. 1 juil. 2024, 17:12, Thomas Schmitt via Grub-devel <
grub-devel@gnu.org> a écrit :

> Hi,
>
> i found a wrong word in an error message of grub-core/fs/iso9660.c
> function grub_iso9660_uuid():
>
>  grub_error (GRUB_ERR_BAD_NUMBER, "no creation date in filesystem
> to generate UUID");
>
> The missing entity would be the modification date
>   data->voldesc.modified
> rather than the creation date
>   data->voldesc.created
>
> The fix is obviously simple. But i have no idea how to get the code
> execution to this function for testing.
> An ISO with 0 date would be easy to fake. A use case known to me is in
> GRUB configuration with command "search" and option "--fs-uuid".
> In grub-fstest.c i see no opportunity to perform "search".
>
> Any idea how to test grub_iso9660_uuid() from userland of a running
> GNU/Linux ?
>
grub-fstest IMAGE ls -- -l

>
>
> Have a nice day :)
>
> Thomas
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [RFC][PATCH v1 0/4] How to add --set=VARNAME to the ls command?

[Vladimir 'phcoder' Serbinenko]

Did you try:
insmod regexp
for x in (*); do

done
Just trying to understand the problem

Le dim. 30 juin 2024, 19:26, Denis 'GNUtoo' Carikli <
gnu...@cyberdimension.org> a écrit :

> Hi,
>
> The problem we try to solve with --set=VARNAME in ls.
> =
> In the GNU Boot project (a free software distribution that releases
> free software boot firmware images), we provide images with (a
> deblobbed) Coreboot and GRUB (run as a Coreboot payload). We use GRUB
> mainly to find other configuration files like syslinux.cfg (to boot on
> external medias) or grub.cfg (to boot on the (usually GNU/Linux)
> distribution installed to the hard disk / SSD).
>
> We also provide images with a SeaBIOS Coreboot payload instead, but we
> plan to make the images with GRUB become the preffered way of booting
> because in practice it works very well with the Coreboot Framebuffer,
> and with it we only lack a way to reliabily list the devices being
> present in order to be able to also find grub.cfg config files inside
> filesystems present on LVM logical volumes as well.
>
> The alternative to using GRUB as a Coreboot payload is to use SeaBIOS
> instead but that doesn't work well because when SeaBIOS loads the
> (usually GNU/Linux) distribution's GRUB, it results in a black screen
> unless the users tweak the /etc/default/grub configuration to use the
> 'console' output instead of the default gfxterm, and we also want less
> technical users to be able to easily use computers with GNU Boot. This
> issue is probably due to SeaVGABIOS that probably doesn't fully
> implement the VGA standard, so my guess is that fixing this is more
> work than adding --set=VARNAME to the 'ls' command.
>
> Our current GRUB configuration file is in our git repository[1] and it
> hardcodes devices like ahciX,Y and then tries to find the grub.cfg
> with (a limited) number of X,Y combination.
>
> [1]
> https://git.savannah.gnu.org/cgit/gnuboot.git/tree/resources/grub/config/grub.cfg
>
> Questions about the implementation
> ==
> The patch set that follows is far from optimal:
>
> * The 'commands/ls: add --set=VARNAME.' patch only implements
>   --set=VARNAME for 'ls' without other arguments, and it returns an
>   error otherwise. I'm not sure if it's the right solution but in
>   another hand implementing --set=VARNAME for all the ls command would
>   make the patch too big given how the implementation is done (more
>   on that later).
>
> * The patches adding --set=VARNAME 'commands/ls: add --set=VARNAME.'
>   changes is not very intrusive but the later patch 'commands/ls:
>   support --set for files/directories.' shows the broader issue very
>   clearly: all the prints are duplicated with some 'if (varname) {
>   ... }' construct.
>
> Since here my goal is only to add '--set=VARNAME' for 'ls' without
> arguments, what would be the best way to proceed?
>
> Would a patch that doesn't cover all the 'ls' arguments be acceptable?
> If not, I guess that the way to go would be to rework a bit the
> printing as with the current way, there is too much duplication of
> code and it also makes the code harder to follow which in turn makes
> maintenance of this code harder.
>
> In this case what kind of API would be acceptable? Should we introduce
> some functions that have an argument that can select where to print?
>
> If so would something similar to fprintf be ok? It could be used like
> that 'grub_xfprintf( varname ? stdout : varname, "%s\n", "Hello
> world");' and make the code more redable than with the 'commands/ls:
> support --set for files/directories.' patch.
>
> Denis 'GNUtoo' Carikli (4):
>   Add grub_env_append function.
>   Add command to append to existing environment variables.
>   commands/ls: add --set=VARNAME.
>   commands/ls: support --set for files/directories.
>
>  grub-core/commands/ls.c  | 249 ++-
>  grub-core/kern/corecmd.c |  25 
>  grub-core/kern/env.c |  38 ++
>  include/grub/env.h   |   1 +
>  4 files changed, 282 insertions(+), 31 deletions(-)
>
> --
> 2.45.1
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v5 1/2] lsefi: fixed memory leaks

[Vladimir 'phcoder' Serbinenko]

Reviewed-By: Vladimir Serbinenko 

On Fri, Jun 28, 2024 at 2:14 PM Renaud Métrich  wrote:
>
> Signed-off-by: Renaud Métrich 
> ---
>  grub-core/commands/efi/lsefi.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/grub-core/commands/efi/lsefi.c b/grub-core/commands/efi/lsefi.c
> index 7b8316d41..bda25a3a9 100644
> --- a/grub-core/commands/efi/lsefi.c
> +++ b/grub-core/commands/efi/lsefi.c
> @@ -127,8 +127,12 @@ grub_cmd_lsefi (grub_command_t cmd __attribute__ 
> ((unused)),
> grub_printf ("  %pG\n", protocols[j]);
> }
>
> +  if (protocols)
> +   grub_efi_free_pool (protocols);
>  }
>
> +  grub_free (handles);
> +
>return 0;
>  }
>
> --
> 2.45.2
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH v18 12/25] key_protector: Add key protectors framework

[Vladimir 'phcoder' Serbinenko]

> +  if (protector == NULL || protector->name == NULL || grub_strlen 
> (protector->name) == 0)
> +return GRUB_ERR_BAD_ARGUMENT;
> +
Here and in the other places you miss grub_error. Note that the
message in such technical cases should be left untranslated (no N_
mark).

> +  if (protector == NULL || grub_strlen (protector) == 0)

Rather than checking strlen just do protector[0] == '\0'

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

Re: [PATCH v5 2/2] efi: new 'connectefi' command

[Vladimir 'phcoder' Serbinenko]

t; + total_connected++;
> + grub_dprintf ("efi", "  handle %p: connected\n", handle);
> +   }
> + else
> +   grub_dprintf ("efi", "  handle %p: failed to connect (%d)\n",
> +handle, (grub_efi_int8_t) status);
> +
> + struct grub_efi_already_handled *item = grub_malloc(sizeof (*item));
> + if (!item)
> +   break; /* fatal  */
> + grub_list_push (GRUB_AS_LIST_P (&already_handled), GRUB_AS_LIST 
> (item));
> + item->handle = handle;
> +   }
> +
> +  grub_free (handles);
> +  if (grub_err != GRUB_ERR_NONE)
> +   break; /* fatal  */
> +
> +  if (items && items[s].flags & SEARCHED_ITEM_FLAG_LOOP && connected)
> +   {
> + connected = 0;
> + goto loop;
> +   }
> +
> +  free_handle_list ();
> +}
> +
> +  free_handle_list ();
> +
> +  if (total_connected)
> +grub_efidisk_reenumerate_disks ();
> +
> +  return grub_err;
> +}
> +
> +static grub_command_t cmd;
> +
> +GRUB_MOD_INIT(connectefi)
> +{
> +  cmd = grub_register_command ("connectefi", grub_cmd_connectefi,
> +  /* TRANSLATORS: only translate 'all' keyword */
> +  N_("pciroot|scsi|all"),
> +  N_("Connect EFI handles."
> + " If 'pciroot' is specified, connect PCI"
> + " root EFI handles recursively."
> + " If 'scsi' is specified, connect SCSI"
> + " I/O EFI handles recursively."
> + " If 'all' is specified, connect all"
> + " EFI handles recursively."));
> +}
> +
> +GRUB_MOD_FINI(connectefi)
> +{
> +  grub_unregister_command (cmd);
> +}
> diff --git a/grub-core/commands/efi/lsefi.c b/grub-core/commands/efi/lsefi.c
> index bda25a3a9..95a873823 100644
> --- a/grub-core/commands/efi/lsefi.c
> +++ b/grub-core/commands/efi/lsefi.c
> @@ -19,6 +19,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
> index 3b5ed5691..525d59838 100644
> --- a/grub-core/disk/efi/efidisk.c
> +++ b/grub-core/disk/efi/efidisk.c
> @@ -396,6 +396,19 @@ enumerate_disks (void)
>free_devices (devices);
>  }
>
> +void
> +grub_efidisk_reenumerate_disks (void)
> +{
> +  free_devices (fd_devices);
> +  free_devices (hd_devices);
> +  free_devices (cd_devices);
> +  fd_devices = 0;
> +  hd_devices = 0;
> +  cd_devices = 0;
> +
> +  enumerate_disks ();
> +}
> +
>  static int
>  grub_efidisk_iterate (grub_disk_dev_iterate_hook_t hook, void *hook_data,
>   grub_disk_pull_t pull)
> diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
> index b93ae3aba..b7c861b1e 100644
> --- a/grub-core/kern/efi/efi.c
> +++ b/grub-core/kern/efi/efi.c
> @@ -96,6 +96,19 @@ grub_efi_locate_handle (grub_efi_locate_search_type_t 
> search_type,
>return buffer;
>  }
>
> +grub_efi_status_t
> +grub_efi_connect_controller (grub_efi_handle_t controller_handle,
> +grub_efi_handle_t *driver_image_handle,
> +grub_efi_device_path_protocol_t 
> *remaining_device_path,
> +grub_efi_boolean_t recursive)
> +{
> +  grub_efi_boot_services_t *b;
> +
> +  b = grub_efi_system_table->boot_services;
> +  return efi_call_4 (b->connect_controller, controller_handle,
> +driver_image_handle, remaining_device_path, recursive);
> +}
> +
>  void *
>  grub_efi_open_protocol (grub_efi_handle_t handle,
> grub_guid_t *protocol,
> diff --git a/include/grub/efi/disk.h b/include/grub/efi/disk.h
> index 254475c84..6845c2f1f 100644
> --- a/include/grub/efi/disk.h
> +++ b/include/grub/efi/disk.h
> @@ -27,6 +27,8 @@ grub_efi_handle_t
>  EXPORT_FUNC(grub_efidisk_get_device_handle) (grub_disk_t disk);
>  char *EXPORT_FUNC(grub_efidisk_get_device_name) (grub_efi_handle_t *handle);
>
> +void EXPORT_FUNC(grub_efidisk_reenumerate_disks) (void);
> +
>  void grub_efidisk_init (void);
>  void grub_efidisk_fini (void);
>
> diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
> index a5cd99e5a..60d0b2bf8 100644
> --- a/include/grub/efi/efi.h
> +++ b/include/grub/efi/efi.h
> @@ -44,6 +44,11 @@ EXPORT_FUNC(grub_efi_locate_handle) 
> (grub_efi_locate_search_type_t search_type,
>  grub_guid_t *protocol,
>  void *search_key,
>  grub_efi_uintn_t *num_handles);
> +grub_efi_status_t
> +EXPORT_FUNC(grub_efi_connect_controller) (grub_efi_handle_t 
> controller_handle,
> + grub_efi_handle_t 
> *driver_image_handle,
> + grub_efi_device_path_protocol_t 
> *remaining_device_path,
> + grub_efi_boolean_t recursive);
>  void *EXPORT_FUNC(grub_efi_open_protocol) (grub_efi_handle_t handle,
>grub_guid_t *protocol,
>grub_efi_uint32_t attributes);
> --
> 2.45.2
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH V6] ieee1275/ofdisk: retry on open and read failure

[Vladimir 'phcoder' Serbinenko]

Should we add a random wait to avoid all the machines retrying au the same
time?

Le mer. 26 juin 2024, 13:57, Mukesh Kumar Chaurasiya 
a écrit :

> Sometimes, when booting from a very busy SAN, the access to the
> disk can fail and then GRUB will eventually drop to GRUB prompt.
> This scenario is more frequent when deploying many machines at
> the same time using the same SAN.
> This patch aims to force the ofdisk module to retry the open or
> read function for network disks after it fails. We use
> DEFAULT_RETRY_TIMEOUT, which is 15 seconds to specify the time it'll
> retry to access the disk before it definitely fails. The timeout can be
> changed by setting the environment variable ofdisk_retry_timeout.
> If the environment variable fails to read, GRUB will consider the
> default value of 15 seconds.
>
> Signed-off-by: Diego Domingos 
> Signed-off-by: Mukesh Kumar Chaurasiya 
> ---
>  docs/grub.texi   |  8 
>  grub-core/disk/ieee1275/ofdisk.c | 82 ++--
>  2 files changed, 87 insertions(+), 3 deletions(-)
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index f3bdc2564..9514271fc 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -3308,6 +3308,7 @@ These variables have special meaning to GRUB.
>  * net_default_ip::
>  * net_default_mac::
>  * net_default_server::
> +* ofdisk_retry_timeout::
>  * pager::
>  * prefix::
>  * pxe_blksize::
> @@ -3738,6 +3739,13 @@ The default is the value of @samp{color_normal}
> (@pxref{color_normal}).
>  @xref{Network}.
>
>
> +@node ofdisk_retry_timeout
> +@subsection ofdisk_retry_timeout
> +
> +The time in seconds till which the GRUB will retry to open or read a disk
> in
> +case of failure to do so. This value defaults to 15 seconds.
> +
> +
>  @node pager
>  @subsection pager
>
> diff --git a/grub-core/disk/ieee1275/ofdisk.c
> b/grub-core/disk/ieee1275/ofdisk.c
> index c6cba0c8a..675e039c5 100644
> --- a/grub-core/disk/ieee1275/ofdisk.c
> +++ b/grub-core/disk/ieee1275/ofdisk.c
> @@ -24,6 +24,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +
> +#define RETRY_DEFAULT_TIMEOUT 15
>
>  static char *last_devpath;
>  static grub_ieee1275_ihandle_t last_ihandle;
> @@ -452,7 +455,7 @@ compute_dev_path (const char *name)
>  }
>
>  static grub_err_t
> -grub_ofdisk_open (const char *name, grub_disk_t disk)
> +grub_ofdisk_open_real (const char *name, grub_disk_t disk)
>  {
>grub_ieee1275_phandle_t dev;
>char *devpath;
> @@ -525,6 +528,54 @@ grub_ofdisk_open (const char *name, grub_disk_t disk)
>return 0;
>  }
>
> +static grub_uint64_t
> +grub_ofdisk_disk_timeout (grub_disk_t disk)
> +{
> +  grub_uint64_t retry = RETRY_DEFAULT_TIMEOUT;
> +  const char *timeout = grub_env_get ("ofdisk_retry_timeout");
> +  const char *timeout_end;
> +
> +  if (grub_strstr (disk->name, "fibre-channel") != NULL ||
> +  grub_strstr (disk->name, "vfc-client") != NULL)
> +{
> +  if (timeout == NULL)
> +  return retry;
> +  retry = grub_strtoul (timeout, &timeout_end, 10);
> +  /* Ignore all errors and return default timeout */
> +  if (*timeout == '\0' ||
> +  *timeout_end != '\0')
> +  return RETRY_DEFAULT_TIMEOUT;
> +}
> +  else
> +return 0;
> +
> +  return retry;
> +}
> +
> +static grub_err_t
> +grub_ofdisk_open (const char *name, grub_disk_t disk)
> +{
> +  grub_err_t err;
> +  grub_uint64_t timeout = grub_get_time_ms () + (grub_ofdisk_disk_timeout
> (disk) * 1000);
> +  grub_uint16_t inc = 0;
> +
> +  do
> +{
> +  err = grub_ofdisk_open_real (name, disk);
> +  if (err == GRUB_ERR_UNKNOWN_DEVICE)
> +  grub_dprintf ("ofdisk", "Failed to open disk %s.\n", name);
> +  if (grub_get_time_ms () >= timeout)
> +break;
> +  grub_dprintf ("ofdisk", "Retry to open disk %s.\n", name);
> +  /*
> +   * Increase wait time for subsequent requests
> +   * Cur time is used as a source of randomness
> +   */
> +  grub_millisleep ((32 << ++inc) * (grub_get_time_ms () % 32));
> +} while (1);
> +  return err;
> +}
> +
>  static void
>  grub_ofdisk_close (grub_disk_t disk)
>  {
> @@ -568,8 +619,8 @@ grub_ofdisk_prepare (grub_disk_t disk,
> grub_disk_addr_t sector)
>  }
>
>  static grub_err_t
> -grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
> - grub_size_t size, char *buf)
> +grub_ofdisk_read_real (grub_disk_t disk, grub_disk_addr_t sector,
> +   grub_size_t size, char *buf)
>  {
>grub_err_t err;
>grub_ssize_t actual;
> @@ -587,6 +638,31 @@ grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t
> sector,
>return 0;
>  }
>
> +static grub_err_t
> +grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
> +  grub_size_t size, char *buf)
> +{
> +  grub_err_t err;
> +  grub_uint64_t timeout = grub_get_time_ms () + (grub_ofdisk_disk_timeout
> (disk) * 1000);
> +  grub_uint16_t inc = 0;
> +
> +  do
> +{
> +  err = grub_ofdisk_read_real (disk, sector, 

Re: [PATCH v3 2/6] ieee1275/powerpc: enables device mapper discovery

[Vladimir 'phcoder' Serbinenko]

Concept of ofpathname is clear: return OF path to a given device or NULL if
none. Returning anything else is a bug even if it ends up helping you
somewhere.

Le jeu. 20 juin 2024, 12:45, avnish  a écrit :

> Hi Vladimir,
>
> We have implemented this code to enable the ieee1275 hint for grub. We
> had scenarios (in SLES) like the disk had PReP partition followed by an
> LVM and, inside this LVM , the boot partition. So, we implemented this
> code to make grub able to generate the hint.


Do you mean search hint? Correct hint for lvm is lvmid/... .

As for restricting to PPC: it solves exactly nothing of concerns.
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] loader/efi/fdt: Add fdtdump command to access device tree

[Vladimir 'phcoder' Serbinenko]

gt; +
> +  if (fw_fdt == NULL)
> +  return grub_error (GRUB_ERR_IO,
> + N_("No device tree found"));
> +
> +  if (state[0].set)
> +  value = grub_fdt_get_prop (fw_fdt, 0, state[0].arg, NULL);
> +
> +  if (value == NULL)
> +return grub_error (GRUB_ERR_OUT_OF_RANGE,
> +   N_("failed to retrieve the prop field"));
> +
> +  if (state[1].set)
> +grub_env_set (state[1].arg, value);
> +  else
> +grub_printf ("%s\n", value);
> +
> +  return GRUB_ERR_NONE;
> +}
> +
>  static grub_command_t cmd_devicetree;
> +static grub_extcmd_t cmd_fdtdump;
>
>  GRUB_MOD_INIT (fdt)
>  {
> +  fw_fdt = grub_efi_get_firmware_fdt();
> +
> +  cmd_fdtdump =
> +grub_register_extcmd ("fdtdump", grub_cmd_fdtdump, 0,
> +  N_("[-p] [--set variable]"),
> +          N_("Retrieve device tree information."),
> +  options_fdtdump);
>cmd_devicetree =
>  grub_register_command_lockdown ("devicetree", grub_cmd_devicetree, 0,
> N_("Load DTB file."));
> @@ -179,4 +225,5 @@ GRUB_MOD_INIT (fdt)
>  GRUB_MOD_FINI (fdt)
>  {
>grub_unregister_command (cmd_devicetree);
> +  grub_unregister_extcmd (cmd_fdtdump);
>  }
> --
> 2.43.0
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [Solved] Re: How to test the git clone without "make install" ?

[Vladimir 'phcoder' Serbinenko]

Le ven. 14 juin 2024, 19:27, Thomas Schmitt via Grub-devel <
grub-devel@gnu.org> a écrit :

> Hi,
>
> Maximilian Stendler wrote:
> > to keep the host installation clean, I would probably use a container.
>
> Yes, a virtual machine came to my mind. Easy to clone and to dispose.
> But there must be some better way to test a utility built from git
> independenly of systemwide directories.
>
>
> Vladimir 'phcoder' Serbinenko wrote:
> > Set pkgdatadir environment variable
>
> Ahum ...
>   rm /usr/local/share/grub
>   pkgdatadir=. ./grub-mkrescue -o /dvdbuffer/test.iso
> yields indeed an ISO with EFI boot equipment.
>
> But what to do about /usr/local/lib/grub ?
> I found option -d meanwhile. After some forth and back i came to
>
>
>   pkgdatadir=. ./grub-mkrescue -o /dvdbuffer/test.iso -d ./grub-core
>
>
> which to my surprise creates an ISO with boot equipment for legacy BIOS:
>
>   $ xorriso -indev /dvdbuffer/test.iso -report_el_torito plain
> -report_system_area plain
>   ...
>   El Torito images   :   N  Pltf  B   Emul  Ld_seg  Hdpt  Ldsiz LBA
>   El Torito boot img :   1  BIOS  y   none  0x  0x00  41397
>   El Torito img path :   1  /boot/grub/i386-pc/eltorito.img
>   El Torito img opts :   1  boot-info-table grub2-boot-info
>   ...
>   System area summary: MBR protective-msdos-label grub2-mbr cyl-align-off
>   ...
>   MBR partition table:   N Status  TypeStart   Blocks
>   MBR partition  :   1   0x80  0xcd113783
>
> While i used the Debian system directories it was EFI:
>
>   El Torito images   :   N  Pltf  B   Emul  Ld_seg  Hdpt  Ldsiz LBA
>   El Torito boot img :   1  UEFI  y   none  0x  0x00   5760  52
>   El Torito img path :   1  /efi.img
>   ...
>   System area summary: MBR protective-msdos-label cyl-align-off GPT APM
>   ...
>   MBR partition table:   N Status  TypeStart   Blocks
>   MBR partition  :   1   0x00  0xee118015
>   ... and a GPT and an Apple Partition Map for HFS+ ...
>
>
> So i will start a new thread with the question:
>   How do i convince the git clone to produce programs and ISO for 64 bit
>   EFI.
>

./configure --with-platform=efi --target=x86_64

>
>
>
> Have a nice day :)
>
> Thomas
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: How to test the git clone without "make install" ?

[Vladimir 'phcoder' Serbinenko]

Set pkgdatadir environment variable

Le ven. 14 juin 2024, 16:17, Thomas Schmitt via Grub-devel <
grub-devel@gnu.org> a écrit :

> Hi,
>
> on occasion of
>   https://savannah.gnu.org/bugs/index.php?65880
>   "heap-buffer-overflow in grub-mkrescue.c"
> i try to get grub-mkrescue running from git.
>
> My problem is that grub_util_get_pkglibdir() returns
>   /usr/local/lib/grub
> and grub_util_get_pkgdatadir() returns
>   /usr/local/share/grub
> which of course do not come with a Debian installation.
> So grub-mkrescue produces only a very small ISO with no boot lures or
> boot programs. Quite unrealistic for testing.
>
> I was able to overcome this obstacle by
>   ln -s /usr/lib/grub /usr/local/lib/grub
>   ln -s /usr/share/grub /usr/local/share/grub
> but i understand that now my grub-mkrescue actually copies the ISO content
> from the Debian installation and not from the git clone.
>
> The manual
>   https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html
> does not give me a clue how i would get the /usr/local/*/grub
> directories populated with the files made from the git clone.
> I guess "make install" would do it for me, but i fear that this does
> too many other things to the GRUB installation of my vanilla Debian.
> In general i would prefer to keep the git files away from any system
> directory.
>
> So what can i do to make the files built from git available to
> ./grub-mkrescue built from git, without frankensteining my Debian 12 ?
>
>
> Have a nice day :)
>
> Thomas
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 2/6] ieee1275/powerpc: enables device mapper discovery

[Vladimir 'phcoder' Serbinenko]

Le ven. 7 juin 2024, 10:34, avnish  a écrit :

> On 2024-06-06 21:04, Vladimir 'phcoder' Serbinenko wrote:
> > 2 problems: * How does dm device ends up on ofpathname? It should be
> > handled by grub internal logic in most cases and not end up in
> > of-specific paths * Why not use existing devmapper functions already
> > present in codebase? Le jeu. 6 juin 2024,
> > ZjQcmQRYFpfptBannerStart
> >
> >  This Message Is From an External Sender
> >  This message came from outside your organization.
> >
> >  Report Suspicious
> >
> > ZjQcmQRYFpfptBannerEnd
> > 2 problems:
> > * How does dm device ends up on ofpathname? It should be handled by
> > grub internal logic in most cases and not end up in of-specific paths
> > * Why not use existing devmapper functions already present in
> > codebase?
> >
>
> Hi Vladimir,
> Thank you so much for your response!
>
> We have observed that whenever we are dealing with the devices like
> "/dev/dm-*", the ofpath returns null.
>
It's expected behavior

> To resolve this, as no such required functions has been implemented to
> handle this kind of case. We have done changes based on the requirement
> that will look into /sys/block/dm-* devices and search slave
> devices recursively inside slaves directory to find the root disk.

Installing on e.g. LVM is invalid. Only 3 cases can your approach work:
1) multipath
2) maybe RAID1 as well in some cases
3) if ofw assembles raid somehow
Which one do you have? We need more details.
This patch enables invalid configs like installing on LVM

>
> Regards,
> Avnish Chouhan
>
> > Le jeu. 6 juin 2024, 14:40, Avnish Chouhan  a
> > écrit :
> >
> >> This patch enables the device mapper discovery on ofpath.c.
> >> Currently,
> >> when we are dealing with a device like /dev/dm-* the ofpath returns
> >> null
> >> since there is no function implemented to handle this case.
> >>
> >> This patch implements a function that will look into /sys/block/dm-*
> >> devices and search recursively inside slaves directory to find the
> >> root
> >> disk.
> >>
> >> Signed-off-by: Diego Domingos 
> >> Signed-off-by: Avnish Chouhan 
> >> ---
> >> grub-core/osdep/linux/ofpath.c | 64
> >> +-
> >> 1 file changed, 63 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/grub-core/osdep/linux/ofpath.c
> >> b/grub-core/osdep/linux/ofpath.c
> >> index 0f5d54e9f2d..cc849d9c94c 100644
> >> --- a/grub-core/osdep/linux/ofpath.c
> >> +++ b/grub-core/osdep/linux/ofpath.c
> >> @@ -37,6 +37,7 @@
> >> #include 
> >> #include 
> >> #include 
> >> +#include 
> >>
> >> #ifdef __sparc__
> >> typedef enum
> >> @@ -755,13 +756,74 @@ strip_trailing_digits (const char *p)
> >> return new;
> >> }
> >>
> >> +static char *
> >> +get_slave_from_dm (const char * device)
> >> +{
> >> +  char *curr_device, *tmp;
> >> +  char *directory;
> >> +  char *ret = NULL;
> >> +  directory = grub_strdup (device);
> >> +  tmp = get_basename (directory);
> >> +  curr_device = grub_strdup (tmp);
> >> +  *tmp = '\0';
> >> +
> >> +  /* Recursively check for slaves devices so we can find the root
> >> device */
> >> +  while ((curr_device[0] == 'd') && (curr_device[1] == 'm') &&
> >> (curr_device[2] == '-'))
> >> +{
> >> +  DIR *dp;
> >> +  struct dirent *ep;
> >> +  char* device_path;
> >> +  device_path = grub_xasprintf ("/sys/block/%s/slaves",
> >> curr_device);
> >> +  dp = opendir (device_path);
> >> +
> >> +  if (dp != NULL)
> >> +{
> >> +  ep = readdir (dp);
> >> +
> >> +  while (ep != NULL)
> >> +{
> >> +  /* avoid some system directories */
> >> +  if (!strcmp(ep->d_name,"."))
> >> +goto next_dir;
> >> +  if (!strcmp(ep->d_name,".."))
> >> +goto next_dir;
> >> +
> >> +  free (curr_device);
> >> +  free (ret);
> >> +  curr_device = grub_strdup (ep->d_name);
> >> +  ret = grub_xasprintf ("%s%s", directory,
> >> curr_device);
&g

Re: [PATCH v3 2/6] ieee1275/powerpc: enables device mapper discovery

[Vladimir 'phcoder' Serbinenko]

2 problems:
* How does dm device ends up on ofpathname? It should be handled by grub
internal logic in most cases and not end up in of-specific paths
* Why not use existing devmapper functions already present in codebase?

Le jeu. 6 juin 2024, 14:40, Avnish Chouhan  a écrit :

> This patch enables the device mapper discovery on ofpath.c. Currently,
> when we are dealing with a device like /dev/dm-* the ofpath returns null
> since there is no function implemented to handle this case.
>
> This patch implements a function that will look into /sys/block/dm-*
> devices and search recursively inside slaves directory to find the root
> disk.
>
> Signed-off-by: Diego Domingos 
> Signed-off-by: Avnish Chouhan 
> ---
>  grub-core/osdep/linux/ofpath.c | 64
> +-
>  1 file changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/osdep/linux/ofpath.c
> b/grub-core/osdep/linux/ofpath.c
> index 0f5d54e9f2d..cc849d9c94c 100644
> --- a/grub-core/osdep/linux/ofpath.c
> +++ b/grub-core/osdep/linux/ofpath.c
> @@ -37,6 +37,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #ifdef __sparc__
>  typedef enum
> @@ -755,13 +756,74 @@ strip_trailing_digits (const char *p)
>return new;
>  }
>
> +static char *
> +get_slave_from_dm (const char * device)
> +{
> +  char *curr_device, *tmp;
> +  char *directory;
> +  char *ret = NULL;
> +  directory = grub_strdup (device);
> +  tmp = get_basename (directory);
> +  curr_device = grub_strdup (tmp);
> +  *tmp = '\0';
> +
> +  /* Recursively check for slaves devices so we can find the root device
> */
> +  while ((curr_device[0] == 'd') && (curr_device[1] == 'm') &&
> (curr_device[2] == '-'))
> +{
> +  DIR *dp;
> +  struct dirent *ep;
> +  char* device_path;
> +  device_path = grub_xasprintf ("/sys/block/%s/slaves", curr_device);
> +  dp = opendir (device_path);
> +
> +  if (dp != NULL)
> +{
> +  ep = readdir (dp);
> +
> +  while (ep != NULL)
> +{
> +  /* avoid some system directories */
> +  if (!strcmp(ep->d_name,"."))
> +goto next_dir;
> +  if (!strcmp(ep->d_name,".."))
> +goto next_dir;
> +
> +  free (curr_device);
> +  free (ret);
> +  curr_device = grub_strdup (ep->d_name);
> +  ret = grub_xasprintf ("%s%s", directory, curr_device);
> +  break;
> +
> +  next_dir:
> +  ep = readdir (dp);
> +  continue;
> +}
> +  closedir (dp);
> +}
> +  else
> +grub_util_warn (_("cannot open directory `%s'"), device_path);
> +  free (device_path);
> +}
> +  free (directory);
> +  free (curr_device);
> +
> +  return ret;
> +}
> +
>  char *
>  grub_util_devname_to_ofpath (const char *sys_devname)
>  {
> -  char *name_buf, *device, *devnode, *devicenode, *ofpath;
> +  char *name_buf, *device, *devnode, *devicenode, *ofpath, *realname;
>
>name_buf = xrealpath (sys_devname);
>
> +  realname = get_slave_from_dm (name_buf);
> +  if (realname)
> +{
> +  free (name_buf);
> +  name_buf = realname;
> +}
> +
>device = get_basename (name_buf);
>devnode = strip_trailing_digits (name_buf);
>devicenode = strip_trailing_digits (device);
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] cli_lock: Added build option to block command line interface

[Vladimir 'phcoder' Serbinenko]

a3 100644
> --- a/util/grub-install-common.c
> +++ b/util/grub-install-common.c
> @@ -466,6 +466,7 @@ static size_t npubkeys;
>  static char *sbat;
>  static int disable_shim_lock;
>  static grub_compression_t compression;
> +static int disable_cli;
>
>  int
>  grub_install_parse (int key, char *arg)
> @@ -504,6 +505,9 @@ grub_install_parse (int key, char *arg)
>  case GRUB_INSTALL_OPTIONS_DISABLE_SHIM_LOCK:
>disable_shim_lock = 1;
>return 1;
> +case GRUB_INSTALL_OPTIONS_DISABLE_CLI:
> +  disable_cli = 1;
> +  return 1;
>
>  case GRUB_INSTALL_OPTIONS_VERBOSITY:
>verbosity++;
> @@ -679,11 +683,12 @@ grub_install_make_image_wrap_file (const char *dir, 
> const char *prefix,
>*p = '\0';
>
>grub_util_info ("grub-mkimage --directory '%s' --prefix '%s' --output '%s'"
> - " --format '%s' --compression '%s'%s%s%s\n",
> + " --format '%s' --compression '%s'%s%s%s%s\n",
>   dir, prefix, outname,
>   mkimage_target, compnames[compression],
>   note ? " --note" : "",
> - disable_shim_lock ? " --disable-shim-lock" : "", s);
> + disable_shim_lock ? " --disable-shim-lock" : "",
> + disable_cli ? " --disable-cli" : "", s);
>free (s);
>
>tgt = grub_install_get_image_target (mkimage_target);
> @@ -694,7 +699,7 @@ grub_install_make_image_wrap_file (const char *dir, const 
> char *prefix,
>modules.entries, memdisk_path,
>pubkeys, npubkeys, config_path, tgt,
>note, compression, dtb, sbat,
> -  disable_shim_lock);
> +  disable_shim_lock, disable_cli);
>while (dc--)
>  grub_install_pop_module ();
>  }
> diff --git a/util/grub-mkimage.c b/util/grub-mkimage.c
> index c0d559937..547f7310f 100644
> --- a/util/grub-mkimage.c
> +++ b/util/grub-mkimage.c
> @@ -83,6 +83,7 @@ static struct argp_option options[] = {
>{"compression",  'C', "(xz|none|auto)", 0, N_("choose the compression to 
> use for core image"), 0},
>{"sbat", 's', N_("FILE"), 0, N_("SBAT metadata"), 0},
>{"disable-shim-lock", GRUB_INSTALL_OPTIONS_DISABLE_SHIM_LOCK, 0, 0, 
> N_("disable shim_lock verifier"), 0},
> +  {"disable-cli", GRUB_INSTALL_OPTIONS_DISABLE_CLI, 0, 0, N_("disable 
> command line interface access"), 0},
>{"verbose", 'v', 0,  0, N_("print verbose messages."), 0},
>{ 0, 0, 0, 0, 0, 0 }
>  };
> @@ -128,6 +129,7 @@ struct arguments
>char *sbat;
>int note;
>int disable_shim_lock;
> +  int disable_cli;
>const struct grub_install_image_target_desc *image_target;
>grub_compression_t comp;
>  };
> @@ -239,6 +241,10 @@ argp_parser (int key, char *arg, struct argp_state 
> *state)
>arguments->disable_shim_lock = 1;
>break;
>
> +case GRUB_INSTALL_OPTIONS_DISABLE_CLI:
> +  arguments->disable_cli = 1;
> +  break;
> +
>  case 'v':
>verbosity++;
>break;
> @@ -325,7 +331,8 @@ main (int argc, char *argv[])
>arguments.npubkeys, arguments.config,
>arguments.image_target, arguments.note,
>arguments.comp, arguments.dtb,
> -  arguments.sbat, arguments.disable_shim_lock);
> +  arguments.sbat, arguments.disable_shim_lock,
> +  arguments.disable_cli);
>
>if (grub_util_file_sync (fp) < 0)
>  grub_util_error (_("cannot sync `%s': %s"), arguments.output ? : 
> "stdout",
> diff --git a/util/mkimage.c b/util/mkimage.c
> index 4237383ac..8c5660825 100644
> --- a/util/mkimage.c
> +++ b/util/mkimage.c
> @@ -886,7 +886,8 @@ grub_install_generate_image (const char *dir, const char 
> *prefix,
>  size_t npubkeys, char *config_path,
>  const struct grub_install_image_target_desc 
> *image_target,
>  int note, grub_compression_t comp, const char 
> *dtb_path,
> -const char *sbat_path, int disable_shim_lock)
> +const char *sbat_path, int disable_shim_lock,
> +int disable_cli)
>  {
>char *kernel_img, *core_img;
>size_t total_module_size, core_size;
> @@ -948,6 +949,9 @@ grub_install_generate_image (const char *dir, const char 
> *prefix,
>if (disable_shim_lock)
>  total_module_size += sizeof (struct grub_module_header);
>
> +  if (disable_cli)
> +total_module_size += sizeof (struct grub_module_header);
> +
>if (config_path)
>  {
>config_size = ALIGN_ADDR (grub_util_get_image_size (config_path) + 1);
> @@ -1094,6 +1098,16 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>offset += sizeof (*header);
>  }
>
> +  if (disable_cli)
> +{
> +  struct grub_module_header *header;
> +
> +  header = (struct grub_module_header *) (kernel_img + offset);
> +  header->type = grub_host_to_target32 (OBJ_TYPE_DISABLE_CLI);
> +  header->size = grub_host_to_target32 (sizeof (*header));
> +  offset += sizeof (*header);
> +}
> +
>if (config_path)
>  {
>struct grub_module_header *header;
> --
> 2.27.0
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH v3 5/5] keccak: Disable acceleration with SSE asm

[Vladimir 'phcoder' Serbinenko]

Sorry, sent wrong commit. Will resend in next version

Le mar. 28 mai 2024, 08:49, Gary Lin  a écrit :

> On Fri, May 24, 2024 at 08:30:06PM +0300, Vladimir Serbinenko wrote:
> > ---
> >  .../lib/libgcrypt-patches/02_keccak_sse.diff  | 19 +++
> >  1 file changed, 19 insertions(+)
> >  create mode 100644 grub-core/lib/libgcrypt-patches/02_keccak_sse.diff
> >
> > diff --git a/grub-core/lib/libgcrypt-patches/02_keccak_sse.diff
> b/grub-core/lib/libgcrypt-patches/02_keccak_sse.diff
> > new file mode 100644
> > index 0..980ebb2b7
> > --- /dev/null
> > +++ b/grub-core/lib/libgcrypt-patches/02_keccak_sse.diff
> > @@ -0,0 +1,19 @@
> > +commit b0cf06271da5fe20360953a53a47c69da89669cd
> > +Author: Vladimir Serbinenko 
> > +Date:   Sun Apr 7 06:33:11 2024 +0300
> > +
> > +keccak: Disable acceleration with SSE asm
> > +
> > +diff --git a/grub-core/lib/libgcrypt/cipher/keccak.c
> b/grub-core/lib/libgcrypt/cipher/keccak.c
> Since this patch file is applied after importing libgcrypt to
> libgcrypt-grub,
> the target file has to be libgcrypt-grub/cipher/keccak.c.
>
> Gary Lin
>
> > +index 11e64b3e7..8b570263b 100644
> > +--- a/grub-core/lib/libgcrypt/cipher/keccak.c
> >  b/grub-core/lib/libgcrypt/cipher/keccak.c
> > +@@ -251,7 +251,7 @@ keccak_absorb_lane32bi(u32 *lane, u32 x0, u32 x1)
> > + /* Construct generic 64-bit implementation. */
> > + #ifdef USE_64BIT
> > +
> > +-#if __GNUC__ >= 4 && defined(__x86_64__)
> > ++#if __GNUC__ >= 4 && defined(__x86_64__) && 0
> > +
> > + static inline void absorb_lanes64_8(u64 *dst, const byte *in)
> > + {
> > --
> > 2.39.2
> >
> >
> > ___
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Mandatory install device check for PowerPC

[Vladimir 'phcoder' Serbinenko]

Le lun. 27 mai 2024, 16:38, Avnish Chouhan  a écrit :

> This patch adds a check on install_device while installing grub for
> PowerPC.
> If install_device is not mentioned in grub2-install, the error will be
> thrown.
> Running grub2-install on PowerPC without the install_device may
> result in boot corruption.
>
This breaks PowerMac booting which doesn't have install device.

>
> Signed-off-by: Avnish Chouhan 
> ---
>  util/grub-install.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/util/grub-install.c b/util/grub-install.c
> index 5babc7a..192d2a8 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -970,6 +970,8 @@ main (int argc, char *argv[])
>  case GRUB_INSTALL_PLATFORM_POWERPC_IEEE1275:
>if (install_device)
> is_prep = 1;
> +  else
> +grub_util_error ("%s", _("install device isn't specified"));
>break;
>  case GRUB_INSTALL_PLATFORM_MIPS_ARC:
>  case GRUB_INSTALL_PLATFORM_MIPSEL_ARC:
> --
> 2.39.3
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 13/15] efi: Provide wrappers for load_image, start_image, unload_image

[Vladimir 'phcoder' Serbinenko]

gt; +   boot_image, size, &image_handle);
>if (status != GRUB_EFI_SUCCESS)
>  {
>if (status == GRUB_EFI_OUT_OF_RESOURCES)
> @@ -422,7 +419,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
> ((unused)),
>  b->free_pages (address, pages);
>
>if (image_handle != NULL)
> -b->unload_image (image_handle);
> +grub_efi_unload_image (image_handle);
>
>grub_dl_unref (my_mod);
>
> diff --git a/grub-core/loader/efi/linux.c b/grub-core/loader/efi/linux.c
> index bfbd95aee..58be3c9f8 100644
> --- a/grub-core/loader/efi/linux.c
> +++ b/grub-core/loader/efi/linux.c
> @@ -187,7 +187,6 @@ grub_arch_efi_linux_boot_image (grub_addr_t addr, 
> grub_size_t size, char *args)
>  {
>grub_efi_memory_mapped_device_path_t *mempath;
>grub_efi_handle_t image_handle;
> -  grub_efi_boot_services_t *b;
>grub_efi_status_t status;
>grub_efi_loaded_image_t *loaded_image;
>int len;
> @@ -207,10 +206,9 @@ grub_arch_efi_linux_boot_image (grub_addr_t addr, 
> grub_size_t size, char *args)
>mempath[1].header.subtype = GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE;
>mempath[1].header.length = sizeof (grub_efi_device_path_t);
>
> -  b = grub_efi_system_table->boot_services;
> -  status = b->load_image (0, grub_efi_image_handle,
> - (grub_efi_device_path_t *) mempath,
> - (void *) addr, size, &image_handle);
> +  status = grub_efi_load_image (0, grub_efi_image_handle,
> +   (grub_efi_device_path_t *)mempath,
> +   (void *)addr, size, &image_handle);
>if (status != GRUB_EFI_SUCCESS)
>  return grub_error (GRUB_ERR_BAD_OS, "cannot load image");
>
> @@ -235,14 +233,14 @@ grub_arch_efi_linux_boot_image (grub_addr_t addr, 
> grub_size_t size, char *args)
> (grub_uint8_t *) args, len, NULL);
>
>grub_dprintf ("linux", "starting image %p\n", image_handle);
> -  status = b->start_image (image_handle, 0, NULL);
> +  status = grub_efi_start_image (image_handle, 0, NULL);
>
>/* When successful, not reached */
>grub_error (GRUB_ERR_BAD_OS, "start_image() returned 0x%" 
> PRIxGRUB_EFI_UINTN_T, status);
>grub_efi_free_pages ((grub_addr_t) loaded_image->load_options,
>GRUB_EFI_BYTES_TO_PAGES 
> (loaded_image->load_options_size));
>  unload:
> -  b->unload_image (image_handle);
> +  grub_efi_unload_image (image_handle);
>
>return grub_errno;
>  }
> diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
> index 194adc1f7..6b517a1ea 100644
> --- a/include/grub/efi/efi.h
> +++ b/include/grub/efi/efi.h
> @@ -132,6 +132,43 @@ grub_err_t 
> grub_arch_efi_linux_load_image_header(grub_file_t file,
>  grub_err_t grub_arch_efi_linux_boot_image(grub_addr_t addr, grub_size_t size,
> char *args);
>
> +grub_efi_status_t
> +EXPORT_FUNC (grub_efi_load_image) (grub_efi_boolean_t boot_policy,
> +  grub_efi_handle_t parent_image_handle,
> +  grub_efi_device_path_t *file_path,
> +  void *source_buffer, grub_efi_uintn_t 
> source_size,
> +  grub_efi_handle_t *image_handle);
> +
> +grub_efi_status_t
> +EXPORT_FUNC (grub_efi_start_image) (grub_efi_handle_t image_handle,
> +   grub_efi_uintn_t *exit_data_size,
> +   grub_efi_char16_t **exit_data);
> +
> +grub_efi_status_t
> +EXPORT_FUNC (grub_efi_unload_image) (grub_efi_handle_t image_handle);
> +
> +typedef struct grub_efi_loader
> +{
> +  grub_efi_status_t (__grub_efi_api *load_image) (grub_efi_boolean_t 
> boot_policy,
> +  grub_efi_handle_t parent_image_handle,
> +  grub_efi_device_path_t *file_path,
> +  void *source_buffer,
> +  grub_efi_uintn_t source_size,
> +  grub_efi_handle_t *image_handle);
> +
> +  grub_efi_status_t (__grub_efi_api *start_image) (grub_efi_handle_t 
> image_handle,
> +   grub_efi_uintn_t *exit_data_size,
> +   grub_efi_char16_t **exit_data);
> +
> +  grub_efi_status_t (__grub_efi_api *unload_image) (grub_efi_handle_t 
> image_handle);
> +} grub_efi_loader_t;
> +
> +grub_err_t
> +EXPORT_FUNC (grub_efi_register_loader) (const grub_efi_loader_t *loader);
> +
> +grub_err_t
> +EXPORT_FUNC (grub_efi_unregister_loader) (const grub_efi_loader_t *loader);
> +
>  grub_addr_t grub_efi_section_addr (const char *section);
>
>  void grub_efi_mm_init (void);
> --
> 2.39.2
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



--
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH 00/15] UEFI NX support and NX Linux loader using shim loader protocol

[Vladimir 'phcoder' Serbinenko]

> Future work:
> - Completely disable non-NX compatible loaders when heap allocation are 
> marked NX.
>   This should be independent from the NX-enforcement flag and Secure Boot 
> status.
>   (Note that this is non-crtitical for security, as non-UEFI/Linux loaders 
> are disabled
>by SB lockdown, but would be nice to avoid crashes for unsuspecting users 
> on future
>hardware).
> - Implement NX in non-Linux loaders where applicable.

I think it applies to all loader actually and first part is unnecessarry.

Path number 15 is missing

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

Re: [PATCH 10/15] grub_dl_set_mem_attrs(): add self-check for the tramp/GOT sizes

[Vladimir 'phcoder' Serbinenko]

; (cherry picked from commit deb4c03d7658f0695db8217896c2830009b7469d)
> Signed-off-by: Jan Setje-Eilers 
> Signed-off-by: Mate Kukri 
> ---
>  grub-core/kern/dl.c | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c
> index 9f31ad3b9..042033a90 100644
> --- a/grub-core/kern/dl.c
> +++ b/grub-core/kern/dl.c
> @@ -645,7 +645,7 @@ grub_dl_set_mem_attrs (grub_dl_t mod, void *ehdr)
>  #if !defined (__i386__) && !defined (__x86_64__) && !defined(__riscv)
>grub_size_t arch_addralign = grub_arch_dl_min_alignment ();
>grub_addr_t tgaddr;
> -  grub_uint64_t tgsz;
> +  grub_size_t tgsz;
>  #endif
>
>grub_dprintf ("modules", "updating memory attributes for \"%s\"\n",
> @@ -698,6 +698,15 @@ grub_dl_set_mem_attrs (grub_dl_t mod, void *ehdr)
>
>grub_dprintf ("modules", "updating attributes for GOT and 
> trampolines\n",
> mod->name);
> +  if (tgaddr < (grub_addr_t)mod->base ||
> +  tgsz > (grub_addr_t)-1 - tgaddr ||
> + tgaddr + tgsz > (grub_addr_t)mod->base + mod->sz)
> +   return grub_error (GRUB_ERR_BUG,
> +  "BUG: trying to protect pages outside of module "
> +  "allocation (\"%s\"): module base %p, size 0x%"
> +  PRIxGRUB_SIZE "; tramp/GOT base 0x%" PRIxGRUB_ADDR
> +  ", size 0x%" PRIxGRUB_SIZE,
> +  mod->name, mod->base, mod->sz, tgaddr, tgsz);
>grub_update_mem_attrs (tgaddr, tgsz, GRUB_MEM_ATTR_R|GRUB_MEM_ATTR_X,
>  GRUB_MEM_ATTR_W);
>  }
> --
> 2.39.2
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH 09/15] grub_dl_load_segments(): page-align the tramp/GOT areas too

[Vladimir 'phcoder' Serbinenko]

> -  tsize += ALIGN_UP (tramp, GRUB_ARCH_DL_TRAMP_ALIGN);
> -  if (talign < GRUB_ARCH_DL_TRAMP_ALIGN)
> -talign = GRUB_ARCH_DL_TRAMP_ALIGN;
> -  tsize += ALIGN_UP (got, GRUB_ARCH_DL_GOT_ALIGN);
> -  if (talign < GRUB_ARCH_DL_GOT_ALIGN)
> -talign = GRUB_ARCH_DL_GOT_ALIGN;
> +  tramp_align = GRUB_ARCH_DL_TRAMP_ALIGN;
> +  if (tramp_align < arch_addralign)
> +tramp_align = arch_addralign;
> +  tsize += ALIGN_UP (tramp, tramp_align);
> +  if (talign < tramp_align)
> +talign = tramp_align;
> +  got_align = GRUB_ARCH_DL_GOT_ALIGN;
> +  if (got_align < arch_addralign)
> +got_align = arch_addralign;
> +  tsize += ALIGN_UP (got, got_align);
> +  if (talign < got_align)
> +talign = got_align;

Why not use grub_max for better readability?

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

Re: [PATCH 08/15] nx: set the nx compatible flag in EFI grub images

[Vladimir 'phcoder' Serbinenko]

Reviewed By: Vladimir Serbinenko

On Fri, May 24, 2024 at 2:06 PM Mate Kukri  wrote:
>
> From: Peter Jones 
>
> For NX, we need the grub binary to announce that it is compatible with
> the NX feature.  This implies that when loading the executable grub
> image, several attributes are true:
>
> - the binary doesn't need an executable stack
> - the binary doesn't need sections to be both executable and writable
> - the binary knows how to use the EFI Memory Attributes protocol on code
>   it is loading.
>
> This patch adds a definition for the PE DLL Characteristics flag
> GRUB_PE32_NX_COMPAT, and changes grub-mkimage to set that flag.
>
> Signed-off-by: Peter Jones 
> (cherry picked from commit a9ec858bd62b004c331cad9b5b00071d3081b626)
> Signed-off-by: Jan Setje-Eilers 
>
>  Conflicts:
> util/mkimage.c
>
> Signed-off-by: Mate Kukri 
> ---
>  util/mkimage.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/util/mkimage.c b/util/mkimage.c
> index 4237383ac..9b4720e21 100644
> --- a/util/mkimage.c
> +++ b/util/mkimage.c
> @@ -1403,6 +1403,7 @@ grub_install_generate_image (const char *dir, const 
> char *prefix,
>  #pragma GCC diagnostic push
>  #pragma GCC diagnostic ignored "-Wdangling-pointer"
>  #endif
> +   PE_OHDR (o32, o64, dll_characteristics) = grub_host_to_target16 
> (GRUB_PE32_NX_COMPAT);
> PE_OHDR (o32, o64, header_size) = grub_host_to_target32 (header_size);
> PE_OHDR (o32, o64, entry_addr) = grub_host_to_target32 
> (layout.start_address);
> PE_OHDR (o32, o64, image_base) = 0;
> --
> 2.39.2
>
>
> ___________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH 07/15] nx: set page permissions for loaded modules.

[Vladimir 'phcoder' Serbinenko]

= GRUB_MEM_ATTR_W;
> + clear_attrs &= ~GRUB_MEM_ATTR_W;
> +   }
> +
> +  if (s->sh_flags & SHF_EXECINSTR)
> +   {
> + set_attrs |= GRUB_MEM_ATTR_X;
> + clear_attrs &= ~GRUB_MEM_ATTR_X;
> +   }
> +
> +  grub_dprintf ("modules", "setting memory attrs for section \"%s\" to 
> -%s%s%s+%s%s%s\n",
> +   grub_dl_get_section_name(e, s),
> +   (clear_attrs & GRUB_MEM_ATTR_R) ? "r" : "",
> +   (clear_attrs & GRUB_MEM_ATTR_W) ? "w" : "",
> +   (clear_attrs & GRUB_MEM_ATTR_X) ? "x" : "",
> +   (set_attrs & GRUB_MEM_ATTR_R) ? "r" : "",
> +   (set_attrs & GRUB_MEM_ATTR_W) ? "w" : "",
> +   (set_attrs & GRUB_MEM_ATTR_X) ? "x" : "");
> +  grub_update_mem_attrs ((grub_addr_t)(seg->addr), seg->size, set_attrs, 
> clear_attrs);
> +}
> +
> +#if !defined (__i386__) && !defined (__x86_64__) && !defined(__riscv)
> +  tgaddr = grub_min((grub_addr_t)mod->tramp, (grub_addr_t)mod->got);
> +  tgsz = grub_max((grub_addr_t)mod->trampptr, (grub_addr_t)mod->gotptr) - 
> tgaddr;
> +
> +  if (tgsz)
> +{
> +  tgsz = ALIGN_UP(tgsz, arch_addralign);
> +
> +  grub_dprintf ("modules", "updating attributes for GOT and 
> trampolines\n",
> +   mod->name);
> +  grub_update_mem_attrs (tgaddr, tgsz, GRUB_MEM_ATTR_R|GRUB_MEM_ATTR_X,
> +GRUB_MEM_ATTR_W);
> +}
> +#endif
> +
> +  grub_dprintf ("modules", "done updating module memory attributes for 
> \"%s\"\n",
> +   mod->name);
> +
>return GRUB_ERR_NONE;
>  }
>
> @@ -660,6 +734,7 @@ grub_dl_load_core_noinit (void *addr, grub_size_t size)
>mod->ref_count = 1;
>
>grub_dprintf ("modules", "relocating to %p\n", mod);
> +
Needless hunk
>/* Me, Vladimir Serbinenko, hereby I add this module check as per new
>   GNU module policy. Note that this license check is informative only.
>   Modules have to be licensed under GPLv3 or GPLv3+ (optionally
> @@ -673,7 +748,8 @@ grub_dl_load_core_noinit (void *addr, grub_size_t size)
>|| grub_dl_resolve_dependencies (mod, e)
>|| grub_dl_load_segments (mod, e)
>|| grub_dl_resolve_symbols (mod, e)
> -  || grub_dl_relocate_symbols (mod, e))
> +  || grub_dl_relocate_symbols (mod, e)
> +  || grub_dl_set_mem_attrs (mod, e))
>  {
>mod->fini = 0;
>grub_dl_unload (mod);
> diff --git a/include/grub/dl.h b/include/grub/dl.h
> index 8a37cc16f..4c8a9b4ee 100644
> --- a/include/grub/dl.h
> +++ b/include/grub/dl.h
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #endif
>
>  /*
> @@ -254,6 +255,49 @@ grub_dl_is_persistent (grub_dl_t mod)
>return mod->persistent;
>  }
>
> +static inline const char *
> +grub_dl_get_section_name (const Elf_Ehdr *e, const Elf_Shdr *s)
> +{
> +  Elf_Shdr *str_s;
> +  const char *str;
> +
> +  str_s = (Elf_Shdr *) ((char *) e + e->e_shoff + e->e_shstrndx * 
> e->e_shentsize);
> +  str = (char *) e + str_s->sh_offset;
> +
> +  return str + s->sh_name;
> +}
> +
> +static inline long
> +grub_dl_find_section_index (Elf_Ehdr *e, const char *name)
> +{
> +  Elf_Shdr *s;
> +  const char *str;
> +  unsigned i;
> +
> +  s = (Elf_Shdr *) ((char *) e + e->e_shoff + e->e_shstrndx * 
> e->e_shentsize);
> +  str = (char *) e + s->sh_offset;
> +
> +  for (i = 0, s = (Elf_Shdr *) ((char *) e + e->e_shoff);
> +   i < e->e_shnum;
> +   i++, s = (Elf_Shdr *) ((char *) s + e->e_shentsize))
> +if (grub_strcmp (str + s->sh_name, name) == 0)
> +  return (long)i;
> +  return -1;
> +}
> +
> +/* Return the segment for a section of index N */
> +static inline grub_dl_segment_t
> +grub_dl_find_segment (grub_dl_t mod, unsigned n)
> +{
> +  grub_dl_segment_t seg;
> +
> +  for (seg = mod->segment; seg; seg = seg->next)
> +if (seg->section == n)
> +  return seg;
> +
> +  return NULL;
> +}
> +

I like those inlines

>  #endif
>
>  grub_err_t grub_dl_register_symbol (const char *name, void *addr,
> --
> 2.39.2
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH 06/15] nx: add memory attribute get/set API

[Vladimir 'phcoder' Serbinenko]

w' : '-',
> +   (after & GRUB_MEM_ATTR_X) ? 'x' : '-');
> +
> +  return grub_efi_status_to_err (efi_status);
> +}
> diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
> index d44d00ad7..b686e8afe 100644
> --- a/include/grub/efi/api.h
> +++ b/include/grub/efi/api.h
> @@ -379,6 +379,11 @@
>  {0xb6, 0xc7, 0x44, 0x0b, 0x29, 0xbb, 0x8c, 0x4f } \
>}
>
> +#define GRUB_EFI_MEMORY_ATTRIBUTE_PROTOCOL_GUID \
> +  { 0xf4560cf6, 0x40ec, 0x4b4a, \
> +{ 0xa1, 0x92, 0xbf, 0x1d, 0x57, 0xd0, 0xb1, 0x89 } \
> +  }
> +
>  struct grub_efi_sal_system_table
>  {
>grub_uint32_t signature;
> @@ -1815,4 +1820,24 @@ struct initrd_media_device_path {
>  } GRUB_PACKED;
>  typedef struct initrd_media_device_path initrd_media_device_path_t;
>
> +struct grub_efi_memory_attribute_protocol
> +{
> +  grub_efi_status_t (__grub_efi_api *get_memory_attributes) (
> +   struct grub_efi_memory_attribute_protocol *this,
> +   grub_efi_physical_address_t base_address,
> +   grub_efi_uint64_t length,
> +   grub_efi_uint64_t *attributes);
> +  grub_efi_status_t (__grub_efi_api *set_memory_attributes) (
> +   struct grub_efi_memory_attribute_protocol *this,
> +   grub_efi_physical_address_t base_address,
> +   grub_efi_uint64_t length,
> +   grub_efi_uint64_t attributes);
> +  grub_efi_status_t (__grub_efi_api *clear_memory_attributes) (
> +   struct grub_efi_memory_attribute_protocol *this,
> +   grub_efi_physical_address_t base_address,
> +   grub_efi_uint64_t length,
> +   grub_efi_uint64_t attributes);
> +};
> +typedef struct grub_efi_memory_attribute_protocol 
> grub_efi_memory_attribute_protocol_t;
> +
>  #endif /* ! GRUB_EFI_API_HEADER */
> diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
> index a5cd99e5a..194adc1f7 100644
> --- a/include/grub/efi/efi.h
> +++ b/include/grub/efi/efi.h
> @@ -151,4 +151,6 @@ struct grub_net_card;
>  grub_efi_handle_t
>  grub_efinet_get_device_handle (struct grub_net_card *card);
>
> +grub_err_t EXPORT_FUNC(grub_efi_status_to_err) (grub_efi_status_t status);
> +
>  #endif /* ! GRUB_EFI_EFI_HEADER */
> diff --git a/include/grub/mm.h b/include/grub/mm.h
> index f3bf87fa0..8ee1fc717 100644
> --- a/include/grub/mm.h
> +++ b/include/grub/mm.h
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>
>  #ifndef NULL
> @@ -56,6 +57,37 @@ void *EXPORT_FUNC(grub_realloc) (void *ptr, grub_size_t 
> size);
>  void *EXPORT_FUNC(grub_memalign) (grub_size_t align, grub_size_t size);
>  #endif
>
> +#define GRUB_MEM_ATTR_R0x0004LLU
> +#define GRUB_MEM_ATTR_W0x0002LLU
> +#define GRUB_MEM_ATTR_X0x0001LLU
> +
> +#ifdef GRUB_MACHINE_EFI
> +grub_err_t EXPORT_FUNC(grub_get_mem_attrs) (grub_addr_t addr,
> +   grub_size_t size,
> +   grub_uint64_t *attrs);
> +grub_err_t EXPORT_FUNC(grub_update_mem_attrs) (grub_addr_t addr,
> +          grub_size_t size,
> +  grub_uint64_t set_attrs,
> +  grub_uint64_t clear_attrs);
> +#else /* !GRUB_MACHINE_EFI */
> +static inline grub_err_t
> +grub_get_mem_attrs (grub_addr_t addr __attribute__((__unused__)),
> +   grub_size_t size __attribute__((__unused__)),
> +   grub_uint64_t *attrs __attribute__((__unused__)))
> +{
> +  return GRUB_ERR_NONE;
> +}
> +
> +static inline grub_err_t
> +grub_update_mem_attrs (grub_addr_t addr __attribute__((__unused__)),
> +  grub_size_t size __attribute__((__unused__)),
> +  grub_uint64_t set_attrs __attribute__((__unused__)),
> +  grub_uint64_t clear_attrs __attribute__((__unused__)))
> +{
> +  return GRUB_ERR_NONE;
> +}
> +#endif /* GRUB_MACHINE_EFI */
> +
>  void grub_mm_check_real (const char *file, int line);
>  #define grub_mm_check() grub_mm_check_real (GRUB_FILE, __LINE__);
>
> --
> 2.39.2
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH 05/15] modules: load module sections at page-aligned addresses

[Vladimir 'phcoder' Serbinenko]

What happens when 2 sections with the same NX and WR attributes follow
each other? Do we need to split them per page? How often do we
needlessly split in the current codebase?

> to dump into the ELF section header, which is often pretty useless.

I think this is not  true. It's not useless, just doesn't take the NX
requirement into account. I think it would be a better formulation

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

Re: [PATCH 04/15] pe: add the DOS header struct and fix some bad naming.

[Vladimir 'phcoder' Serbinenko]

>  struct grub_msdos_image_header
>  {
>/* This is always 'MZ'. (GRUB_PE32_MAGIC)  */
Please adjust this

> @@ -171,6 +194,8 @@ struct grub_pe32_optional_header
>struct grub_pe32_data_directory reserved_entry;
>  };
>
> +#define GRUB_PE32_NX_COMPAT 0x0100
> +
This was not mentioned in the description. Looks like 2 patches are conflated.

>  struct grub_pe64_optional_header
>  {
>grub_uint16_t magic;
> @@ -236,7 +261,11 @@ struct grub_pe64_optional_header
>  struct grub_pe32_section_table
>  {
>char name[8];
> -  grub_uint32_t virtual_size;
> +  union
> +{
> +  grub_uint32_t physical_address;
> +  grub_uint32_t virtual_size;
> +};
>grub_uint32_t virtual_address;
>grub_uint32_t raw_data_size;
>grub_uint32_t raw_data_offset;
> @@ -247,12 +276,18 @@ struct grub_pe32_section_table
>grub_uint32_t characteristics;
>  };
>
> +#define GRUB_PE32_SCN_TYPE_NO_PAD  0x0008
>  #define GRUB_PE32_SCN_CNT_CODE 0x0020
>  #define GRUB_PE32_SCN_CNT_INITIALIZED_DATA 0x0040
> -#define GRUB_PE32_SCN_MEM_DISCARDABLE  0x0200
> -#define GRUB_PE32_SCN_MEM_EXECUTE  0x2000
> -#define GRUB_PE32_SCN_MEM_READ 0x4000
> -#define GRUB_PE32_SCN_MEM_WRITE0x8000
> +#define GRUB_PE32_SCN_CNT_UNINITIALIZED_DATA   0x0080
> +#define GRUB_PE32_SCN_LNK_OTHER0x0100
> +#define GRUB_PE32_SCN_LNK_INFO 0x0200
> +#define GRUB_PE32_SCN_LNK_REMOVE   0x0800
> +#define GRUB_PE32_SCN_LNK_COMDAT   0x1000
> +#define GRUB_PE32_SCN_GPREL0x8000
> +#define GRUB_PE32_SCN_MEM_16BIT0x0002
> +#define GRUB_PE32_SCN_MEM_LOCKED   0x0004
> +#define GRUB_PE32_SCN_MEM_PRELOAD  0x0008
>
Again, not mentioned

>  #define GRUB_PE32_SCN_ALIGN_1BYTES 0x0010
>  #define GRUB_PE32_SCN_ALIGN_2BYTES 0x0020
> @@ -261,11 +296,28 @@ struct grub_pe32_section_table
>  #define GRUB_PE32_SCN_ALIGN_16BYTES0x0050
>  #define GRUB_PE32_SCN_ALIGN_32BYTES0x0060
>  #define GRUB_PE32_SCN_ALIGN_64BYTES0x0070
> +#define GRUB_PE32_SCN_ALIGN_128BYTES   0x0080
> +#define GRUB_PE32_SCN_ALIGN_256BYTES   0x0090
> +#define GRUB_PE32_SCN_ALIGN_512BYTES   0x00A0
> +#define GRUB_PE32_SCN_ALIGN_1024BYTES  0x00B0
> +#define GRUB_PE32_SCN_ALIGN_2048BYTES  0x00C0
> +#define GRUB_PE32_SCN_ALIGN_4096BYTES  0x00D0
> +#define GRUB_PE32_SCN_ALIGN_8192BYTES  0x00E0
>
>  #define GRUB_PE32_SCN_ALIGN_SHIFT  20
>  #define GRUB_PE32_SCN_ALIGN_MASK   7
>
> -#define GRUB_PE32_SIGNATURE_SIZE 4
> +#define GRUB_PE32_SCN_LNK_NRELOC_OVFL  0x0100
> +#define GRUB_PE32_SCN_MEM_DISCARDABLE  0x0200
> +#define GRUB_PE32_SCN_MEM_NOT_CACHED   0x0400
> +#define GRUB_PE32_SCN_MEM_NOT_PAGED0x0800
> +#define GRUB_PE32_SCN_MEM_SHARED   0x1000
> +#define GRUB_PE32_SCN_MEM_EXECUTE  0x2000
> +#define GRUB_PE32_SCN_MEM_READ 0x4000
> +#define GRUB_PE32_SCN_MEM_WRITE0x8000
> +
> +#define GRUB_PE32_SIGNATURE_SIZE   4
> +#define GRUB_PE32_SIGNATURE"PE\0\0"
>
>  #if GRUB_TARGET_SIZEOF_VOID_P == 8
>  #define GRUB_PE32_NATIVE_MAGIC GRUB_PE32_PE64_MAGIC
> @@ -290,6 +342,40 @@ struct grub_pe_image_header
>  #endif
>  };
>
> +struct grub_pe32_header
> +{
> +  /* This should be filled in with GRUB_PE32_MSDOS_STUB.  */
> +  grub_uint8_t msdos_stub[GRUB_PE32_MSDOS_STUB_SIZE];
> +
> +  /* This is always PE\0\0.  */
> +  char signature[GRUB_PE32_SIGNATURE_SIZE];
> +
> +  /* The COFF file header.  */
> +  struct grub_pe32_coff_header coff_header;
> +
> +#if GRUB_TARGET_SIZEOF_VOID_P == 8
> +  /* The Optional header.  */
> +  struct grub_pe64_optional_header optional_header;
> +#else
> +  /* The Optional header.  */
> +  struct grub_pe32_optional_header optional_header;
> +#endif
> +};
> +
> +struct grub_pe32_header_32
> +{
> +  char signature[GRUB_PE32_SIGNATURE_SIZE];
> +  struct grub_pe32_coff_header coff_header;
> +  struct grub_pe32_optional_header optional_header;
> +};
> +
> +struct grub_pe32_header_64
> +{
> +  char signature[GRUB_PE32_SIGNATURE_SIZE];
> +  struct grub_pe32_coff_header coff_header;
> +  struct grub_pe64_optional_header optional_header;
> +};
> +
>  struct grub_pe32_fixup_block
>  {
&g

Re: [PATCH 03/15] modules: Don't allocate space for non-allocable sections.

[Vladimir 'phcoder' Serbinenko]

Reviewed-By: Vladimir Serbinenko

On Fri, May 24, 2024 at 2:06 PM Mate Kukri  wrote:
>
> From: Peter Jones 
>
> Currently when loading grub modules, we allocate space for all sections,
> including those without SHF_ALLOC set.  We then copy the sections that
> /do/ have SHF_ALLOC set into the allocated memory, leaving some of our
> allocation untouched forever.  Additionally, on platforms with GOT
> fixups and trampolines, we currently compute alignment round-ups for the
> sections and sections with sh_size = 0.
>
> This patch removes the extra space from the allocation computation, and
> makes the allocation computation loop skip empty sections as the loading
> loop does.
>
> Signed-off-by: Peter Jones 
> (cherry picked from commit 0f76b53f2fe86542123c7aa1ae39c90852972a99)
> Signed-off-by: Jan Setje-Eilers 
> Signed-off-by: Mate Kukri 
> ---
>  grub-core/kern/dl.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c
> index 0bf40caa6..37db9fab0 100644
> --- a/grub-core/kern/dl.c
> +++ b/grub-core/kern/dl.c
> @@ -237,6 +237,9 @@ grub_dl_load_segments (grub_dl_t mod, const Elf_Ehdr *e)
> i < e->e_shnum;
> i++, s = (const Elf_Shdr *)((const char *) s + e->e_shentsize))
>  {
> +  if (s->sh_size == 0 || !(s->sh_flags & SHF_ALLOC))
> +   continue;
> +
>tsize = ALIGN_UP (tsize, s->sh_addralign) + s->sh_size;
>if (talign < s->sh_addralign)
> talign = s->sh_addralign;
> --
> 2.39.2
>
>
> ___________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH 02/15] modules: strip .llvm_addrsig sections and similar.

[Vladimir 'phcoder' Serbinenko]

Reviewed-By: Vladimir Serbinenko

On Fri, May 24, 2024 at 2:06 PM Mate Kukri  wrote:
>
> From: Peter Jones 
>
> Currently grub modules built with clang or gcc have several sections
> which we don't actually need or support.
>
> We already have a list of section to skip in genmod.sh, and this patch
> adds the following sections to that list (as well as a few newlines):
>
> .note.gnu.property
> .llvm*
>
> Note that the glob there won't work without a new enough linker, but the
> failure is just reversion to the status quo, so that's not a big problem.
>
> Signed-off-by: Peter Jones 
> (cherry picked from commit 0f66524e94d3c4f4d669d75c2122b0f1036776ea)
> Signed-off-by: Jan Setje-Eilers 
> Signed-off-by: Mate Kukri 
> ---
>  grub-core/genmod.sh.in | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/genmod.sh.in b/grub-core/genmod.sh.in
> index e57c4d920..337753c57 100644
> --- a/grub-core/genmod.sh.in
> +++ b/grub-core/genmod.sh.in
> @@ -57,8 +57,11 @@ if test x@TARGET_APPLE_LINKER@ != x1; then
> @TARGET_STRIP@ --strip-unneeded \
> -K grub_mod_init -K grub_mod_fini \
> -K _grub_mod_init -K _grub_mod_fini \
> -   -R .note.gnu.gold-version -R .note.GNU-stack \
> +   -R .note.GNU-stack \
> +   -R .note.gnu.gold-version \
> +   -R .note.gnu.property \
> -R .gnu.build.attributes \
> +   -R '.llvm*' \
> -R .rel.gnu.build.attributes \
> -R .rela.gnu.build.attributes \
> -R .eh_frame -R .rela.eh_frame -R .rel.eh_frame \
> --
> 2.39.2
>
>
> ___________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH 01/15] modules: make .module_license read-only

[Vladimir 'phcoder' Serbinenko]

Reviewed-By: Vladimir Serbinenko

On Fri, May 24, 2024 at 2:05 PM Mate Kukri  wrote:
>
> From: Peter Jones 
>
> Currently .module_license is set writable (that is, the section has the
> SHF_WRITE flag set) in the module's ELF headers.  This probably never
> actually matters, but it can't possibly be correct.
>
> This patch sets that data as "const", which causes that flag not to be
> set.
>
> Signed-off-by: Peter Jones 
> (cherry picked from commit f6563e15bb490bb76a1a95cd3648fe03d1134d14)
> Signed-off-by: Jan Setje-Eilers 
> Signed-off-by: Mate Kukri 
> ---
>  include/grub/dl.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/grub/dl.h b/include/grub/dl.h
> index cd1f46c8b..750fc8d3d 100644
> --- a/include/grub/dl.h
> +++ b/include/grub/dl.h
> @@ -119,7 +119,7 @@ grub_mod_fini (void)
>  #define ATTRIBUTE_USED __unused__
>  #endif
>  #define GRUB_MOD_LICENSE(license)  \
> -  static char grub_module_license[] __attribute__ ((section 
> (GRUB_MOD_SECTION (module_license)), ATTRIBUTE_USED)) = "LICENSE=" license;
> +  static const char grub_module_license[] __attribute__ ((section 
> (GRUB_MOD_SECTION (module_license)), ATTRIBUTE_USED)) = "LICENSE=" license;
>  #define GRUB_MOD_DEP(name) \
>  static const char grub_module_depend_##name[] \
>   __attribute__((section(GRUB_MOD_SECTION(moddeps)), ATTRIBUTE_USED)) = #name
> --
> 2.39.2
>
>
> ___________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH 1/3] Import libgcrypt 1.10.3

[Vladimir 'phcoder' Serbinenko]

I think at least AUTHORS and COPYING should be included. Rest is for the
ease of update in the future.

Le mar. 21 mai 2024, 10:32, Gary Lin  a écrit :

> Hi Vladimir,
>
> Originally, there are only cipher, mpi, and src in the libgcrypt
> directory, but the unnecessary stuff, e.g. AUTHORS, COPYING, build-aux,
> tests, etc., was added and bloated the size of the patch. Could you
> remove them and only keep the necessary files?
>
> Thanks,
>
> Gary Lin
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v14 1/3] safemath: Add ALIGN_UP_OVF() that checks for {over, under}flow

[Vladimir 'phcoder' Serbinenko]

Le ven. 17 mai 2024, 21:26, Gao Xiang  a
écrit :

> Hi Vladimir,
>
> On 2024/5/18 00:38, Vladimir 'phcoder' Serbinenko wrote:
> > I think that given that align is a non zero const we can trust it.
>
>  From the EROFS specific cases, they are always non-zero values,
> So I agree with you on this..
>
> Yet Daniel said ".. Be careful with underflows too." in
> https://lore.kernel.org/r/zkcv7g7mjbg8s...@tomti.i.net-space.pl
> although I'm not quite sure but I guess like this.
>
> Also as a generic helper, I think `align` could be zero if it's
> a variable..
>

It's rare for align to be a variable and then additional checks are needed
that align is a power of 2. And power of 2 is never zero. This check is
better done on the caller side. You just need a comment specify that it's
caller responsibility to check it.

>
> Anyway, either way works for EROFS.   I just try my best to do
> what I could do for this first series for upstreaming and we
> could move forward to the next step..
>
> Thanks,
> Gao Xiang
>
> >
> > Le ven. 17 mai 2024, 15:56, Gao Xiang  <mailto:hsiang...@linux.alibaba.com>> a écrit :
> >
> > The following EROFS patch will use this helper to handle overflow
> > ALIGN_UP() cases.
> >
> > Signed-off-by: Gao Xiang  hsiang...@linux.alibaba.com>>
> > ---
> >   include/grub/safemath.h | 16 
> >   1 file changed, 16 insertions(+)
> >
> > diff --git a/include/grub/safemath.h b/include/grub/safemath.h
> > index fbd9b5925..baaea0ef4 100644
> > --- a/include/grub/safemath.h
> > +++ b/include/grub/safemath.h
> > @@ -32,6 +32,22 @@
> >
> >   #define grub_cast(a, res)  grub_add ((a), 0, (res))
> >
> > +#define ALIGN_UP_OVF(v, align, res)\
> > +({ \
> > +  bool __failed;   \
> > +  typeof(v) a; \
> > +   \
> > +  __failed = grub_sub ((typeof(v))(align), 1, &a); \
> > +  if (__failed == false)   \
> > +{  \
> > +__failed = grub_add (v, a, res);   \
> > +if (__failed == false) \
> > +  {\
> > +*(res) &= ~a;  \
> > +  }\
> > +}  \
> > +__failed;})
> > +
> >   #else
> >   #error gcc 5.1 or newer or clang 8.0 or newer is required
> >   #endif
> > --
> > 2.39.3
> >
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v14 1/3] safemath: Add ALIGN_UP_OVF() that checks for {over, under}flow

[Vladimir 'phcoder' Serbinenko]

I think that given that align is a non zero const we can trust it.

Le ven. 17 mai 2024, 15:56, Gao Xiang  a
écrit :

> The following EROFS patch will use this helper to handle overflow
> ALIGN_UP() cases.
>
> Signed-off-by: Gao Xiang 
> ---
>  include/grub/safemath.h | 16 
>  1 file changed, 16 insertions(+)
>
> diff --git a/include/grub/safemath.h b/include/grub/safemath.h
> index fbd9b5925..baaea0ef4 100644
> --- a/include/grub/safemath.h
> +++ b/include/grub/safemath.h
> @@ -32,6 +32,22 @@
>
>  #define grub_cast(a, res)  grub_add ((a), 0, (res))
>
> +#define ALIGN_UP_OVF(v, align, res)\
> +({ \
> +  bool __failed;   \
> +  typeof(v) a; \
> +   \
> +  __failed = grub_sub ((typeof(v))(align), 1, &a); \
> +  if (__failed == false)   \
> +{  \
> +__failed = grub_add (v, a, res);   \
> +if (__failed == false) \
> +  {\
> +*(res) &= ~a;  \
> +  }\
> +}  \
> +__failed;})
> +
>  #else
>  #error gcc 5.1 or newer or clang 8.0 or newer is required
>  #endif
> --
> 2.39.3
>
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 3/3] keccak: Disable acceleration with SSE asm

[Vladimir 'phcoder' Serbinenko]

Le ven. 17 mai 2024, 14:15, Daniel Kiper  a écrit :

> Why is this patch needed? Should not we disable SSE using compiler flags?
>
We do but the code in question uses SSE on every x64 platform. So we need
to disable this acceleration. At least for now. If we want to enable SSE
(maybe we should) we can revisit this but this is a separate discussion and
separate patchset

>
> Daniel
>
> On Thu, May 16, 2024 at 09:27:43PM +0300, Vladimir Serbinenko wrote:
> > ---
> >  grub-core/lib/libgcrypt/cipher/keccak.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/grub-core/lib/libgcrypt/cipher/keccak.c
> b/grub-core/lib/libgcrypt/cipher/keccak.c
> > index 11e64b3e7..8b570263b 100644
> > --- a/grub-core/lib/libgcrypt/cipher/keccak.c
> > +++ b/grub-core/lib/libgcrypt/cipher/keccak.c
> > @@ -251,7 +251,7 @@ keccak_absorb_lane32bi(u32 *lane, u32 x0, u32 x1)
> >  /* Construct generic 64-bit implementation. */
> >  #ifdef USE_64BIT
> >
> > -#if __GNUC__ >= 4 && defined(__x86_64__)
> > +#if __GNUC__ >= 4 && defined(__x86_64__) && 0
> >
> >  static inline void absorb_lanes64_8(u64 *dst, const byte *in)
> >  {
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH 0/3] Upgrade to libgcrypt 1.10.3

[Vladimir 'phcoder' Serbinenko]

This series of patches upgrades to libgcrypt 1.10.3
First one just imports libgcrypt tarball as-is
Second one updates import script
Third one add a small adjustments to make it work with GRUB x64

-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH] elf: accept elf files without section header table

[Vladimir 'phcoder' Serbinenko]

Hey. Can you describe what exactly are you trying to solve?

Le ven. 3 mai 2024, 17:46, Mathieu Mirmont  a écrit :

> It isn't an error for an elf executable to be lacking a section header
> table. In this case we should just be returning shnum = 0.
>
> According to the elf(5) manual page:
> "
> e_shoff  This member holds the section header table's file offset in
>bytes. If the file has no section header table, this member
>holds zero.
> "
>
> Signed-off-by: Mathieu Mirmont 
> ---
>   grub-core/kern/elfXX.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/grub-core/kern/elfXX.c b/grub-core/kern/elfXX.c
> index aabf4b9d7..fe4ffb261 100644
> --- a/grub-core/kern/elfXX.c
> +++ b/grub-core/kern/elfXX.c
> @@ -220,12 +220,12 @@ grub_elfXX_get_shnum (ElfXX_Ehdr *e, ElfXX_Shnum
> *shnum)
> if (e == NULL)
>   return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed
> for elf header"));
>
> +  if (e->e_shoff == 0)
> +return GRUB_ERR_NONE;
> +
> *shnum = e->e_shnum;
> if (*shnum == SHN_UNDEF)
>   {
> -  if (e->e_shoff == 0)
> -
> return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid section header
> table offset in e_shoff"));
> -
> s = (ElfXX_Shdr *) ((grub_uint8_t *) e + e->e_shoff);
> *shnum = s->sh_size;
> if (*shnum < SHN_LORESERVE)
> --
> 2.30.2
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v8 1/2] fs/erofs: Add support for EROFS

[Vladimir 'phcoder' Serbinenko]

On Tue, Apr 30, 2024 at 4:27 PM Yifan Zhao  wrote:
>
>
>> +  err = grub_disk_read (disk, EROFS_SUPER_OFFSET >> GRUB_DISK_SECTOR_BITS, 
>> 0,
>> +   sizeof (sb), &sb);
>> +  if (grub_errno == GRUB_ERR_OUT_OF_RANGE)
>> +grub_error (GRUB_ERR_BAD_FS, "not a valid erofs filesystem");
>
> OUT_OF_RANGE is already treated the same asBAD_FS in context of opening a file
>>
>>
>
> Sorry, I didn't fully understand that. I copied similar logic from the ext2 
> driver (fs/ext2.c).
>
> Are you suggesting that I should remove the error code conversion here?
>

Yes. Since 2012 (commit bfb320c644e2a3ea463f8bbf72507b2f7b7132d0) this
logic is no longer necessary. ext2 has this logic back from 2009 and
probably should be removed by now

>
> All the other suggestions are great, I will reply in subsequent emails.
>
>
> Thanks,
>
> Yifan Zhao



-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH v8 1/2] fs/erofs: Add support for EROFS

[Vladimir 'phcoder' Serbinenko]

Generally looks good. Few comments. I have missed part of discussion, so
point me to relevant parts of it if I miss something that's already been
discussed

Le mer. 24 avr. 2024, 14:31, Yifan Zhao  a écrit :
De qq,

> +  struct grub_erofs_super *sb = &node->data->sb;
> +
> +  return (grub_le_to_cpu32 (sb->meta_blkaddr) << sb->log2_blksz) +
> (node->ino << EROFS_ISLOTBITS);
>
Here you have an overflow. You shift 32-bit value and so it's shifted as
32-bit and only then it's implicitly cast to 64-bit. You need an explicit
cast before shift but after bytesesp for meta_blkaddr.

+}
> +
> +static grub_err_t
> +erofs_read_inode (struct grub_erofs_data *data, grub_fshelp_node_t node)
> +{
> +  struct grub_erofs_inode_compact *dic;
> +  grub_err_t err;
> +  grub_uint16_t i_format;
> +  grub_uint64_t addr = erofs_iloc (node);
> +
> +  dic = (struct grub_erofs_inode_compact *) &node->inode;
>
Why not use union member here?


> +static grub_uint64_t
> +erofs_inode_file_size (grub_fshelp_node_t node)
> +{
> +  struct grub_erofs_inode_compact *dic = (struct grub_erofs_inode_compact
> *) &node->inode;
>
Ditto

>
>
+  grub_uint32_t blocksz = erofs_blocksz (node->data);
> +
> +  file_size = erofs_inode_file_size (node);
> +  nblocks = (file_size + blocksz - 1) >> node->data->sb.log2_blksz;
> +  lastblk = nblocks - tailendpacking;
> +
> +  map->m_flags = EROFS_MAP_MAPPED;
> +
> +  if (map->m_la < (lastblk * blocksz))
>
Is this multiplication checked somewhere?

+{
> +  if (grub_mul (grub_le_to_cpu32 (node->inode.i_u.raw_blkaddr),
> blocksz, &map->m_pa) ||
>
Missing cast to 64-bit.

+ grub_add (map->m_pa, map->m_la, &map->m_pa))
> +   return GRUB_ERR_OUT_OF_RANGE;
>
It looks like you miss a grub_error

+  if (grub_sub (lastblk * blocksz, map->m_la, &map->m_plen))
>
Ditto

> +   return GRUB_ERR_OUT_OF_RANGE;
>
Ditto. I stop reviewing this particular construct

+}
> +  else if (tailendpacking)
> +{
> +  if (grub_add (erofs_iloc (node), erofs_inode_size (node),
> &map->m_pa) ||
> + grub_add (map->m_pa, erofs_inode_xattr_ibody_size (node),
> &map->m_pa) ||
> + grub_add (map->m_pa, map->m_la % blocksz, &map->m_pa))
> +   return GRUB_ERR_OUT_OF_RANGE;
> +  if (grub_sub (file_size, map->m_la, &map->m_plen))
> +   return GRUB_ERR_OUT_OF_RANGE;
> +
> +  if (((map->m_pa % blocksz) + map->m_plen) > blocksz)
>
Addition not checked. If there is a reason it can't overflow even if FS is
maliciously corrupted, please add it in the comment

>
>
> +  pos = ALIGN_UP (pos, unit);
>
Potential overflow if pos is only slightly under it's limit.

+  if (grub_add (pos, chunknr * unit, &pos))
>
Does this multiplication need verification?

> +
> +  if (chunk_format & EROFS_CHUNK_FORMAT_INDEXES)
> +{
> +  struct grub_erofs_inode_chunk_index idx;
> +  grub_uint32_t blkaddr;
>
I recommend making it 64-bit to avoid casts

> +   {
> + map->m_pa = blkaddr << node->data->sb.log2_blksz;
>
Overflow again.

> +   {
> + map->m_pa = blkaddr << node->data->sb.log2_blksz;
>
Overflow

> +
> +  eend = grub_min (offset + size, map.m_la + map.m_llen);
>
Where is it checked that m_la+m_llen don't overflow? It can be checked when
you read in the  map it should be trimmed or error-out

> +  file_size = erofs_inode_file_size (dir);
> +  buf = grub_malloc (blocksz);
> +  if (!buf)
> +{
> +  grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
>

No need for grub_error: malloc already does it.

> + else
> +   de_namelen = grub_le_to_cpu16 (de[1].nameoff) - nameoff;
>

 This needs a check that it's not negative

> +
>
+  err = grub_disk_read (disk, EROFS_SUPER_OFFSET >> GRUB_DISK_SECTOR_BITS,
> 0,
> +   sizeof (sb), &sb);
> +  if (grub_errno == GRUB_ERR_OUT_OF_RANGE)
> +grub_error (GRUB_ERR_BAD_FS, "not a valid erofs filesystem");
>
OUT_OF_RANGE is already treated the same asBAD_FS in context of opening a
file

>
> +  if (!data)
> +{
> +  grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
>
Ditto

> +static grub_ssize_t
> +grub_erofs_read (grub_file_t file, char *buf, grub_size_t len)
> +{
> +  struct grub_erofs_data *data = file->data;
> +  struct grub_fshelp_node *inode = &data->inode;
> +  grub_off_t off = file->offset;
> +  grub_uint64_t ret = 0, file_size;
> +  grub_err_t err;
> +
> +  if (!inode->inode_loaded)
> +{
> +  err = erofs_read_inode (data, inode);
> +  if (err != GRUB_ERR_NONE)
> +   {
> + grub_error (GRUB_ERR_IO, "cannot read @ inode %"
> PRIuGRUB_UINT64_T, inode->ino);
>
Any reason to transform error? This could be something else than I/O error.
Why not just propagate it?


> +
> +  err = erofs_read_raw_data (inode, (grub_uint8_t *) buf, len, off, &ret);
> +  if (err != GRUB_ERR_NONE)
> +{
> +  grub_error (GRUB_ERR_IO, "cannot read file @ inode %"
> PRIuGRUB_UINT64_T, inode->ino);
>
Again, I propose to just propagate underlying error.


>
> +grub_size_t
> +g

Re: [PATCH v8 1/2] fs/erofs: Add support for EROFS

[Vladimir 'phcoder' Serbinenko]

Not a full review. Please don't put new functions like strnlen into kernel
of they are useful only to one module. Kernel size is restricted to a
different degree on some platforms

Le mer. 24 avr. 2024, 14:31, Yifan Zhao  a écrit :

> EROFS [1] is a lightweight read-only filesystem designed for performance
> which has already been shipped in most Linux distributions as well as
> widely
> used in several scenarios, such as Android system partitions, container
> images, and rootfs for embedded devices.
>
> This patch brings EROFS uncompressed support. Now, it's possible to boot
> directly through GRUB with an EROFS rootfs.
>
> EROFS compressed files will be supported later since it has more work to
> polish.
>
> [1] https://erofs.docs.kernel.org
>
> Signed-off-by: Yifan Zhao 
> ---
>  INSTALL |   8 +-
>  Makefile.util.def   |   1 +
>  docs/grub.texi  |   3 +-
>  grub-core/Makefile.core.def |   5 +
>  grub-core/fs/erofs.c| 984 
>  grub-core/kern/misc.c   |  14 +
>  include/grub/misc.h |   1 +
>  7 files changed, 1011 insertions(+), 5 deletions(-)
>  create mode 100644 grub-core/fs/erofs.c
>
> diff --git a/INSTALL b/INSTALL
> index 8d9207c84..84030c9f4 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -77,15 +77,15 @@ Prerequisites for make-check:
>
>  * If running a Linux kernel the following modules must be loaded:
>- fuse, loop
> -  - btrfs, ext4, f2fs, fat, hfs, hfsplus, jfs, mac-roman, minix, nilfs2,
> +  - btrfs, erofs, ext4, f2fs, fat, hfs, hfsplus, jfs, mac-roman, minix,
> nilfs2,
>  reiserfs, udf, xfs
>- On newer kernels, the exfat kernel modules may be used instead of the
>  exfat FUSE filesystem
>  * The following are Debian named packages required mostly for the full
>suite of filesystem testing (but some are needed by other tests as
> well):
> -  - btrfs-progs, dosfstools, e2fsprogs, exfat-utils, f2fs-tools, genromfs,
> -hfsprogs, jfsutils, nilfs-tools, ntfs-3g, reiserfsprogs,
> squashfs-tools,
> -reiserfsprogs, udftools, xfsprogs, zfs-fuse
> +  - btrfs-progs, dosfstools, e2fsprogs, erofs-utils, exfat-utils,
> f2fs-tools,
> +genromfs, hfsprogs, jfsutils, nilfs-tools, ntfs-3g, reiserfsprogs,
> +squashfs-tools, reiserfsprogs, udftools, xfsprogs, zfs-fuse
>- exfat-fuse, if not using the exfat kernel module
>- gzip, lzop, xz-utils
>- attr, cpio, g++, gawk, parted, recode, tar, util-linux
> diff --git a/Makefile.util.def b/Makefile.util.def
> index 9432365a9..8d3bc107f 100644
> --- a/Makefile.util.def
> +++ b/Makefile.util.def
> @@ -98,6 +98,7 @@ library = {
>common = grub-core/fs/cpio_be.c;
>common = grub-core/fs/odc.c;
>common = grub-core/fs/newc.c;
> +  common = grub-core/fs/erofs.c;
>common = grub-core/fs/ext2.c;
>common = grub-core/fs/fat.c;
>common = grub-core/fs/exfat.c;
> diff --git a/docs/grub.texi b/docs/grub.texi
> index a225f9a88..396f711df 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -353,6 +353,7 @@ blocklist notation. The currently supported filesystem
> types are @dfn{Amiga
>  Fast FileSystem (AFFS)}, @dfn{AtheOS fs}, @dfn{BeFS},
>  @dfn{BtrFS} (including raid0, raid1, raid10, gzip and lzo),
>  @dfn{cpio} (little- and big-endian bin, odc and newc variants),
> +@dfn{EROFS} (only uncompressed support for now),
>  @dfn{Linux ext2/ext3/ext4}, @dfn{DOS FAT12/FAT16/FAT32},
>  @dfn{exFAT}, @dfn{F2FS}, @dfn{HFS}, @dfn{HFS+},
>  @dfn{ISO9660} (including Joliet, Rock-ridge and multi-chunk files),
> @@ -6267,7 +6268,7 @@ assumed to be encoded in UTF-8.
>  NTFS, JFS, UDF, HFS+, exFAT, long filenames in FAT, Joliet part of
>  ISO9660 are treated as UTF-16 as per specification. AFS and BFS are read
>  as UTF-8, again according to specification. BtrFS, cpio, tar, squash4,
> minix,
> -minix2, minix3, ROMFS, ReiserFS, XFS, ext2, ext3, ext4, FAT (short names),
> +minix2, minix3, ROMFS, ReiserFS, XFS, EROFS, ext2, ext3, ext4, FAT (short
> names),
>  F2FS, RockRidge part of ISO9660, nilfs2, UFS1, UFS2 and ZFS are assumed
>  to be UTF-8. This might be false on systems configured with legacy charset
>  but as long as the charset used is superset of ASCII you should be able to
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 007ff628e..8b4dbcd66 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1442,6 +1442,11 @@ module = {
>common = fs/odc.c;
>  };
>
> +module = {
> +  name = erofs;
> +  common = fs/erofs.c;
> +};
> +
>  module = {
>name = ext2;
>common = fs/ext2.c;
> diff --git a/grub-core/fs/erofs.c b/grub-core/fs/erofs.c
> new file mode 100644
> index 0..13f92e71a
> --- /dev/null
> +++ b/grub-core/fs/erofs.c
> @@ -0,0 +1,984 @@
> +/* erofs.c - Enhanced Read-Only File System */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2024 Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or mo

Re: [PATCH 1/2] Allow "fallback" to include entries by title, not just number.

[Vladimir 'phcoder' Serbinenko]

Selecting by title is deprecated and kept only for compatibility. Please
use id instead

Le lun. 22 avr. 2024, 17:56, Marek Marczykowski-Górecki <
marma...@invisiblethingslab.com> a écrit :

> From: Peter Jones 
>
> Resolves: rhbz#1026084
>
> Signed-off-by: Peter Jones 
> ---
>  grub-core/normal/menu.c | 85 --
>  1 file changed, 58 insertions(+), 27 deletions(-)
>
> diff --git a/grub-core/normal/menu.c b/grub-core/normal/menu.c
> index 6a90e09..6444ee6 100644
> --- a/grub-core/normal/menu.c
> +++ b/grub-core/normal/menu.c
> @@ -163,15 +163,40 @@ grub_menu_set_timeout (int timeout)
>  }
>  }
>
> +static int
> +menuentry_eq (const char *id, const char *spec)
> +{
> +  const char *ptr1, *ptr2;
> +  ptr1 = id;
> +  ptr2 = spec;
> +  while (1)
> +{
> +  if (*ptr2 == '>' && ptr2[1] != '>' && *ptr1 == 0)
> +   return ptr2 - spec;
> +  if (*ptr2 == '>' && ptr2[1] != '>')
> +   return 0;
> +  if (*ptr2 == '>')
> +   ptr2++;
> +  if (*ptr1 != *ptr2)
> +   return 0;
> +  if (*ptr1 == 0)
> +   return ptr1 - id;
> +  ptr1++;
> +  ptr2++;
> +}
> +  return 0;
> +}
> +
>  /* Get the first entry number from the value of the environment variable
> NAME,
> which is a space-separated list of non-negative integers.  The entry
> number
> which is returned is stripped from the value of NAME.  If no entry
> number
> can be found, -1 is returned.  */
>  static int
> -get_and_remove_first_entry_number (const char *name)
> +get_and_remove_first_entry_number (grub_menu_t menu, const char *name)
>  {
>const char *val, *tail;
>int entry;
> +  int sz = 0;
>
>val = grub_env_get (name);
>if (! val)
> @@ -181,9 +206,39 @@ get_and_remove_first_entry_number (const char *name)
>
>entry = (int) grub_strtoul (val, &tail, 0);
>
> +  if (grub_errno == GRUB_ERR_BAD_NUMBER)
> +{
> +  /* See if the variable matches the title of a menu entry.  */
> +  grub_menu_entry_t e = menu->entry_list;
> +  int i;
> +
> +  for (i = 0; e; i++)
> +   {
> + sz = menuentry_eq (e->title, val);
> + if (sz < 1)
> +   sz = menuentry_eq (e->id, val);
> +
> + if (sz >= 1)
> +   {
> + entry = i;
> + break;
> +   }
> + e = e->next;
> +   }
> +
> +  if (sz > 0)
> +   grub_errno = GRUB_ERR_NONE;
> +
> +  if (! e)
> +   entry = -1;
> +}
> +
>if (grub_errno == GRUB_ERR_NONE)
>  {
> -  /* Skip whitespace to find the next digit.  */
> +  if (sz > 0)
> +   tail += sz;
> +
> +  /* Skip whitespace to find the next entry.  */
>while (*tail && grub_isspace (*tail))
> tail++;
>grub_env_set (name, tail);
> @@ -346,7 +401,7 @@ grub_menu_execute_with_fallback (grub_menu_t menu,
>grub_menu_execute_entry (entry, 1);
>
>/* Deal with fallback entries.  */
> -  while ((fallback_entry = get_and_remove_first_entry_number ("fallback"))
> +  while ((fallback_entry = get_and_remove_first_entry_number (menu,
> "fallback"))
>  >= 0)
>  {
>grub_print_error ();
> @@ -464,30 +519,6 @@ grub_menu_register_viewer (struct grub_menu_viewer
> *viewer)
>viewers = viewer;
>  }
>
> -static int
> -menuentry_eq (const char *id, const char *spec)
> -{
> -  const char *ptr1, *ptr2;
> -  ptr1 = id;
> -  ptr2 = spec;
> -  while (1)
> -{
> -  if (*ptr2 == '>' && ptr2[1] != '>' && *ptr1 == 0)
> -   return 1;
> -  if (*ptr2 == '>' && ptr2[1] != '>')
> -   return 0;
> -  if (*ptr2 == '>')
> -   ptr2++;
> -  if (*ptr1 != *ptr2)
> -   return 0;
> -  if (*ptr1 == 0)
> -   return 1;
> -  ptr1++;
> -  ptr2++;
> -}
> -}
> -
> -
>  /* Get the entry number from the variable NAME.  */
>  static int
>  get_entry_number (grub_menu_t menu, const char *name)
> --
> git-series 0.9.1
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 2/2] Fix menu entry selection based on ID and title

[Vladimir 'phcoder' Serbinenko]

I'm unsure whether an id starting with a numeral should be considered valid
at all. Variable named 0x would be incorrect

Le lun. 22 avr. 2024, 17:57, Marek Marczykowski-Górecki <
marma...@invisiblethingslab.com> a écrit :

> From: Peter Jones 
>
> Currently if grub_strtoul(saved_entry_value, NULL, 0) does not return an
> error, we assume the value it has produced is a correct index into our
> menu entry list, and do not try to interpret the value as the "id" or
> "title" .  In cases where "id" or "title" start with a numeral, this
> makes them impossible to use as selection criteria.
>
> This patch splits the search into three phases - matching id, matching
> title, and only once those have been exhausted, trying to interpret the
> ID as a numeral.  In that case, we also require that the entire string
> is numeric, not merely a string with leading numeric characters.
>
> Resolves: rhbz#1640979
>
> Signed-off-by: Peter Jones 
> [javierm: fix menu entry selection based on title]
> Signed-off-by: Javier Martinez Canillas 
> ---
>  grub-core/normal/menu.c | 141 -
>  1 file changed, 71 insertions(+), 70 deletions(-)
>
> diff --git a/grub-core/normal/menu.c b/grub-core/normal/menu.c
> index 6444ee6..b0cfa32 100644
> --- a/grub-core/normal/menu.c
> +++ b/grub-core/normal/menu.c
> @@ -164,12 +164,12 @@ grub_menu_set_timeout (int timeout)
>  }
>
>  static int
> -menuentry_eq (const char *id, const char *spec)
> +menuentry_eq (const char *id, const char *spec, int limit)
>  {
>const char *ptr1, *ptr2;
>ptr1 = id;
>ptr2 = spec;
> -  while (1)
> +  while (limit == -1 || ptr1 - id <= limit)
>  {
>if (*ptr2 == '>' && ptr2[1] != '>' && *ptr1 == 0)
> return ptr2 - spec;
> @@ -178,7 +178,11 @@ menuentry_eq (const char *id, const char *spec)
>if (*ptr2 == '>')
> ptr2++;
>if (*ptr1 != *ptr2)
> -   return 0;
> +   {
> + if (limit > -1 && ptr1 - id == limit && !*ptr1 &&
> grub_isspace(*ptr2))
> +   return ptr1 -id -1;
> + return 0;
> +   }
>if (*ptr1 == 0)
> return ptr1 - id;
>ptr1++;
> @@ -187,6 +191,58 @@ menuentry_eq (const char *id, const char *spec)
>return 0;
>  }
>
> +static int
> +get_entry_number_helper(grub_menu_t menu,
> +   const char * const val, const char ** const tail)
> +{
> +  /* See if the variable matches the title of a menu entry.  */
> +  int entry = -1;
> +  grub_menu_entry_t e;
> +  int i;
> +
> +  for (i = 0, e = menu->entry_list; e; i++)
> +{
> +  int l = 0;
> +  while (val[l] && !grub_isspace(val[l]))
> +   l++;
> +
> +  if (menuentry_eq (e->id, val, l))
> +   {
> + if (tail)
> +   *tail = val + l;
> + return i;
> +   }
> +  e = e->next;
> +}
> +
> +  for (i = 0, e = menu->entry_list; e; i++)
> +{
> +
> +  if (menuentry_eq (e->title, val, -1))
> +   {
> + if (tail)
> +   *tail = NULL;
> + return i;
> +   }
> +  e = e->next;
> +}
> +
> +  if (tail)
> +*tail = NULL;
> +
> +  entry = (int) grub_strtoul (val, tail, 0);
> +  if (grub_errno == GRUB_ERR_BAD_NUMBER ||
> +  (*tail && **tail && !grub_isspace(**tail)))
> +{
> +  entry = -1;
> +  if (tail)
> +   *tail = NULL;
> +  grub_errno = GRUB_ERR_NONE;
> +}
> +
> +  return entry;
> +}
> +
>  /* Get the first entry number from the value of the environment variable
> NAME,
> which is a space-separated list of non-negative integers.  The entry
> number
> which is returned is stripped from the value of NAME.  If no entry
> number
> @@ -196,7 +252,6 @@ get_and_remove_first_entry_number (grub_menu_t menu,
> const char *name)
>  {
>const char *val, *tail;
>int entry;
> -  int sz = 0;
>
>val = grub_env_get (name);
>if (! val)
> @@ -204,50 +259,24 @@ get_and_remove_first_entry_number (grub_menu_t menu,
> const char *name)
>
>grub_error_push ();
>
> -  entry = (int) grub_strtoul (val, &tail, 0);
> -
> -  if (grub_errno == GRUB_ERR_BAD_NUMBER)
> -{
> -  /* See if the variable matches the title of a menu entry.  */
> -  grub_menu_entry_t e = menu->entry_list;
> -  int i;
> -
> -  for (i = 0; e; i++)
> -   {
> - sz = menuentry_eq (e->title, val);
> - if (sz < 1)
> -   sz = menuentry_eq (e->id, val);
> -
> - if (sz >= 1)
> -   {
> - entry = i;
> - break;
> -   }
> - e = e->next;
> -   }
> +  entry = get_entry_number_helper(menu, val, &tail);
> +  if (!(*tail == 0 || grub_isspace(*tail)))
> +entry = -1;
>
> -  if (sz > 0)
> -   grub_errno = GRUB_ERR_NONE;
> -
> -  if (! e)
> -   entry = -1;
> -}
> -
> -  if (grub_errno == GRUB_ERR_NONE)
> +  if (entry >= 0)
>  {
> -  if (sz > 0)
> -   tail += sz;
> -
>/* Skip whitespace to find the next entry.  */
>while (*tail && grub_is

Re: [PATCH] windows: Add _stack_chk_guard/_stack_chk_fail symbols for Windows 64-bit target

[Vladimir 'phcoder' Serbinenko]

LGTM. Reviewed-by: Vladimir Serbinenko

Le mar. 9 avr. 2024, 20:56, Daniel Kiper  a écrit :

> Otherwise the GRUB cannot start when stack protector is enabled on EFI
> platforms.
>
> Signed-off-by: Daniel Kiper 
> ---
>  include/grub/stack_protector.h | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/include/grub/stack_protector.h
> b/include/grub/stack_protector.h
> index c88dc00b5..13d2657d9 100644
> --- a/include/grub/stack_protector.h
> +++ b/include/grub/stack_protector.h
> @@ -25,6 +25,10 @@
>  #ifdef GRUB_STACK_PROTECTOR
>  extern grub_addr_t EXPORT_VAR (__stack_chk_guard);
>  extern void __attribute__ ((noreturn)) EXPORT_FUNC (__stack_chk_fail)
> (void);
> +#if defined(_WIN64) && !defined(__CYGWIN__) /* MinGW, Windows 64-bit
> target. */
> +static grub_addr_t __attribute__ ((weakref("__stack_chk_guard")))
> EXPORT_VAR (_stack_chk_guard);
> +static void __attribute__ ((noreturn, weakref("__stack_chk_fail")))
> EXPORT_FUNC (_stack_chk_fail) (void);
> +#endif
>  #endif
>
>  #endif /* GRUB_STACK_PROTECTOR_H */
> --
> 2.11.0
>
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: New italian language for Grub 2.00-pre1 - 2.06-pre2 - 2.12-rc1

[Vladimir 'phcoder' Serbinenko]

Sorry but we don't manage translation repository. Please contact
translationproject. https://translationproject.org/team/it.html

On Fri, Apr 5, 2024 at 10:18 PM bovirus (gmail)  wrote:
>
> HI.
>
>
> Attached
>
>
>
> Italian translation for grub 2.06-pre1 - Translated 100%
>
> Italian translation for grub 2.06-pre2 - Translated 100%
>
> Italian translation for grub 2.12-rc1 - Translated 100%
>
>
>
> Please update in language repository.
>
>
>
> Thanks.
>
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH] STACK_PROTECTOR: Support symbols emitted by windows compiler

[Vladimir 'phcoder' Serbinenko]

I didn't know about using weakref for this but I'm fine with the approach.
Just one thing: can we condition it on HAVE_ASM_USCORE test instead of
platform?

Le jeu. 4 avr. 2024, 23:47, Daniel Kiper  a écrit :

> Adding Ard, Glenn and Dave...
>
> First of all, sorry for late reply but I was busy with other stuff...
>
> On Fri, Mar 15, 2024 at 09:43:22PM +0300, Vladimir 'phcoder' Serbinenko
> wrote:
> > stack protector needs symbols with just one underscore in C
> > name unlike unix variant that needs double underscore.
> > Supply both symbols for simplicity
> >
> > Signed-off-by: Vladimir Serbinenko 
>
> I do not like your solution because it adds unneeded code/symbols to
> some EFI builds. I come up with two other solutions.
>
> First, let's define an alias when using MinGW build environment:
>
> diff --git a/include/grub/stack_protector.h
> b/include/grub/stack_protector.h
> index c88dc00b5..8d99fd50e 100644
> --- a/include/grub/stack_protector.h
> +++ b/include/grub/stack_protector.h
> @@ -25,6 +25,10 @@
>  #ifdef GRUB_STACK_PROTECTOR
>  extern grub_addr_t EXPORT_VAR (__stack_chk_guard);
>  extern void __attribute__ ((noreturn)) EXPORT_FUNC (__stack_chk_fail)
> (void);
> +#if defined(_WIN32) && !defined(__CYGWIN__)
> +static grub_addr_t __attribute__ ((weakref("__stack_chk_guard")))
> EXPORT_VAR (_stack_chk_guard);
> +static void __attribute__ ((noreturn, weakref("__stack_chk_fail")))
> EXPORT_FUNC (_stack_chk_fail) (void);
> +#endif
>  #endif
>
>  #endif /* GRUB_STACK_PROTECTOR_H */
>
> Tested and it works. We have to use weakref() instead of alias() due to
> definition in separate translation unit.
>
> Second, do not drop leading underscore:
>
> diff --git a/conf/Makefile.common b/conf/Makefile.common
> index b8f216f6c..fd20f6b4e 100644
> --- a/conf/Makefile.common
> +++ b/conf/Makefile.common
> @@ -38,7 +38,7 @@ CCASFLAGS_DEFAULT = $(CPPFLAGS_DEFAULT) -DASM_FILE=1
>  BUILD_CPPFLAGS += $(CPPFLAGS_DEFAULT)
>
>  CFLAGS_KERNEL = $(CFLAGS_PLATFORM) -ffreestanding
> -LDFLAGS_KERNEL = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC)
> +LDFLAGS_KERNEL = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC)
> -Wl,--leading-underscore
>  CPPFLAGS_KERNEL = $(CPPFLAGS_CPU) $(CPPFLAGS_PLATFORM) -DGRUB_KERNEL=1
>  CCASFLAGS_KERNEL = $(CCASFLAGS_CPU) $(CCASFLAGS_PLATFORM)
>  STRIPFLAGS_KERNEL = -R .rel.dyn -R .reginfo -R .note -R .comment -R
> .drectve -R .note.gnu.gold-version -R .MIPS.abiflags -R .ARM.exidx
> @@ -51,12 +51,12 @@ endif
>  endif
>
>  CFLAGS_MODULE = $(CFLAGS_PLATFORM) -ffreestanding
> -LDFLAGS_MODULE = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC)
> -Wl,-r
> +LDFLAGS_MODULE = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC)
> -Wl,-r -Wl,--leading-underscore
>  CPPFLAGS_MODULE = $(CPPFLAGS_CPU) $(CPPFLAGS_PLATFORM)
>  CCASFLAGS_MODULE = $(CCASFLAGS_CPU) $(CCASFLAGS_PLATFORM)
>
>  CFLAGS_IMAGE = $(CFLAGS_PLATFORM) -fno-builtin
> -LDFLAGS_IMAGE = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC)
> -Wl,-S
> +LDFLAGS_IMAGE = $(LDFLAGS_PLATFORM) -nostdlib $(TARGET_LDFLAGS_OLDMAGIC)
> -Wl,-S -Wl,--leading-underscore
>  CPPFLAGS_IMAGE = $(CPPFLAGS_CPU) $(CPPFLAGS_PLATFORM)
>  CCASFLAGS_IMAGE = $(CCASFLAGS_CPU) $(CCASFLAGS_PLATFORM)
>
> diff --git a/grub-core/genmod.sh.in b/grub-core/genmod.sh.in
> index e57c4d920..04b680ad0 100644
> --- a/grub-core/genmod.sh.in
> +++ b/grub-core/genmod.sh.in
> @@ -83,9 +83,9 @@ else
>  for dep in $deps; do echo "char moddep_$dep[] __attribute__
> ((section(\"_moddeps, _moddeps\"))) = \"$dep\";" >>$t2; done
>
>  if test -n "$deps"; then
> -   @TARGET_CC@ @TARGET_LDFLAGS@ -ffreestanding -nostdlib -o
> $tmpfile2 $t1 $t2 $tmpfile -Wl,-r
> +   @TARGET_CC@ @TARGET_LDFLAGS@ -ffreestanding -nostdlib
> -Wl,--leading-underscore -o $tmpfile2 $t1 $t2 $tmpfile -Wl,-r
>  else
> -   @TARGET_CC@ @TARGET_LDFLAGS@ -ffreestanding -nostdlib -o
> $tmpfile2 $t1 $tmpfile -Wl,-r
> +   @TARGET_CC@ @TARGET_LDFLAGS@ -ffreestanding -nostdlib
> -Wl,--leading-underscore -o $tmpfile2 $t1 $tmpfile -Wl,-r
>  fi
>  rm -f $t1 $t2 $tmpfile
>  mv $tmpfile2 $tmpfile
> diff --git a/util/grub-pe2elf.c b/util/grub-pe2elf.c
> index 11331294f..233118252 100644
> --- a/util/grub-pe2elf.c
> +++ b/util/grub-pe2elf.c
> @@ -71,7 +71,7 @@ insert_string (const char *name)
>  {
>int len, result;
>
> -  if (*name == '_')
> +  if (*name == '_' && !strcmp("__stack_chk_fail", name) &&
> !strcmp("__stack_chk_guard", name))
>  name++;
>
>len = strlen (name);
>
> This one works too. However, it breaks normal builds. So, it requires some
> tweaking to make it production ready.
>
> Anyway, I prefer first solution because it is simpler. I do not mention
> that the second one may break other symbols with leading underscore.
>
> What do you think?
>
> Daniel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] fs/xfs: Handle non-continuous data blocks in directory extents

[Vladimir 'phcoder' Serbinenko]

On Sat, Feb 17, 2024 at 12:39 AM Jon DeVree  wrote:
>
> On Thu, Feb 15, 2024 at 20:33:13 +0300, Vladimir 'phcoder' Serbinenko wrote:
> > Is the pointer guaranteed to be aligned? If not you have to use unaligned
> > access.
>
> I don't know what alignment guarantees grub_malloc() makes. As long as
> they are close to what the regular C malloc() guarantees then it should
> be fine.

grub_malloc design guarantees the largest natural alignment of all types.
grub_malloc implementation guarantees alignment of 4 * sizeof(void*).

My concern was that the pointer is moved after alloc but it turns own
it was not the case.
So there are no unaligned access concerns

The code still violates cast-align and strict aliasing invariants but
so is the rest of XFS code.
So I'm happy to merge this patch

Reviewed-By: Vladimir Serbinenko 

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

[PATCH] Implement fmap support

[Vladimir 'phcoder' Serbinenko]

fmap (flash map) is a partition table used by coreboot. Support it in
order to retrieve the files in non-default partition

Signed-off-by: Vladimir Serbinenko 

-- 
Regards
Vladimir 'phcoder' Serbinenko


0001-Implement-fmap-support.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] grub-fstest: Add a new command zfs-bootfs.

[Vladimir 'phcoder' Serbinenko]

This is useful to check zfs-bootfs command.

Signed-off-by: Vladimir Serbinenko 

-- 
Regards
Vladimir 'phcoder' Serbinenko


0001-grub-fstest-Add-a-new-command-zfs-bootfs.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Improve cbfs detection

[Vladimir 'phcoder' Serbinenko]

With FMAP and non-continuous SPI old way of reading CBFS pointer
is no longer reliable. Hence use new lbio tags to detect the correct
cbfs layout

Signed-off-by: Vladimir Serbinenko 

-- 
Regards
Vladimir 'phcoder' Serbinenko


0001-Improve-cbfs-detection.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Add commands for reading and writing raw bytes to CMOS

[Vladimir 'phcoder' Serbinenko]

With some coreboot configs setting a byte to a magic value
changes behaviour on next boot. Setting bit-by-bit is
possible but not convenient. Add cmosread and cmoswrite for
convenience.

Signed-off-by: Vladimir Serbinenko 

-- 
Regards
Vladimir 'phcoder' Serbinenko


0001-Add-commands-for-reading-and-writing-raw-bytes-to-CM.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Enable cmos on x86 EFI

[Vladimir 'phcoder' Serbinenko]

CMOS actually exists on most EFI platforms and in some cases used to
store useful data that makes it useful for GRUB to read/write it

As for datetime keep using EFI API and not CMOS

Signed-off-by: Vladimir Serbinenko 

-- 
Regards
Vladimir 'phcoder' Serbinenko


0001-Enable-cmos-on-x86-EFI.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v2] acpi: Mark MADT entries as packed.

[Vladimir 'phcoder' Serbinenko]

No alignment is guaranteed and in fact on my ia64 sapic is aligned
to 4 bytes instead of 8 and causes a trap. It affects only rarely used
lsacpi command and so went unnoticed.

Signed-off-by: Vladimir Serbinenko 

-- 
Regards
Vladimir 'phcoder' Serbinenko


0001-acpi-Mark-MADT-entries-as-packed.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v2] MULTIBOOT: Fix handling of errors in broken aout-kludge

[Vladimir 'phcoder' Serbinenko]

Current code in some codepaths neither discards nor reports
errors. Properly surface the error

While on it split 2 cases of unrelated variables both named err.

Signed-off-by: Vladimir Serbinenko 

-- 
Regards
Vladimir 'phcoder' Serbinenko


0001-MULTIBOOT-Fix-handling-of-errors-in-broken-aout-klud.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] STACK_PROTECTOR: Support symbols emitted by windows compiler

[Vladimir 'phcoder' Serbinenko]

stack protector needs symbols with just one underscore in C
name unlike unix variant that needs double underscore.
Supply both symbols for simplicity

Signed-off-by: Vladimir Serbinenko 

-- 
Regards
Vladimir 'phcoder' Serbinenko


0001-STACK_PROTECTOR-Support-symbols-emitted-by-windows-c.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 2/6] ieee1275/powerpc: enables device mapper discovery

[Vladimir 'phcoder' Serbinenko]

On Mon, May 8, 2023 at 5:01 PM Avnish Chouhan  wrote:
>
> From: Diego Domingos 
>
> This patch enables the device mapper discovery on ofpath.c. Currently,
> when we are dealing with a device like /dev/dm-* the ofpath returns null
> since there is no function implemented to handle this case.
Why is this result incorrect? Does your OFW handle device-mapper? If
it doesn't then the answer to "what is the OFW path of dm-*?" is
"none" and null is the correct return

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

Re: [PATCH 3/7] multiboot2: Add support for the load type header tag

[Vladimir 'phcoder' Serbinenko]

Not a full review. Just one blocking problem


>
>  }
> +  case MULTIBOOT_LOAD_TYPE_PE:
> +  grub_fatal ("Unsupported load type: %u\n", mld.load_type);
> +  default:
> +/* should be impossible */
> +grub_fatal ("Unknown load type: %u\n", mld.load_type);
>
Don't use grub_fatal for this. grub_fatal is only when continue to execute
grub is unwise. Here you just have an unsupported file. This is definitely
a GRUB_ERR_BAD_OS


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

Re: [PATCH 7/7] verifiers: Verify after decompression

[Vladimir 'phcoder' Serbinenko]

Verifying after decompression is a bad security practice. It relies on
decompression having no security holes. Given how complex decompression is,
this is almost guaranteed to be false.

Le mer. 13 mars 2024, 18:08, Ross Lagerwall via Grub-devel <
grub-devel@gnu.org> a écrit :

> It is convenient and common to have binaries stored in gzip archives
> (e.g. xen.gz). Verification should be run after decompression rather
> than before so reorder the file filter list as appropriate.
>
> Signed-off-by: Ross Lagerwall 
> ---
>  include/grub/file.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/grub/file.h b/include/grub/file.h
> index a5bf3a792d6f..a1ef3582bc7b 100644
> --- a/include/grub/file.h
> +++ b/include/grub/file.h
> @@ -182,10 +182,10 @@ extern grub_disk_read_hook_t
> EXPORT_VAR(grub_file_progress_hook);
>  /* Filters with lower ID are executed first.  */
>  typedef enum grub_file_filter_id
>{
> -GRUB_FILE_FILTER_VERIFY,
>  GRUB_FILE_FILTER_GZIO,
>  GRUB_FILE_FILTER_XZIO,
>  GRUB_FILE_FILTER_LZOPIO,
> +GRUB_FILE_FILTER_VERIFY,
>  GRUB_FILE_FILTER_MAX,
>  GRUB_FILE_FILTER_COMPRESSION_FIRST = GRUB_FILE_FILTER_GZIO,
>  GRUB_FILE_FILTER_COMPRESSION_LAST = GRUB_FILE_FILTER_LZOPIO,
> --
> 2.43.0
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/7] multiboot2: Add load type header and support for the PE binary type

[Vladimir 'phcoder' Serbinenko]

Please take this discussion to a separate thread. In this thread it's
pure offtopic

On Thu, Mar 14, 2024 at 10:25 AM Jan Beulich via Grub-devel
 wrote:
>
> On 13.03.2024 16:07, Ross Lagerwall wrote:
> > In addition to the existing address and ELF load types, specify a new
> > optional PE binary load type. This new type is a useful addition since
> > PE binaries can be signed and verified (i.e. used with Secure Boot).
>
> And the consideration to have ELF signable (by whatever extension to
> the ELF spec) went nowhere?
>
> Jan
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH 0/4] More ls improvements

[Vladimir 'phcoder' Serbinenko]

Le ven. 6 oct. 2023, 10:25, Glenn Washburn  a
écrit :

> On Thu, 14 Sep 2023 15:08:00 -0500
> Glenn Washburn  wrote:
>
> > On Thu, 14 Sep 2023 17:37:10 +0200
> > "Vladimir 'phcoder' Serbinenko"  wrote:
> >
> > > Le lun. 14 août 2023, 20:58, Glenn Washburn <
> developm...@efficientek.com> a
> > > écrit :
> > >
> > > > Currently when given a path to a file, ls will open the file to
> determine
> > > > if its is valid and then run the appropriate print function, in
> contrast to
> > > > directory arguments that use the directory iterator and callback on
> each
> > > > file. One issue with this is that opening a file does not allow
> access to
> > > > its modification time information, whereas the info object from the
> > > > callback
> > > > called by the directory iterator does and the longlist print
> function will
> > > > print the modification time if present. The result is that when
> longlisting
> > > > ls arguments, directory arguments show moditication times but file
> > > > arguments
> > > > do not. Patch 2 rectifies this an in the process simplifies the code
> path
> > > > by using the directory iterator for file arguments as well.
> > > >
> > > > The implementation of patch 2 exposed a bug in grub_disk_read()
> which is
> > > > fixed in patch 1.
> > > >
> > > > Patches 3 and 4 aim to make the output of GRUB's ls look more like
> GNU's
> > > > ls output. And patch 4 also fixes an issue where there are blank
> lines
> > > > between consecutive file arguments.
> > > >
> > > > Glenn
> > > >
> > > > Glenn Washburn (4):
> > > >   disk: Reset grub_errno upon entering grub_disk_read()
> > > >
> > > Where does the error come from? We generally prefer to have
> > > grub_print_error() (better) or resetting grib_errno after the error is
> > > produced rather than blanketly reset grub_errno at the beginning
> >
> > Take a look at patch 2, you'll see that the changes add another
> > (fs->fs_dir)(...). This added line is in an "if" block that is only
> > entered when grub_errno is not GRUB_ERR_SUCCESS. So under normal and
> > expected conditions, grub_errno will be set (when there are
> > non-directory arguments). This error is more of a flag than a real
> > error that should be shown the user.
> >
> > The preferred usage policy of grub_errno that you suggest is aligned
> > with "man errno". So it has that going for it. I don't particularly
>
> Actually, I take that back. My reading of the man page leads me to
> conclude that library calls can do what they wish with errno, so I
> think clearing it as in this patch is perfectly reasonable. The man
> page states, if the errno needs to be used across another library call,
> then it must be saved. The same should be expected of grub calls. I'll
> admit its a bit of an apples and oranges comparison, but this makes
> sense to me.
>


While grub_errno is named after posix, it's not designed in the same way.
It's more in line with the design of exceptions. I agree that this
situation is unfortunate and ideally we need to change it centrally. Let me
start a separate thread with possible options for this.

>
> > like it because, for one, I've seen odd read failures in the past that
> > I suspect are due to read returning with an error, when it actually
> > succeed. So it can give false positives that doesn't make sense and are
> > hard to track down. I see a few options here:
> >
> > 1. Have read() return err, instead of grub_errno, but that has a couple
> > problems. First, it probably will then ignore error from the cache
> > code. And second, I've not checked for usages of read() where
> > grub_errno is checked instead of the return value for error, but those
> > might exist as well.
> >
> > 2. Set grub_errno before every call to read(). But then that's not
> > really different that doing that at the start of the function.
> >
> > 3. A compromise option could be to output to the debug log a message
> > saying that read was called with grub_errno set. But that can easily be
> > overflowed, so it might not be noticed. And even if it is seen, what you
> > really care about is the caller, but is there a good way to get that
> > without having a debugger already attached? When the backtrace
> > functionality works again (perhaps soon), that could be used, but 

Re: Need help: alternative module inclusion - duplicate symbols

[Vladimir 'phcoder' Serbinenko]

You can add a dispatcher which switches between 2 modules. E.g.
real_do_stuff (...);
fake_do_stuff(...);

static int (*do_stuff)( ...);
And switch pointer between 2 implementations. See terminal output and
grub_xputs for examples

On Mon, Feb 26, 2024 at 1:51 PM Michael Lawnick via Grub-devel
 wrote:
>
> Hi group,
>
> hope you can help me:
> I have modules pci_fpga.c and pci_fpga_emul.c with same functions in
> them but different implementation, one for the real device, the other
> one is just emulating.
>
> What I now want is being able to include one of both versions through
> build command. I started with this:
>
> Makefile.core.def:
> ...
> module = {
>   name = pci_fpga;
>   common = startlib/board/pci_fpga.c;
>   enable = efi;
> };
>
> module = {
>   name = pci_fpga_emul;
>   common = startlib/board/pci_fpga_emul.c;
>   enable = efi;
> };
> ...
>
> but then I get duplicate symbols error even without giving pci_fpga or
> pci_fpga_emul on the build.
>
> For using different packages than 'common' like this
> module = {
>   name = pci_fpga;
>   pci_fpga = startlib/board/pci_fpga.c;
>   enable = efi;
> };
> I could not find means to get pci_fpga included.
>
> Same problem if trying to approach it via different enable flag: How to do?
>
> Can anybody help? In docs I couldn't find usable hints.
>
> --
> KR
> Michael
>
> ___________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH] efidisk: Breakup large reads into batches

[Vladimir 'phcoder' Serbinenko]

Why is setting max_agglomerate not enough to achieve the same effect?

On Fri, Feb 23, 2024 at 8:12 PM Mate Kukri  wrote:
>
> From: David F 
>
> Work around firmware bugs that cause large reads to fail from certain
> devices.
>
> Report-By: David F 
> Signed-off-by: Mate Kukri 
> ---
>  grub-core/disk/efi/efidisk.c | 43 +---
>  1 file changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
> index 3b5ed5691..079b72ab7 100644
> --- a/grub-core/disk/efi/efidisk.c
> +++ b/grub-core/disk/efi/efidisk.c
> @@ -27,6 +27,9 @@
>  #include 
>  #include 
>
> +/* break up io to prevent problems under some UEFI envrionments */
> +#define EFIDISK_C_MAXIOSECS 0x100U
> +
>  struct grub_efidisk_data
>  {
>grub_efi_handle_t handle;
> @@ -599,20 +602,34 @@ grub_efidisk_read (struct grub_disk *disk, 
> grub_disk_addr_t sector,
>grub_size_t size, char *buf)
>  {
>grub_efi_status_t status;
> +  grub_size_t sector_count;
>
> -  grub_dprintf ("efidisk",
> -   "reading 0x%lx sectors at the sector 0x%llx from %s\n",
> -   (unsigned long) size, (unsigned long long) sector, 
> disk->name);
> -
> -  status = grub_efidisk_readwrite (disk, sector, size, buf, 0);
> -
> -  if (status == GRUB_EFI_NO_MEDIA)
> -return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("no media in `%s'"), 
> disk->name);
> -  else if (status != GRUB_EFI_SUCCESS)
> -return grub_error (GRUB_ERR_READ_ERROR,
> -  N_("failure reading sector 0x%llx from `%s'"),
> -  (unsigned long long) sector,
> -  disk->name);
> +  /* break up reads to EFIDISK_C_MAXIOSECS size chunks */
> +  do
> +{
> +  /* determine number of sectors this cycle */
> +  sector_count = (size > EFIDISK_C_MAXIOSECS) ? EFIDISK_C_MAXIOSECS : 
> size;
> +
> +  /* output debug information */
> +  grub_dprintf ("efidisk",
> +   "reading 0x%lx sectors at the sector 0x%llx from %s\n",
> +   (unsigned long) sector_count, (unsigned long long) 
> sector, disk->name);
> +
> +  status = grub_efidisk_readwrite (disk, sector, sector_count, buf, 0);
> +
> +  if (status == GRUB_EFI_NO_MEDIA)
> +   return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("no media in `%s'"), 
> disk->name);
> +  if (status != GRUB_EFI_SUCCESS)
> +   return grub_error (GRUB_ERR_READ_ERROR,
> +  N_("failure reading 0x%lx sector(s) from sector 
> 0x%llx on `%s'"),
> +  (unsigned long) sector_count, (unsigned long long) 
> sector, disk->name);
> +
> +  /* next cycle */
> +  buf += (grub_efi_uintn_t) sector_count << disk->log_sector_size;;
> +  sector += sector_count;
> +  size -= sector_count;
> +}
> +  while (size);
>
>return GRUB_ERR_NONE;
>  }
> --
> 2.39.2
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: Failed to build with "-mretpoline"

[Vladimir 'phcoder' Serbinenko]

Hello. Why does it create duplicate global symbols? How does LD handle the
same situation?

Le ven. 9 févr. 2024, 16:58, Oleksiy Obitotskyi -X (oobitots - GLOBALLOGIC
INC at Cisco) via Grub-devel  a écrit :

> Hi,
>
> We are bulding grub with clang that use -mretpoline option. This leads to
> symbols __llvm_retpoline_[eax/ecx/edx, etc.] generated into binaries. And
> caused  duplicates error during moddep.lst creation.
> I'm not sure if this fix will be a bit project specific?
>
> diff --git a/grub-core/genmoddep.awk b/grub-core/genmoddep.awk
> index 2474363..f859854 100644
> --- a/grub-core/genmoddep.awk
> +++ b/grub-core/genmoddep.awk
> @@ -18,7 +18,7 @@ BEGIN {
> {
> if ($1 == "defined") {
> - if ($3 !~ /^\.refptr\./ && $3 in symtab) {
> + if ($3 !~ /__llvm_retpoline_.*/ && $3 !~ /^\.refptr\./ && $3 in symtab) {
> printf "%s in %s is duplicated in %s\n", $3, $2, symtab[$3] >"/dev/stderr";
> error++;
> }
>
> Regards,
> Oleksiy
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] MULTIBOOT: Fix handling of errors in broken aout-kludge

[Vladimir 'phcoder' Serbinenko]

-- 
Regards
Vladimir 'phcoder' Serbinenko


0001-MULTIBOOT-Fix-handling-of-errors-in-broken-aout-klud.patch
Description: Binary data
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] fs/xfs: Handle non-continuous data blocks in directory extents

[Vladimir 'phcoder' Serbinenko]

Is the pointer guaranteed to be aligned? If not you have to use unaligned
access. Any reason not to check it against correct magic and not just
zero-out?

Le dim. 11 févr. 2024, 18:36, Jon DeVree  a écrit :

> The directory extent list does not have to be a continuous list of data
> blocks. When GRUB tries to read a non-existant member of the list,
> grub_xfs_read_file() will return a block of zero'ed memory. Checking for
> a zero'ed magic number is sufficient to skip this non-existant data
> block.
>
> Prior to commit 07318ee7e this was handled as a subtle side effect of
> reading the (non-existant) tail data structure. Since the block was
> zero'ed the computation of the number of directory entries in the block
> would return 0 as well.
>
> Fixes: 07318ee7e (fs/xfs: Fix XFS directory extent parsing)
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2254370
> Signed-off-by: Jon DeVree 
> ---
>  grub-core/fs/xfs.c | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
> index bc2224dbb..a6cce9cfe 100644
> --- a/grub-core/fs/xfs.c
> +++ b/grub-core/fs/xfs.c
> @@ -902,6 +902,7 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
> grub_xfs_first_de(dir->data,
> dirblock);
> int entries = -1;
> char *end = dirblock + dirblk_size;
> +   grub_uint32_t magic;
>
> numread = grub_xfs_read_file (dir, 0, 0,
>   blk << dirblk_log2,
> @@ -912,6 +913,15 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
> return 0;
>   }
>
> +   /*
> +* If this data block isn't actually part of the extent list
> then
> +* grub_xfs_read_file() returns a block of zeros. So if the
> magic number
> +* field is all zeros then this block should be skipped.
> +*/
> +   magic = *(grub_uint32_t *)dirblock;
> +   if (!magic)
> + continue;
> +
> /*
>  * Leaf and tail information are only in the data block if the
> number
>  * of extents is 1.
> --
> 2.43.0
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Critical Boot Loader Vulnerability in Shim Impacts Nearly All Linux Distros

[Vladimir 'phcoder' Serbinenko]

Shim is not a component of grub

Le ven. 9 févr. 2024, 07:41, Turritopsis Dohrnii Teo En Ming via Grub-devel
 a écrit :

> Subject: Critical Boot Loader Vulnerability in Shim Impacts Nearly All
> Linux Distros
>
> Good day from Singapore,
>
> I have just come across this article.
>
> Article: Critical Boot Loader Vulnerability in Shim Impacts Nearly All
> Linux Distros
> Link:
> https://thehackernews.com/2024/02/critical-bootloader-vulnerability-in.html
>
> May I know if Shim is an important component of GNU Grub?
>
> Thank you.
>
> Regards,
>
> Mr. Turritopsis Dohrnii Teo En Ming
> Targeted Individual in Singapore
> Blogs:
> https://tdtemcerts.blogspot.com
> https://tdtemcerts.wordpress.com
> GIMP also stands for Government-Induced Medical Problems.
>
>
>
>
>
>
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 2/2] disk: increase sector size up to 127 for LBA reads

[Vladimir 'phcoder' Serbinenko]

I have few concerns:
1. The name of your patch is misleading. You don't increase sector size
2. This increases memory pressure on systems that are already low on usable
memory. Sparc64 comes to mind. Due to the way how off handles memory map
there, even if you have a terabyte of RAM, only few MiB are usable at boot.
We may need to do it dynamically, otherwise we either leave a lot of gain
on systems that can read megabytes at once or we fail on low memory systems
3. Firmware bugs. Not even just BIOS. This changes all the systems. We need
to review all the drivers
4. You mentioned that other implementations that increase the size of
single read have a fallback. Do we have one?

Le mar. 10 oct. 2023, 21:30, ValdikSS via Grub-devel  a
écrit :

> Increase the value from 63 to speed up reading process.
>
> This commit increases two limits: the low-level int 13h reading code
> and a high-level reading code with disk cache.
> The disk cache imposes an overall limitation of a higher-layer reading
> code. The original comment regarding 16K is incorrect, it was
> 512<<6 = 32768, and now it is 512<<7 = 65536.
>
> According to [Wikipedia][1] and [OSDev][2], the upper safe value for LBA
> read using IBM/MS INT13 Extensions is 127 sectors due to the
> limitations of some BIOSes.
> GRUB [already enforced][3] the limit, but it was no-op due to other
> constraints. This value is also used in [syslinux][4].
>
> As we're now reading up to 127 sectors of 512 bytes, we need to be able
> to store in the cache up to 65024 bytes. Without this change, GRUB
> wouldn't try to read more than 64 sectors at once
> (even if the lower reading layer allows it).
>
> [1]:
> https://en.wikipedia.org/wiki/INT_13H#INT_13h_AH=42h:_Extended_Read_Sectors_From_Drive
> [2]:
> https://wiki.osdev.org/Disk_access_using_the_BIOS_(INT_13h)#LBA_in_Extended_Mode
> [3]:
> https://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/disk/i386/pc/biosdisk.c?h=grub-2.12-rc1#n430
> [4]:
> https://repo.or.cz/syslinux.git/blob/refs/tags/syslinux-6.03:/core/fs/diskio_bios.c#l349
>
> Signed-off-by: ValdikSS 
> ---
>  include/grub/disk.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/grub/disk.h b/include/grub/disk.h
> index be032a72c..608deb034 100644
> --- a/include/grub/disk.h
> +++ b/include/grub/disk.h
> @@ -184,14 +184,14 @@ typedef struct grub_disk_memberlist
> *grub_disk_memberlist_t;
>  #define GRUB_MDRAID_MAX_DISKS  4096
>
>  /* The size of a disk cache in 512B units. Must be at least as big as the
> -   largest supported sector size, currently 16K.  */
> -#define GRUB_DISK_CACHE_BITS   6
> +   largest supported sector size, currently 64K.  */
> +#define GRUB_DISK_CACHE_BITS   7
>  #define GRUB_DISK_CACHE_SIZE   (1 << GRUB_DISK_CACHE_BITS)
>
>  #define GRUB_DISK_MAX_MAX_AGGLOMERATE ((1 << (30 - GRUB_DISK_CACHE_BITS -
> GRUB_DISK_SECTOR_BITS)) - 1)
>
>  /* Maximum number of sectors to read in LBA mode at once */
> -#define GRUB_DISK_MAX_LBA_SECTORS 63
> +#define GRUB_DISK_MAX_LBA_SECTORS 127
>
>  /* Return value of grub_disk_native_sectors() in case disk size is
> unknown. */
>  #define GRUB_DISK_SIZE_UNKNOWN  0xULL
> --
> 2.41.0
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: State of Argon2 support

[Vladimir 'phcoder' Serbinenko]

Le ven. 26 janv. 2024, 20:50, Daniel Kiper  a écrit :

> On Fri, Jan 26, 2024 at 03:18:57AM -0500, Nikolaos Chatzikonstantinou
> wrote:
> > On Thu, Jan 25, 2024 at 1:15 PM Daniel Kiper 
> wrote:
> > >
> > > Adding Vladimir who knows GRUB history better than I...
> > >
> > > On Wed, Jan 24, 2024 at 01:23:55AM -0500, Nikolaos Chatzikonstantinou
> wrote:
> > >
> > > [...]
> > >
> > > > My apologies for the repeated messages, but I came up with just one
> > > > more question that I'm curious about. To summarize my questions:
> > > >
> > > > 1. Where is the libgcrypt bundle from grub from? I think my
> > > > investigation has led me around version 1.7.0 of libgcrypt, but if I
> > > > can get a precise commit or version, that would be useful.
> > > >
> > > > ... and now to my new question:
> > >
> > > Vladimir, could you help with that?
> > >
> > > > 2. What is the reason libgcrypt is bundled as opposed to a regular
> dependency?
> > >
> > > I am not entirely sure I understand the question. Could you elaborate?
> >
> > By bundling, I mean that someone copied libgcrypt files into the GRUB
> project.
> >
> > To elaborate further, regular programs (not like GRUB which is a
> > bootloader) can link statically or dynamically to libraries; but also,
> > there's a third option, to dump the source code of a library directly
> > into the source tree of the project. To my understanding this third
> > option (which is not really a third linker option as it is not related
> > to the linker) is chosen when the project needs to include its own
> > patch set to the library. I am curious if GRUB has patched libgcrypt
> > for some reason, and is that why libgcrypt is bundled with GRUB?
>
> I think Vladimir could tell us more here...
>
> Anyway, I think we should avoid patching libgcrypt, or any given library
> merged with GRUB source, as much as possible.
>
This was my goal as well. Almost all the changes are difficult to avoid.
But at least they are automated in most cases. See import_gcry script. I'm
not on my computer now. I hope to find a time to have a look until the end
of next week.

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

Re: [PATCH] cli_lock: Added build option to block command line interface

[Vladimir 'phcoder' Serbinenko]

Le ven. 26 janv. 2024, 23:15, Daniel Kiper  a écrit :

> On Fri, Jan 26, 2024 at 02:12:31AM +0300, Vladimir 'phcoder' Serbinenko
> wrote:
> > Please detail your use case. GRUB already had user framework in the same
> > problem space.
>
> I am not sure what exactly you mean by "user framework". Could you
> elaborate or point us code examples?
>

https://www.gnu.org/software/grub/manual/grub/grub.html#Authentication-and-authorisation

>
> Anyway, we need a mechanism to disallow access to CLI, arguments editing
> for kernels, etc. I.e. user cannot change widely understood boot config
> even being at the console.
>
Yes and it's exactly what happens as soon as superuser is set. Basically
this patch is equivalent to having a superuser without a valid password.
AFAIR it's achieved by setting superuser's but not issuing password
commands. If not we can have password_deny command.

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

Re: [PATCH] cli_lock: Added build option to block command line interface

[Vladimir 'phcoder' Serbinenko]

Please detail your use case. GRUB already had user framework in the same
problem space.

Le mer. 24 janv. 2024, 09:27, Alec Brown  a écrit :

> Added functionality to disable command line interface access and editing
> of GRUB
> menu entries if GRUB image is built with --disable-cli.
>
> Signed-off-by: Alec Brown 
> ---
>  docs/grub.texi |  6 --
>  grub-core/kern/main.c  | 28 
>  grub-core/kern/rescue_reader.c | 13 +
>  grub-core/normal/auth.c|  3 +++
>  grub-core/normal/menu_text.c   | 31 +--
>  include/grub/kernel.h  |  3 ++-
>  include/grub/misc.h|  2 ++
>  include/grub/util/install.h|  8 ++--
>  util/grub-install-common.c | 11 ---
>  util/grub-mkimage.c|  9 -
>  util/mkimage.c | 16 +++-
>  11 files changed, 106 insertions(+), 24 deletions(-)
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index a225f9a88..96ab17d5b 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -6412,8 +6412,10 @@ the GRUB command line, edit menu entries, and
> execute any menu entry.  If
>  @samp{superusers} is set, then use of the command line and editing of menu
>  entries are automatically restricted to superusers. Setting
> @samp{superusers}
>  to empty string effectively disables both access to CLI and editing of
> menu
> -entries. Note: The environment variable needs to be exported to also
> affect
> -the section defined by the @samp{submenu} command (@pxref{submenu}).
> +entries. Building a grub image with @samp{--disable-cli} option will also
> +disable access to CLI and editing of menu entries, as well as disabling
> rescue
> +mode. Note: The environment variable needs to be exported to also affect
> the
> +section defined by the @samp{submenu} command (@pxref{submenu}).
>
>  Other users may be allowed to execute specific menu entries by giving a
> list of
>  usernames (as above) using the @option{--users} option to the
> diff --git a/grub-core/kern/main.c b/grub-core/kern/main.c
> index 731c07c29..30643164e 100644
> --- a/grub-core/kern/main.c
> +++ b/grub-core/kern/main.c
> @@ -30,11 +30,14 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #ifdef GRUB_MACHINE_PCBIOS
>  #include 
>  #endif
>
> +static bool cli_disabled = false;
> +
>  grub_addr_t
>  grub_modules_get_end (void)
>  {
> @@ -237,6 +240,28 @@ grub_load_normal_mode (void)
>grub_command_execute ("normal", 0, 0);
>  }
>
> +bool
> +grub_is_cli_disabled (void)
> +{
> +  return cli_disabled;
> +}
> +
> +static void
> +check_is_cli_disabled (void)
> +{
> +  struct grub_module_header *header;
> +  header = 0;
> +
> +  FOR_MODULES (header)
> +{
> +  if (header->type == OBJ_TYPE_DISABLE_CLI)
> +   {
> + cli_disabled = true;
> + return;
> +   }
> +}
> +}
> +
>  static void
>  reclaim_module_space (void)
>  {
> @@ -294,6 +319,9 @@ grub_main (void)
>
>grub_boot_time ("After loading embedded modules.");
>
> +  /* Check if the CLI should be disabled */
> +  check_is_cli_disabled ();
> +
>/* It is better to set the root device as soon as possible,
>   for convenience.  */
>grub_set_prefix_and_root ();
> diff --git a/grub-core/kern/rescue_reader.c
> b/grub-core/kern/rescue_reader.c
> index dcd7d4439..4259857ba 100644
> --- a/grub-core/kern/rescue_reader.c
> +++ b/grub-core/kern/rescue_reader.c
> @@ -78,6 +78,19 @@ grub_rescue_read_line (char **line, int cont,
>  void __attribute__ ((noreturn))
>  grub_rescue_run (void)
>  {
> +  /* Stall if the CLI has been disabled */
> +  if (grub_is_cli_disabled ())
> +{
> +  grub_printf ("Rescue mode has been disabled...\n");
> +
> +  do
> +   {
> + /* Do not optimize out the loop. */
> + asm volatile ("");
> +   }
> +  while (1);
> +}
> +
>grub_printf ("Entering rescue mode...\n");
>
>while (1)
> diff --git a/grub-core/normal/auth.c b/grub-core/normal/auth.c
> index 517fc623f..d94020186 100644
> --- a/grub-core/normal/auth.c
> +++ b/grub-core/normal/auth.c
> @@ -209,6 +209,9 @@ grub_auth_check_authentication (const char *userlist)
>char entered[GRUB_AUTH_MAX_PASSLEN];
>struct grub_auth_user *user;
>
> +  if (grub_is_cli_disabled ())
> +return GRUB_ACCESS_DENIED;
> +
>grub_memset (login, 0, sizeof (login));
>
>if (is_authenticated (userlist))
> diff --git a/grub-core/normal/menu_text.c b/grub-core/normal/menu_text.c
> index b1321eb26..9c383e64a 100644
> --- a/grub-core/normal/menu_text.c
> +++ b/grub-core/normal/menu_text.c
> @@ -178,21 +178,24 @@ command-line or ESC to discard edits and return to
> the GRUB menu."),
>
>grub_free (msg_translated);
>
> -  if (nested)
> +  if (!grub_is_cli_disabled ())
> {
> - ret += grub_print_message_indented_real
> -   (_("Press enter to boot the selected OS, "
> -  "`e' to edit the commands before booting "
> -

Re: [PATCH 2/3] osdep/unix/getroot.c: Clean up redundant code

[Vladimir 'phcoder' Serbinenko]

Le mar. 23 janv. 2024, 17:44, Daniel Kiper  a écrit :

> Mate,
>
> Next time please respond to all people/addresses in the original
> email...
>
> On Mon, Jan 22, 2024 at 02:09:38PM +, Mate Kukri wrote:
> > Dear Alec, and grub-devel,
> >
> > I haven't checked the specific code in question, but do we really want
> > to be removing such null-assignments? (Thinking about multiple patches
> > exactly like this).
> >
> > In correct code, they are of course redundant by definition, however
> > their intended purpose is that if the code happens to be incorrect,
> > they turn use-after-free bugs into zero page accesses.
> >
> > Since static analysis of a language like C is inherently conservative,
> > it is entirely possible that it is detecting the redundant assignment,
> > but not the use after free it would have prevented.
>
> We know the Coverity makes mistakes. So, we carefully check its reports.
> These ones are not exceptions. Here the Coverity is correct and it is
> safe to remove redundant code. I expect somebody was changing the code
> at some point and forgot to drop surplus assignments.
>
Nope. It was to keep an invariant and ensure we don't double-free
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 0/3] Clean up unused values

[Vladimir 'phcoder' Serbinenko]

I oppose to all 3 patches. These assignments are not redundant but keep an
important invariant: the variable in question can be passed to free().
For this it needs to either be NULL or point to a valid allocated memory.
In this code this ensures that we never double free even after code changes

Le sam. 20 janv. 2024, 05:53, Alec Brown  a écrit :

> Coverity listed three unused value bugs in the GRUB. These patches help
> clean
> up and remove these uneccessary bits of code.
>
> The Coverity bugs being addressed are:
> CID 428875
> CID 428876
> CID 428877
>
> Alec Brown (3):
>   fs/jfs.c: Clean up redundant code
>   osdep/unix/getroot.c: Clean up redundant code
>   loader/i386/multiboot_mbi.c: Clean up redundant code
>
>  grub-core/fs/jfs.c| 1 -
>  grub-core/loader/i386/multiboot_mbi.c | 2 +-
>  grub-core/osdep/unix/getroot.c| 1 -
>  3 files changed, 1 insertion(+), 3 deletions(-)
>
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] osdep/hurd/getroot: Fix 64bit build

[Vladimir 'phcoder' Serbinenko]

LGTM
Reviewed-by: Vladimir Serbinenko

Le sam. 20 janv. 2024, 21:36, Samuel Thibault 
a écrit :

> file_get_fs_options takes a mach_msg_type_number_t (32bit), not a size_t
> (64bit on 64bit platforms).
> ---
>  grub-core/osdep/hurd/getroot.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/grub-core/osdep/hurd/getroot.c
> b/grub-core/osdep/hurd/getroot.c
> index 0efefdab4..b849700e6 100644
> --- a/grub-core/osdep/hurd/getroot.c
> +++ b/grub-core/osdep/hurd/getroot.c
> @@ -58,7 +58,7 @@ grub_util_find_hurd_root_device (const char *path)
>file_t file;
>error_t err;
>char *argz = NULL, *name = NULL, *ret;
> -  size_t argz_len = 0;
> +  mach_msg_type_number_t argz_len = 0;
>int i;
>
>file = file_name_lookup (path, 0, 0);
> --
> 2.43.0
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] util/grub.d/30_os-prober.in: Conditionally show or hide chain and efi menu entries

[Vladimir 'phcoder' Serbinenko]

Le mer. 17 janv. 2024, 18:10, Pascal Hambourg  a
écrit :

> On 17/01/2024 at 04:15, Vladimir 'phcoder' Serbinenko wrote:
> >>onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
> >>  cat << EOF
> >> +if [ "\$grub_platform" != "efi" ]; then
> >
> > This is not the right check. Only "pc" platform supports chainloadin
> > boot sector. All other: coreboot, qemu, emu, ieee1275, xen and
> > non-x86. The only one which might is xen_pvh but this needs to be
> > checked
>
> I expect os-prober to report boot type "chain" only on platforms which
> support chainloading. Isn't it enough ?

Os-prober has no idea how system is booted. And one of my systems can boot
as any of coreboot, multiboot, EFI, PC or xen. Then I can run qemu version
using whole disk. This together means that the same disk can boot in any
way.

> The goal is to hide "chain" menu
> entries when booting in EFI mode, no more.
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] util/grub.d/30_os-prober.in: Skip drivemap for Windows 8 to 19, Server 2012 and later

[Vladimir 'phcoder' Serbinenko]

On Sat, Jan 6, 2024 at 6:38 PM Pascal Hambourg  wrote:
>
> If Windows Vista, Seven and Server 2008 do not need drivemap,
> then later versions using bootmgr too should not need it either.
>
> Note:
> This patch checks Windows versions up to 19 because distinguishing 20
> from 2000 requires more complex logic. Hopefully Windows will drop
> BIOS/legacy boot support before version 20.
Frankly people though x86 would be long dead by now...

> case ${LONGNAME} in
> -   Windows\ Vista*|Windows\ 7*|Windows\ Server\ 2008*)
> +   Windows\ Vista*|Windows\ [781]*|Windows\ Server\ 2008*|Windows\ 
> Server\ 20[1-9]*)

What happens to Windows 1.01 ? Or is it detected as MS-DOS?
-- 
Regards
Vladimir 'phcoder' Serbinenko

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

Re: [PATCH] util/grub.d/30_os-prober.in: Conditionally show or hide chain and efi menu entries

[Vladimir 'phcoder' Serbinenko]

>   onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
> cat << EOF
> +if [ "\$grub_platform" != "efi" ]; then

This is not the right check. Only "pc" platform supports chainloadin
boot sector. All other: coreboot, qemu, emu, ieee1275, xen and
non-x86. The only one which might is xen_pvh but this needs to be
checked

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