Re: [PATCH v10] plainmount: Support plain encryption mode

2023-01-10 Thread Daniel Kiper
On Wed, Dec 28, 2022 at 05:47:29PM +, Maxim Fomin wrote:
> --- Original Message ---
> On Wednesday, December 28th, 2022 at 5:20 PM, Maxim Fomin  
> wrote:
>
> > From e7f54a0b78353c01bf4c966f871b3e3590222743 Mon Sep 17 00:00:00 2001
> > From: Maxim Fomin ma...@fomin.one
> >
> > Date: Wed, 28 Dec 2022 13:19:36 +
> > Subject: [PATCH v10] plainmount: Support plain encryption mode
> >
> > This patch adds support for plain encryption mode (plain dm-crypt) via
> > new module/command named 'plainmount'.
> >
> > Signed-off-by: Maxim Fomin ma...@fomin.one
> >
> > ---
> > Interdiff against v9:
> > diff --git a/grub-core/disk/plainmount.c b/grub-core/disk/plainmount.c
> > index eabedf4c3..47e64805f 100644
> > --- a/grub-core/disk/plainmount.c
> > +++ b/grub-core/disk/plainmount.c
> > @@ -326,7 +326,7 @@ grub_cmd_plainmount (grub_extcmd_context_t ctxt, int 
> > argc, char *args)
> > * Arbitrary data keys are supported via keyfiles.
> > /
> > password_size = grub_strlen (state[OPTION_PASSWORD].arg);
> > - grub_memcpy (key_data, state[OPTION_PASSWORD].arg, password_size);
> > + grub_strcpy ((char) key_data, state[OPTION_PASSWORD].arg);
> > }
> >
> > / Set cipher mode (tested above) */
> > @@ -334,9 +334,16 @@ grub_cmd_plainmount (grub_extcmd_context_t ctxt, int 
> > argc, char **args)
> > mode++ = '\0';
> >
> > / Check cipher */
> > - if (grub_cryptodisk_setcipher (dev, cipher, mode) != GRUB_ERR_NONE)
> > + err = grub_cryptodisk_setcipher (dev, cipher, mode);
> > + if (err != GRUB_ERR_NONE)
> > {
> > - err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid cipher %s"), cipher);
> > + if (err == GRUB_ERR_FILE_NOT_FOUND)
> > + err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid cipher %s"), cipher);
> > + else if (err == GRUB_ERR_BAD_ARGUMENT)
> > + err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid mode %s"), mode);
> > + else
> > + err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid cipher %s or mode 
> > %s"),
> > + cipher, mode);
> > goto fail;
> > }
> >
>
> Hi, Daniel. Unfortunately I was very busy last time and could not get back to 
> the patch earlier.

No problem...

> I have removed one instance of memcpy() where the function operates on C 
> string. Other calls to
> memcpy() operate on data buffers. Also, I improved error message where cipher 
> was correct, but
> the mode was wrong. In previous version for input 'aes-xts-blah' error was 
> 'invalid cipher aes'.

Now your patch is in the upstream. Thank you for doing this work!

Daniel

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


Re: [PATCH] loader/i386/linux.c Correct wrong initrd address for debug

2023-01-10 Thread Daniel Kiper
On Fri, Dec 09, 2022 at 12:07:49PM +0800, Jeremy Szu wrote:
> From: Jeremy Su 
>
> The 'addr' is used to request the memory with specific ranges but the
> real loadable address come from the relocator.
> Thus, print the final retrieved addresses (virtual and physical) for
> initrd.
> ---
>  grub-core/loader/i386/linux.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
> index ee1c3b985..953639445 100644
> --- a/grub-core/loader/i386/linux.c
> +++ b/grub-core/loader/i386/linux.c
> @@ -1207,8 +1207,8 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
> ((unused)),
>if (grub_initrd_load (&initrd_ctx, initrd_mem))
>  goto fail;
>
> -  grub_dprintf ("linux", "Initrd, addr=0x%x, size=0x%x\n",
> - (unsigned) addr, (unsigned) size);
> +  grub_dprintf ("linux", "Initrd (%p) at 0x%" PRIxGRUB_ADDR ", size=0x%x\n",
> + initrd_mem, initrd_mem_target, (unsigned) size);

Sadly this patch does not apply to the upstream...

Daniel

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


Re: [PATCH v8 1/1] plainmount: Support plain encryption mode

2023-01-10 Thread Glenn Washburn
On Wed, 28 Dec 2022 18:05:11 +
Maxim Fomin  wrote:

> --- Original Message ---
> On Saturday, December 24th, 2022 at 2:09 AM, Glenn Washburn
>  wrote:
> > 
> > On Fri, 23 Dec 2022 19:54:47 -0600
> > Glenn Washburn developm...@efficientek.com wrote:
> > 
> > > On Fri, 02 Dec 2022 17:11:23 +
> > > Maxim Fomin ma...@fomin.one wrote:
> > > 
> > > > --- Original Message ---
> > > > On Friday, December 2nd, 2022 at 0:00, Glenn Washburn
> > > > developm...@efficientek.com wrote:
> > > > 
> > > > > I'm now compiling this patch and found a few compile issues
> > > > > below. You're compile testing this right?
> > > > 
> > > > First versions of the patch were tested in pure grub src
> > > > directory. Later I switched to directly making and installing
> > > > GRUB package for my distro using its source script syntax. It
> > > > seems this process was affected by environment options which
> > > > hided these errors/warnings.
> > > > 
> > > > I test the patch on my two old laptops - one with UEFI BIOS
> > > > (x86_64-efi) and one is pre-UEFI (i386-pc). I was compiling
> > > > i386-pc target too, because otherwise the second laptop was
> > > > unbootable. During i386-pc compilation I noticed some warnings
> > > > related to 'PRIuGRUB_XXX' macros which were absent during efi
> > > > target compilation. I noticed that there are similar warnings
> > > > in other modules and decided that there are issues with
> > > > 'PRIuGRUB_XXX' macros at i386-pc platform at global level. In
> > > > any case, these issues didn't cause compilation fail in my
> > > > working environment because I would not be able to compile and
> > > > boot pre-UEFI lap. Do you use -Werror?
> > > 
> > > I didn't see this until just now. In case you're still
> > > interested, no I don't use -Werror or any special compiler flags.
> > > And I'm using gcc version 10.2.1 from a Debian 11 container.
> > 
> > 
> > Correction, -Werror is being used. Perhaps that's a default compiler
> > flag on Debian systems.
> > 
> > Glenn
> > 
> 
> This explains why you have found these issues. However, it does not
> explain how you can compile grub with -Werror because currently there
> are following warnings in x86_64-efi mode:
> grub-core/lib/libgcrypt-grub/mpi/mpi-internal.h:150:24: warning:
> variable ‘_ql’ set but not used [-Wunused-but-set-variable]
> grub-core/lib/libgcrypt-grub/mpi/mpih-div.c:53:9: warning: variable
> ‘dummy’ set but not used [-Wunused-but-set-variable]

Ok I looked at this a little more, I'm getting these warnings when
compiling the speedtest module. They are not being treated as errors.
By default GRUB will use -Werror when building the target, unless
--disable-werror is specified. However, the gcc command for the
speedtest module doesn't have -Werror but a bunch of -W* and does not
include -Wunused-but-set-variable, which is why the compile doesn't
error. So it seems that -Werror is being changed to constituent -W*
options and some -W* are left out in my case (haven't found where this
happens yet). I'm not setting any CFLAGS that might affect this, are
you? Since, I've not seen anyone else complaining about it here, I
suspect this is something odd about your build environment.

What is your build environment? (distro, GCC version)

> 
> When I was working with the patch earlier this year I remember having
> these and several more warnings which prevented me from using
> -Werror. Back then I have removed the switch and have forgotten about
> this issue completely.

Did you remove the switch by using the --disable-werror configure
option? If not, how?

Glenn

> 
> Best regards,
> Maxim Fomin
> 

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


Re: [RFC PATCH v2 1/1] kern/dl: Add module version check

2023-01-10 Thread Glenn Washburn
On Fri, 23 Dec 2022 16:18:43 +0800
Zhang Boyang  wrote:

> Hi,

Hi, sorry I missed this til now.

> On 2022/12/22 14:31, Glenn Washburn wrote:
> > On Wed, 21 Dec 2022 20:11:57 +0800
> > Zhang Boyang  wrote:
> >> +  grub_printf_ (N_("warning: module %.63s has incorrect
> >> version: %.63s != %s\n"),
> >> +  mod->name, modver, PACKAGE_VERSION);
> > 
> > I don't quite like this, but I could live with it. I mostly don't
> > like it because I don't want to get spammed with a lot of these
> > warning messages. But maybe this is the best of the bad set of
> > options.
> > 
> 
> I sent a V3 patch at 
> https://lists.gnu.org/archive/html/grub-devel/2022-12/msg00280.html ,
> in which can disable this warning at build time, however I personally
> don't like it.
> 
> I think preloading modules is a good solution for you. e.g. run
> `insmod foobar` before switching $root/$prefix. This also eliminate
> the possibility of GRUB crashing because of mismatched modules. For a 
> comprehensive list of modules, please see $GRUB_MODULES in 
> https://salsa.debian.org/grub-team/grub/-/blob/master/debian/build-efi-images
> 
> If you think this solution is good to you, I want to drop V3 and
> submit a improved V2 as V4, because it's cleaner.

I don't like that because it requires a priori knowledge of what
modules are being loaded in the sourced config file. However, I do
already preload some modules. Perhaps out of scope for this
patch, but I've been thinking about have a variable modules_prefix,
which is used only to load modules, or some way of decoupling the
module load path from $root. Then I can set that and not worry about how
$root is changed in the script.

> > One thing to note is I believe that this will take GRUB out
> > of graphical mode and put it into text mode. This doesn't affect me
> > (right now) though.
> > 
> 
> I tested on i386-pc and x86_64-efi and there is no such behavior.

Your test was with loading a mismatched module, where the warning is
triggered?

> > Another thing is that, for the use case of loading a grub.cfg from a
> > different GRUB installation, this message from version mismatched
> > modules loaded in the sourcing of the script will be displayed for a
> > fraction of a second until the menu from the sourced grub.cfg is
> > displayed. So it may not be helpful for the user.
> > 
> 
> What about adding a 100ms sleep when the warning is displayed? By the 
> way, this can also be workaround by preloading modules.

Is that long enough to make it readable? It would be nice if GRUB had
a log buffer for the debug log messages. Then it could be put in the
debug log and the user could see that there was a version mismatch.

Anyway, since its just a warning message an doesn't prevent the module
from loading (when not in lockdown), its not really worse than the
current state. Except for getting spammed, which isn't a huge deal. So I
can live with this implementation.

Glenn

> 
> > This isn't likely an issue for loopback.cfg because usually the
> > module loading is done implicitly and all the commands are
> > generally inside the menu entries. In this case, printing to the
> > screen seems like a good idea because directly after the module
> > loading the commands will run, which might crash GRUB. So if the
> > crash hangs the machine the user will see this message and have a
> > clue.
> >
> 
> Best Regards,
> Zhang Boyang
> 
> > Glenn
> > 

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


[PATCH] tests: Fix help test to reflect updated help output

2023-01-10 Thread Glenn Washburn
Commit f5759a878 (normal/help: Add paging instructions to normal and help
prompts) changed the output of the help command, which broke the help
test. This change allows the test to pass.

Signed-off-by: Glenn Washburn 
---
 tests/help_test.in | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/help_test.in b/tests/help_test.in
index b08cf20138..630e9860ae 100644
--- a/tests/help_test.in
+++ b/tests/help_test.in
@@ -8,6 +8,9 @@ Show a help message.
 
 -h, --help  Display this help and exit.
 -u, --usage Display the usage of this command and exit.
+
+
+To enable less(1)-like paging, \"set pager=1\".
 Hello World"
 outpu="$(echo 'help help; hello' | @builddir@/grub-shell)"
 
-- 
2.34.1


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


[PATCH] gdbstub: Unregister gdbstub_break when unloading module

2023-01-10 Thread Glenn Washburn
Signed-off-by: Glenn Washburn 
---
 grub-core/gdb/gdb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/grub-core/gdb/gdb.c b/grub-core/gdb/gdb.c
index 1818cb6f8e..9e091ee1bc 100644
--- a/grub-core/gdb/gdb.c
+++ b/grub-core/gdb/gdb.c
@@ -98,6 +98,7 @@ GRUB_MOD_INIT (gdb)
 GRUB_MOD_FINI (gdb)
 {
   grub_unregister_command (cmd);
+  grub_unregister_command (cmd_break);
   grub_unregister_command (cmd_stop);
   grub_gdb_idtrestore ();
 }
-- 
2.34.1


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


[PATCH 0/3] Trivial changes

2023-01-10 Thread Glenn Washburn
Some spelling and spacing changes. Feel free to merge together or with other
changes as desired.

Glenn

Glenn Washburn (3):
  misc: Spelling fixes
  misc: Fix spacing
  misc: efi: Fix spacing

 grub-core/disk/loopback.c   |  2 +-
 grub-core/gdb/i386/idt.c|  2 +-
 grub-core/gdb/i386/signal.c |  2 +-
 include/grub/efi/api.h  | 58 ++---
 include/grub/partition.h|  2 +-
 5 files changed, 33 insertions(+), 33 deletions(-)

-- 
2.34.1


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


[PATCH 1/3] misc: Spelling fixes

2023-01-10 Thread Glenn Washburn
Signed-off-by: Glenn Washburn 
---
 grub-core/gdb/i386/idt.c| 2 +-
 grub-core/gdb/i386/signal.c | 2 +-
 include/grub/partition.h| 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/grub-core/gdb/i386/idt.c b/grub-core/gdb/i386/idt.c
index 69bfcb089c..43bde51a01 100644
--- a/grub-core/gdb/i386/idt.c
+++ b/grub-core/gdb/i386/idt.c
@@ -1,4 +1,4 @@
-/* idt.c - routines for constructing IDT fot the GDB stub */
+/* idt.c - routines for constructing IDT for the GDB stub */
 /*
  *  Copyright (C) 2006  Lubomir Kundrak
  *
diff --git a/grub-core/gdb/i386/signal.c b/grub-core/gdb/i386/signal.c
index 1ac3bbd183..b169c6b230 100644
--- a/grub-core/gdb/i386/signal.c
+++ b/grub-core/gdb/i386/signal.c
@@ -1,4 +1,4 @@
-/* idt.c - routines for constructing IDT fot the GDB stub */
+/* idt.c - routines for constructing IDT for the GDB stub */
 /*
  *  Copyright (C) 2006  Lubomir Kundrak
  *
diff --git a/include/grub/partition.h b/include/grub/partition.h
index 8208cc7881..c4d71d0eea 100644
--- a/include/grub/partition.h
+++ b/include/grub/partition.h
@@ -87,7 +87,7 @@ struct grub_partition
   /* The type partition map.  */
   grub_partition_map_t partmap;
 
-  /* The type of partition whne it's on MSDOS.
+  /* The type of partition when it's on MSDOS.
  Used for embedding detection.  */
   grub_uint8_t msdostype;
 };
-- 
2.34.1


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


[PATCH 2/3] misc: Fix spacing

2023-01-10 Thread Glenn Washburn
Signed-off-by: Glenn Washburn 
---
 grub-core/disk/loopback.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/disk/loopback.c b/grub-core/disk/loopback.c
index 2cfc53a916..4635dcfdee 100644
--- a/grub-core/disk/loopback.c
+++ b/grub-core/disk/loopback.c
@@ -89,7 +89,7 @@ grub_cmd_loopback (grub_extcmd_context_t ctxt, int argc, char 
**args)
 
   /* Check if `-d' was used.  */
   if (state[0].set)
-  return delete_loopback (args[0]);
+return delete_loopback (args[0]);
 
   if (!state[1].set)
 type |= GRUB_FILE_TYPE_NO_DECOMPRESS;
-- 
2.34.1


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


[PATCH 3/3] misc: efi: Fix spacing

2023-01-10 Thread Glenn Washburn
Signed-off-by: Glenn Washburn 
---
 include/grub/efi/api.h | 58 +-
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
index 75efb4d4c2..9e17b675bb 100644
--- a/include/grub/efi/api.h
+++ b/include/grub/efi/api.h
@@ -1077,8 +1077,8 @@ typedef struct grub_efi_input_key grub_efi_input_key_t;
 typedef grub_efi_uint8_t grub_efi_key_toggle_state_t;
 struct grub_efi_key_state
 {
-   grub_efi_uint32_t key_shift_state;
-   grub_efi_key_toggle_state_t key_toggle_state;
+  grub_efi_uint32_t key_shift_state;
+  grub_efi_key_toggle_state_t key_toggle_state;
 };
 typedef struct grub_efi_key_state grub_efi_key_state_t;
 
@@ -1170,10 +1170,10 @@ struct grub_efi_boot_services
grub_efi_timer_delay_t type,
grub_efi_uint64_t trigger_time);
 
-   grub_efi_status_t
-   (*wait_for_event) (grub_efi_uintn_t num_events,
- grub_efi_event_t *event,
- grub_efi_uintn_t *index);
+  grub_efi_status_t
+  (*wait_for_event) (grub_efi_uintn_t num_events,
+grub_efi_event_t *event,
+grub_efi_uintn_t *index);
 
   grub_efi_status_t
   (*signal_event) (grub_efi_event_t event);
@@ -1184,11 +1184,11 @@ struct grub_efi_boot_services
   grub_efi_status_t
   (*check_event) (grub_efi_event_t event);
 
-   grub_efi_status_t
-   (*install_protocol_interface) (grub_efi_handle_t *handle,
- grub_efi_guid_t *protocol,
- grub_efi_interface_type_t 
protocol_interface_type,
- void *protocol_interface);
+  grub_efi_status_t
+  (*install_protocol_interface) (grub_efi_handle_t *handle,
+grub_efi_guid_t *protocol,
+grub_efi_interface_type_t 
protocol_interface_type,
+void *protocol_interface);
 
   grub_efi_status_t
   (*reinstall_protocol_interface) (grub_efi_handle_t handle,
@@ -1454,29 +1454,29 @@ typedef grub_efi_status_t 
(*grub_efi_key_notify_function_t) (
 
 struct grub_efi_simple_text_input_ex_interface
 {
-   grub_efi_status_t
-   (*reset) (struct grub_efi_simple_text_input_ex_interface *this,
- grub_efi_boolean_t extended_verification);
+  grub_efi_status_t
+  (*reset) (struct grub_efi_simple_text_input_ex_interface *this,
+   grub_efi_boolean_t extended_verification);
 
-   grub_efi_status_t
-   (*read_key_stroke) (struct grub_efi_simple_text_input_ex_interface 
*this,
-   grub_efi_key_data_t *key_data);
+  grub_efi_status_t
+  (*read_key_stroke) (struct grub_efi_simple_text_input_ex_interface *this,
+ grub_efi_key_data_t *key_data);
 
-   grub_efi_event_t wait_for_key;
+  grub_efi_event_t wait_for_key;
 
-   grub_efi_status_t
-   (*set_state) (struct grub_efi_simple_text_input_ex_interface *this,
- grub_efi_key_toggle_state_t *key_toggle_state);
+  grub_efi_status_t
+  (*set_state) (struct grub_efi_simple_text_input_ex_interface *this,
+   grub_efi_key_toggle_state_t *key_toggle_state);
 
-   grub_efi_status_t
-   (*register_key_notify) (struct grub_efi_simple_text_input_ex_interface 
*this,
-   grub_efi_key_data_t *key_data,
-   grub_efi_key_notify_function_t 
key_notification_function,
-   void **notify_handle);
+  grub_efi_status_t
+  (*register_key_notify) (struct grub_efi_simple_text_input_ex_interface *this,
+ grub_efi_key_data_t *key_data,
+ grub_efi_key_notify_function_t 
key_notification_function,
+ void **notify_handle);
 
-   grub_efi_status_t
-   (*unregister_key_notify) (struct 
grub_efi_simple_text_input_ex_interface *this,
- void *notification_handle);
+  grub_efi_status_t
+  (*unregister_key_notify) (struct grub_efi_simple_text_input_ex_interface 
*this,
+   void *notification_handle);
 };
 typedef struct grub_efi_simple_text_input_ex_interface 
grub_efi_simple_text_input_ex_interface_t;
 
-- 
2.34.1


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


[PATCH] gzio: Remove confusing, out-dated comment

2023-01-10 Thread Glenn Washburn
The "transparent" parameter to grub_gzio_open was removed in 2010, fc2ef1172c
(* grub-core/io/gzio.c (grub_gzio_open): Removed "transparent" parameter.)

Signed-off-by: Glenn Washburn 
---
 grub-core/io/gzio.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/grub-core/io/gzio.c b/grub-core/io/gzio.c
index 4fa31ff218..ca93557513 100644
--- a/grub-core/io/gzio.c
+++ b/grub-core/io/gzio.c
@@ -1188,9 +1188,10 @@ initialize_tables (grub_gzio_t gzio)
 }
 
 
-/* Open a new decompressing object on the top of IO. If TRANSPARENT is true,
-   even if IO does not contain data compressed by gzip, return a valid file
-   object. Note that this function won't close IO, even if an error occurs.  */
+/*
+ * Open a new decompressing object on the top of IO.
+ * Note that this function won't close IO, even if an error occurs.
+ */
 static grub_file_t
 grub_gzio_open (grub_file_t io, enum grub_file_type type)
 {
-- 
2.34.1


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


[PATCH v6 03/14] gdb: If no modules have been loaded, do not try to load module symbols

2023-01-10 Thread Glenn Washburn
This prevents load_all_modules from failing when called before any
modules have been loaded. Failures in GDB user-defined functions cause
any function which called them to also fail.

Signed-off-by: Glenn Washburn 
---
 grub-core/gdb_grub.in | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index edb5a8872c..fc17e3d899 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -63,7 +63,9 @@ define load_all_modules
dump_module_sections $this
set $this = $this->next
end
-   match_and_load_symbols
+   if (grub_dl_head != 0)
+   match_and_load_symbols
+   end
 end
 document load_all_modules
Load debugging information for all loaded modules.
-- 
2.34.1


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


[PATCH v6 00/14] GDB script fixes and improvements

2023-01-10 Thread Glenn Washburn
Update since v5:
 * Need to declare grub_efi_print_gdb_info() as static and inline, which
   fixes a compiler error.

This series has been substantially rewritten since v4, although a large
minority of the patches haven't changed much. The biggest change is
that the implementation has been converted to Python instead of what was
done in shell script. I have always felt it should be done in Python, but
it seemed daunting to learn the Python-GDB API, so the shell script seemed
the path of least resistance. I decided to give it a second look and was
surprised it wasn't as bad as I thought it would be. Because the python API
is so tightly integrated into GDB, there are things you can do with it that
either aren't possible or which look like ugly hacks when attempting with
the native script.

The other big change is that there is a new --enable-efi-debug option to
configure which enables the printing of the gdb command for loading the
GRUB kernel symbols. This is disallowed when booting with Secure Boot on.
There are two ways the GDB command is printed: (1) in early EFI setup and
(2) on-demand by a user using the gdbinfo command.

There are a couple new features to the GDB script, like a trivial one that
changes the gdb prompt and another that allows for software breakpoints to
be set before the GRUB image is loaded.

This series also incorporates suggestions given for v4 and adds more to the
documentation.

Glenn

Glenn Washburn (14):
  gdb: Fix redirection issue in dump_module_sections
  gdb: Prevent wrapping when writing to .segments.tmp
  gdb: If no modules have been loaded, do not try to load module symbols
  gdb: Move runtime module loading into runtime_load_module
  gdb: Conditionally run GDB script logic for dynamically or statically
positioned GRUB
  gdb: Only connect to remote target once when first sourced
  gdb: Replace module symbol loading implementation with Python one
  gdb: Add functions to make loading from dynamically positioned targets
easier
  gdb: Add more support for debugging on EFI platforms
  gdb: Allow running user-defined commands at GRUB start
  gdb: Fix issue with breakpoints defined before the GRUB image is
loaded
  gdb: Add extra early initialization symbols for i386-pc
  gdb: Modify gdb prompt when running gdb_grub script
  docs: Add debugging chapter to development documentation

 config.h.in |   3 +
 configure.ac|  11 ++
 docs/grub-dev.texi  | 233 
 grub-core/Makefile.core.def |   5 +-
 grub-core/gdb_grub.in   | 159 +++-
 grub-core/gdb_helper.py.in  | 173 ++
 grub-core/gmodule.pl.in |  30 -
 grub-core/kern/efi/debug.c  |  40 +++
 grub-core/kern/efi/efi.c|   4 +-
 grub-core/kern/efi/init.c   |   7 +-
 include/grub/efi/debug.h|  43 +++
 include/grub/efi/efi.h  |   2 +-
 12 files changed, 620 insertions(+), 90 deletions(-)
 create mode 100644 grub-core/gdb_helper.py.in
 delete mode 100644 grub-core/gmodule.pl.in
 create mode 100644 grub-core/kern/efi/debug.c
 create mode 100644 include/grub/efi/debug.h

Range-diff against v5:
 1:  9f273b8fa5 =  1:  ef2b93c7df gdb: Fix redirection issue in 
dump_module_sections
 2:  85f68a8369 =  2:  fafc15ba07 gdb: Prevent wrapping when writing to 
.segments.tmp
 3:  88b3973cdb =  3:  9a8bc8c099 gdb: If no modules have been loaded, do not 
try to load module symbols
 4:  3037c1da91 =  4:  81eef1558d gdb: Move runtime module loading into 
runtime_load_module
 5:  f6288016f6 =  5:  835f19b2a3 gdb: Conditionally run GDB script logic for 
dynamically or statically positioned GRUB
 6:  da13fbe653 =  6:  ea32a6d83e gdb: Only connect to remote target once when 
first sourced
 7:  8e6059955a =  7:  13a5e05cd9 gdb: Replace module symbol loading 
implementation with Python one
 8:  878900d69b =  8:  c918d24d35 gdb: Add functions to make loading from 
dynamically positioned targets easier
 9:  036549604d !  9:  fd6d2a3ffd gdb: Add more support for debugging on EFI 
platforms
@@ include/grub/efi/debug.h (new)
 +
 +void grub_efi_register_debug_commands (void);
 +
-+void
++static inline void
 +grub_efi_print_gdb_info (void)
 +{
 +#if PRINT_GDB_SYM_LOAD_CMD
10:  959b2f = 10:  649bfc0a4d gdb: Allow running user-defined commands at 
GRUB start
11:  ac9f52b1d9 = 11:  8e9148a32b gdb: Fix issue with breakpoints defined 
before the GRUB image is loaded
12:  eac4405ffb = 12:  eb5f1b5d89 gdb: Add extra early initialization symbols 
for i386-pc
13:  e58715e227 = 13:  0e85de3667 gdb: Modify gdb prompt when running gdb_grub 
script
14:  1979dc664e = 14:  d78a1b27f0 docs: Add debugging chapter to development 
documentation
-- 
2.34.1


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


[PATCH v6 08/14] gdb: Add functions to make loading from dynamically positioned targets easier

2023-01-10 Thread Glenn Washburn
Many targets, such as EFI, load GRUB at addresses that are determined at
runtime. So the load addresses in kernel.exec will almost certainly be
wrong. Given the address of the start of the text segment, these
functions will tell GDB to load the symbols at the proper locations. It
is left up to the user to determine how to get the text address of the
loaded GRUB image.

Signed-off-by: Glenn Washburn 
---
 grub-core/gdb_grub.in  | 21 -
 grub-core/gdb_helper.py.in | 87 ++
 2 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index fc201204de..18ce6b0eb2 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -1,6 +1,6 @@
 ###
 ### Load debuging information about GNU GRUB 2 modules into GDB
-### automatically. Needs readelf, Python and gdb_helper.py script
+### automatically. Needs readelf, objdump, Python and gdb_helper.py script
 ###
 ### Has to be launched from the writable and trusted
 ### directory containing *.image and *.module
@@ -12,6 +12,25 @@
 source gdb_helper.py
 
 
+define dynamic_load_symbols
+   dynamic_load_kernel_exec_symbols $arg0
+
+   # We may have been very late to loading the kernel.exec symbols and
+   # and modules may already be loaded. So load symbols for any already
+   # loaded.
+   load_all_modules
+
+   if $is_grub_loaded()
+   runtime_load_module
+   end
+end
+document dynamic_load_symbols
+   Load debugging symbols from kernel.exec and any loaded modules given
+   the address of the .text segment of the UEFI binary in memory. Also
+   setup session to automatically load module symbols for modules loaded
+   in the future.
+end
+
 define load_all_modules
set $this = grub_dl_head
while ($this != 0)
diff --git a/grub-core/gdb_helper.py.in b/grub-core/gdb_helper.py.in
index 4306ef448a..8d5ee1d292 100644
--- a/grub-core/gdb_helper.py.in
+++ b/grub-core/gdb_helper.py.in
@@ -4,6 +4,23 @@ import subprocess
 
 # Convenience functions #
 
+class IsGrubLoaded (gdb.Function):
+  """Return 1 if GRUB has been loaded in memory, otherwise 0.
+The hueristic used is checking if the first 4 bytes of the memory pointed
+to by the _start symbol are not 0. This is true for QEMU on the first run
+of GRUB. This may not be true on physical hardware, where memory is not
+necessarily cleared on soft reset. This may not also be true in QEMU on
+soft resets. Also this many not be true when chainloading GRUB.
+"""
+
+  def __init__ (self):
+super (IsGrubLoaded, self).__init__ ("is_grub_loaded")
+
+  def invoke (self):
+return int (gdb.parse_and_eval ("*(int *) _start")) != 0
+
+is_grub_loaded = IsGrubLoaded ()
+
 class IsUserCommand (gdb.Function):
   """Set the second argument to true value if first argument is the name
 of a user-defined command.
@@ -24,6 +41,76 @@ is_user_command = IsUserCommand ()
 
 # Commands #
 
+# Loading symbols is complicated by the fact that kernel.exec is an ELF
+# ELF binary, but the UEFI runtime is PE32+. All the data sections of
+# the ELF binary are concatenated (accounting for ELF section alignment)
+# and put into one .data section of the PE32+ runtime image. So given
+# the load address of the .data PE32+ section we can determine the
+# addresses each ELF data section maps to. The UEFI application is
+# loaded into memory just as it is laid out in the file. It is not
+# assumed that the binary is available, but it is known that the .text
+# section directly precedes the .data section and that .data is EFI
+# page aligned. Using this, the .data offset can be found from the .text
+# address.
+class GrubLoadKernelExecSymbols (gdb.Command):
+  """Load debugging symbols from kernel.exec given the address of the
+.text segment of the UEFI binary in memory."""
+
+  PE_SECTION_ALIGN = 12
+
+  def __init__ (self):
+super (GrubLoadKernelExecSymbols, self).__init__ 
("dynamic_load_kernel_exec_symbols",
+ gdb.COMMAND_USER,
+ gdb.COMPLETE_EXPRESSION)
+
+  def invoke (self, arg, from_tty):
+self.dont_repeat ()
+args = gdb.string_to_argv (arg)
+
+if len (args) != 1:
+  raise RuntimeError ("dynamic_load_kernel_exec_symbols expects exactly 
one argument")
+
+sections = self.parse_objdump_sections ("kernel.exec")
+pe_text = args[0]
+text_size = [s['size'] for s in sections if s['name'] == '.text'][0]
+pe_data_offset = self.alignup (text_size, self.PE_SECTION_ALIGN)
+
+sym_load_cmd_parts = ["add-symbol-file", "kernel.exec", pe_text]
+offset = 0
+for section in sections:
+  if 'DATA' in section["flags"] or section["name"] == ".bss":
+offset = self.alignup (offset, section["align"])
+sym_load_cmd_parts.extend (["-s", section["name"], "(%s+0x%x+0x%x)" % 
(pe_text, pe_data_offset, offset)])
+offset += section["

[PATCH v6 07/14] gdb: Replace module symbol loading implementation with Python one

2023-01-10 Thread Glenn Washburn
Remove gmodule.pl and rewrite as a python in gdb_helper.py. This removes
PERL dependency for the GRUB GDB script, but adds Python as a dependency.
This is more desirable because Python is tightly integrated with GDB and
can do things not even available to GDB native scripting language. GDB must
be built with Python, however this is not a major limitation because every
major distro non-end-of-life versions build GDB with Python support. And GDB
has had support for Python since around 7.1-ish, which is about a decade.

This re-implementation has an added feature. If there is a user defined
command named "onload_", then that command will be executed
after the symbols for the specified module are loaded. When debugging a
module it can be desirable to set break points on code in the module.
This is difficult in GRUB because, at GDB start, the module is not loaded
and on EFI platforms its not known ahead of time where the module will
be loaded. So allow users to create an "onload_" command which
will be run when the module with name "modname" is loaded.

Another addition is a new convenience function is defined
$is_user_command(), which returns true if its string argument is
the name of a user-defined command.

A secondary benefit of these changes is that the script does not write
temporary files and has better error handling capabilities.

Signed-off-by: Glenn Washburn 
---
 grub-core/Makefile.core.def |  4 +-
 grub-core/gdb_grub.in   | 54 ++--
 grub-core/gdb_helper.py.in  | 82 +
 grub-core/gmodule.pl.in | 30 --
 4 files changed, 87 insertions(+), 83 deletions(-)
 create mode 100644 grub-core/gdb_helper.py.in
 delete mode 100644 grub-core/gmodule.pl.in

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index ba967aac88..d216a79e83 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -20,8 +20,8 @@ transform_data = {
 
 transform_data = {
   installdir = platform;
-  name = gmodule.pl;
-  common = gmodule.pl.in;
+  name = gdb_helper.py;
+  common = gdb_helper.py.in;
 };
 
 transform_data = {
diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index 4ca939f69a..fc201204de 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -1,6 +1,6 @@
 ###
 ### Load debuging information about GNU GRUB 2 modules into GDB
-### automatically. Needs readelf, Perl and gmodule.pl script
+### automatically. Needs readelf, Python and gdb_helper.py script
 ###
 ### Has to be launched from the writable and trusted
 ### directory containing *.image and *.module
@@ -9,63 +9,15 @@
 ### Lubomir Kundrak 
 ###
 
-# Add section numbers and addresses to .segments.tmp
-define dump_module_sections_helper
-   set $mod = $arg0
-   printf "%s", $mod->name
-   set $segment = $mod->segment
-   while ($segment)
-   printf " %i 0x%lx", $segment->section, $segment->addr
-   set $segment = $segment->next
-   end
-   printf "\n"
-end
+source gdb_helper.py
 
-define dump_module_sections
-   # Set unlimited width so that lines don't get wrapped writing
-   # to .segments.tmp
-   with width 0 -- \
-   with trace-commands off -- \
-   pipe dump_module_sections_helper $arg0 | sh -c 'cat >>.segments.tmp'
-end
-document dump_module_sections
-   Gather information about module whose mod structure was
-   given for use with match_and_load_symbols
-end
-
-# Generate and execute GDB commands and delete temporary files
-# afterwards
-define match_and_load_symbols
-   shell perl gmodule.pl <.segments.tmp >.loadsym.gdb
-   source .loadsym.gdb
-   shell rm -f .segments.tmp .loadsym.gdb
-end
-document match_and_load_symbols
-   Launch script, that matches section names with information
-   generated by dump_module_sections and load debugging info
-   apropriately
-end
-
-###
-
-define load_module
-   dump_module_sections $arg0
-   match_and_load_symbols
-end
-document load_module
-   Load debugging information for module given as argument.
-end
 
 define load_all_modules
-   shell rm -f .segments.tmp
set $this = grub_dl_head
while ($this != 0)
-   dump_module_sections $this
+   load_module $this
set $this = $this->next
end
-   if (grub_dl_head != 0)
-   match_and_load_symbols
-   end
 end
 document load_all_modules
Load debugging information for all loaded modules.
diff --git a/grub-core/gdb_helper.py.in b/grub-core/gdb_helper.py.in
new file mode 100644
index 00..4306ef448a
--- /dev/null
+++ b/grub-core/gdb_helper.py.in
@@ -0,0 +1,82 @@
+import os
+import re
+import subprocess
+
+# Convenience functions #
+
+class IsUserCommand (gdb.Function):
+  """Set the second argument to true value if first argument is the name
+of a user-defined command.
+"""
+
+  def __init__ (self):
+super (IsUserCommand, self).__init_

[PATCH v6 04/14] gdb: Move runtime module loading into runtime_load_module

2023-01-10 Thread Glenn Washburn
By moving this code into a function, it can be run re-utilized while gdb is
running, not just when loading the script. This will also be useful in
some following changes which will make a separate script path for targets
which statically vs dynamically position GRUB code.

Signed-off-by: Glenn Washburn 
---
 grub-core/gdb_grub.in | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index fc17e3d899..d525a5a11f 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -71,16 +71,22 @@ document load_all_modules
Load debugging information for all loaded modules.
 end
 
+define runtime_load_module
+   break grub_dl_add
+   commands
+   silent
+   load_module mod
+   cont
+   end
+end
+document runtime_load_module
+   Load module symbols at runtime as they are loaded.
+end
+
 ###
 
 set confirm off
 file kernel.exec
 target remote :1234
 
-# inform when module is loaded
-break grub_dl_add
-commands
-   silent
-   load_module mod
-   cont
-end
+runtime_load_module
-- 
2.34.1


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


[PATCH v6 14/14] docs: Add debugging chapter to development documentation

2023-01-10 Thread Glenn Washburn
Debugging GRUB can be tricky and require arcane knowledge. This will
help those unfamiliar with the process to get started debugging GRUB
with less effort.

Signed-off-by: Glenn Washburn 
---
 docs/grub-dev.texi | 233 +
 1 file changed, 233 insertions(+)

diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi
index f76fc658bf..18f09a48e7 100644
--- a/docs/grub-dev.texi
+++ b/docs/grub-dev.texi
@@ -79,6 +79,7 @@ This edition documents version @value{VERSION}.
 * Contributing Changes::
 * Setting up and running test suite::
 * Updating External Code::
+* Debugging::
 * Porting::
 * Error Handling::
 * Stack and heap size::
@@ -595,6 +596,238 @@ cp minilzo-2.10/*.[hc] grub-core/lib/minilzo
 rm -r minilzo-2.10*
 @end example
 
+@node Debugging
+@chapter Debugging
+
+GRUB2 can be difficult to debug because it runs on the bare-metal and thus
+does not have the debugging facilities normally provided by an operating
+system. This chapter aims to provide useful information on some ways to
+debug GRUB2 for some architectures. It by no means intends to be exhaustive.
+The focus will be one x86_64 and i386 architectures. Luckily for some issues
+virtual machines have made the ability to debug GRUB2 much easier, and this
+chapter will focus debugging via the QEMU virtual machine. We will not be
+going over debugging of the userland tools (eg. grub-install), there are
+many tutorials on debugging programs in userland.
+
+You will need GDB and the QEMU binaries for your system, on Debian these
+can be installed with the @samp{gdb} and @samp{qemu-system-x86} packages.
+Also it is assumed that you have already successfully compiled GRUB2 from
+source for the target specified in the section below and have some
+familiarity with GDB. When GRUB2 is built it will create many different
+binaries. The ones of concern will be in the @file{grub-core}
+directory of the GRUB2 build dir. To aide in debugging we will want the
+debugging symbols generated during the build because these symbols are not
+kept in the binaries which get installed to the boot location. The build
+process outputs two sets of binaries, one without symbols which gets executed
+at boot, and another set of ELF images with debugging symbols. The built
+images with debugging symbols will have a @file{.image} suffix, and the ones
+without a @file{.img} suffix. Similarly, loadable modules with debugging
+symbols will have a @file{.module} suffix, and ones without a @file{.mod}
+suffix. In the case of the kernel the binary with symbols is named
+@file{kernel.exec}.
+
+In the following sections, information will be provided on debugging on
+various targets using @command{gdb} and the @samp{gdb_grub} GDB script.
+
+@menu
+* i386-pc::
+* x86_64-efi::
+@end menu
+
+@node i386-pc
+@section i386-pc
+
+The i386-pc target is a good place to start when first debugging GRUB2
+because in some respects its easier than EFI platforms. The reason being
+that the initial load address is always known in advance. To start
+debugging GRUB2 first QEMU must be started in GDB stub mode. The following
+command is a simple illustration:
+
+@example
+qemu-system-i386 -drive file=disk.img,format=raw \
+-device virtio-scsi-pci,id=scsi0 -S -s
+@end example
+
+This will start a QEMU instance booting from @file{disk.img}. It will pause
+at start waiting for a GDB instance to attach to it. You should change
+@file{disk.img} to something more appropriate. A block device can be used,
+but you may need to run QEMU as a privileged user.
+
+To connect to this QEMU instance with GDB, the @code{target remote} GDB
+command must be used. We also need to load a binary image, preferably with
+symbols. This can be done using the GDB command @code{file kernel.exec}, if
+GDB is started from the @file{grub-core} directory in the GRUB2 build
+directory. GRUB2 developers have made this more simple by including a GDB
+script which does much of the setup. This file at @file{grub-core/gdb_grub}
+of the build directory and is also installed via @command{make install}.
+If not building GRUB, the distribution may have a package which installs
+this GDB script along with debug symbol binaries, such as Debian's
+@samp{grub-pc-dbg} package. The GDB scripts is intended to by used
+like so, assuming:
+
+@example
+cd $(dirname /path/to/script/gdb_grub)
+gdb -x gdb_grub
+@end example
+
+Once GDB has been started with the @file{gdb_grub} script it will
+automatically connect to the QEMU instance. You can then do things you
+normally would in GDB like set a break point on @var{grub_main}.
+
+Setting breakpoints in modules is trickier since they haven't been loaded
+yet and are loaded at addresses determined at runtime. The module could be
+loaded to different addresses in different QEMU instances. The debug symbols
+in the modules @file{.module} binary, thus are always wrong, and GDB needs
+to be told where to load the symbols to. But this must happen at runtime
+after GRUB2 has dete

[PATCH v6 10/14] gdb: Allow running user-defined commands at GRUB start

2023-01-10 Thread Glenn Washburn
A new command, run_on_start, for things to do when just before GRUB starts
executing. Currently, this is setting up the loading of module symbols as
they are loaded and allowing user-defined script to be run if a command
named "onstart" exists. A thbreak, temporary hardware breakpoint, is used
because a software breakpoint would be overwritten when the firmware loads
the GRUB image into memory. And it should be temporary in order to have as
many of the limited hardware breakpoints available to the user as possible.

Signed-off-by: Glenn Washburn 
---
 grub-core/gdb_grub.in | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index 18ce6b0eb2..281dfb5927 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -15,6 +15,8 @@ source gdb_helper.py
 define dynamic_load_symbols
dynamic_load_kernel_exec_symbols $arg0
 
+   run_on_start
+
# We may have been very late to loading the kernel.exec symbols and
# and modules may already be loaded. So load symbols for any already
# loaded.
@@ -54,6 +56,33 @@ document runtime_load_module
Load module symbols at runtime as they are loaded.
 end
 
+define run_on_start
+   # TODO: Add check to see if _start symbol is defined, if not, then
+   # the symbols have not yet been loaded and this command will not work.
+   thbreak _start
+   commands
+   silent
+
+   runtime_load_module
+
+   if $is_user_command("onstart")
+   onstart
+   end
+   continue
+   end
+end
+document run_on_start
+   On some targets, such as x86_64-efi, even if you know where the
+   firmware will load the GRUB image, you can not simply set a break
+   point before the image is loaded because loading the image
+   overwrites the break point in memory. So setup a hardware watch
+   point, which does not have that problem, and if that gets triggered,
+   then reset the break point. If a user-defined command named
+   "onstart" exists it will be run after the start is hit.
+   NOTE: This assumes symbols have already been correctly loaded for
+   the EFI application.
+end
+
 ###
 
 set confirm off
@@ -71,6 +100,7 @@ if ! $runonce
exec-file kernel.exec
else
file kernel.exec
+   run_on_start
runtime_load_module
end
 
-- 
2.34.1


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


[PATCH v6 12/14] gdb: Add extra early initialization symbols for i386-pc

2023-01-10 Thread Glenn Washburn
Add symbols for boot.image, disk.image, and lzma_decompress.image if the
target is i386-pc. This is only done for i386-pc because that is the only
target that uses the images. By loading the symbols for these images,
these images can be more easily debugged by allowing the setting of break-
points in that code and to see easily get the value of data symbols.

Signed-off-by: Glenn Washburn 
---
 grub-core/gdb_grub.in | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index 8e89bbf368..f188a842ab 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -114,12 +114,18 @@ set confirm off
 # fail.
 
 set $platform_efi = $_streq("@platform@", "efi")
+set $target = "@target_cpu@-@platform@"
 
 if ! $runonce
if $platform_efi
# Only load the executable file, not the symbols
exec-file kernel.exec
else
+   if $_streq($target, "i386-pc")
+   add-symbol-file boot.image
+   add-symbol-file diskboot.image
+   add-symbol-file lzma_decompress.image
+   end
file kernel.exec
run_on_start
runtime_load_module
-- 
2.34.1


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


[PATCH v6 05/14] gdb: Conditionally run GDB script logic for dynamically or statically positioned GRUB

2023-01-10 Thread Glenn Washburn
There are broadly two classes of targets to consider when loading symbols
for GRUB, targets that determine where to load GRUB at runtime
(dynamically positioned) and those that do not (statically positioned).
For statically poisitioned targets, symbol loading is determined at link
time, so nothing more needs to be known to load the symbols. For
dynamically positioned targets, such as EFI targets, at runtime symbols
should be offset by an amount that depends on where the runtime chose to
load GRUB.

It is important to not load symbols statically for dynamic targets
because then when subsequently loading the symbols correctly one must
take care to remove the existing static symbols, otherwise there will be
two sets of symbols and GDB seems to prefer the ones loaded first (ie the
static ones).

Use autoconf variables to generate a gdb_grub for a particular target,
which conditionally run startup code depending on if the target uses
static or dynamic loading.

Signed-off-by: Glenn Washburn 
---
 grub-core/gdb_grub.in | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index d525a5a11f..620d1def72 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -86,7 +86,20 @@ end
 ###
 
 set confirm off
-file kernel.exec
-target remote :1234
 
-runtime_load_module
+# Note: On EFI and other platforms that load GRUB to an address that is
+# determined at runtime, the symbols in kernel.exec will be wrong.
+# However, we must start by loading some executable file or GDB will
+# fail.
+
+set $platform_efi = $_streq("@platform@", "efi")
+
+if $platform_efi
+   # Only load the executable file, not the symbols
+   exec-file kernel.exec
+else
+   file kernel.exec
+   runtime_load_module
+end
+
+target remote :1234
-- 
2.34.1


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


[PATCH v6 01/14] gdb: Fix redirection issue in dump_module_sections

2023-01-10 Thread Glenn Washburn
An error in any GDB command causes it to immediately abort with an error,
this includes any command that calls that command. This leads to an issue
in dump_module_sections where an error causes the command to exit without
turning off file redirection. The user then ends up with a GDB command
line where commands output nothing to the console.

Instead do the work of dump_module_sections in the command
dump_module_sections_helper and run the command using GDB's pipe command
which does the redirection and undoes the redirection when it finishes
regardless of any errors in the command.

Also, remove .segments.tmp file prior to loading modules in case one was
left from a previous run.

Signed-off-by: Glenn Washburn 
---
 grub-core/gdb_grub.in | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index e322d3dc10..4e45ad5622 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -10,15 +10,8 @@
 ###
 
 # Add section numbers and addresses to .segments.tmp
-define dump_module_sections
+define dump_module_sections_helper
set $mod = $arg0
-
-   # FIXME: save logging status
-   set logging file .segments.tmp
-   set logging redirect on
-   set logging overwrite off
-   set logging on
-
printf "%s", $mod->name
set $segment = $mod->segment
while ($segment)
@@ -26,9 +19,10 @@ define dump_module_sections
set $segment = $segment->next
end
printf "\n"
+end
 
-   set logging off
-   # FIXME: restore logging status
+define dump_module_sections
+   pipe dump_module_sections_helper $arg0 | sh -c 'cat >>.segments.tmp'
 end
 document dump_module_sections
Gather information about module whose mod structure was
@@ -59,6 +53,7 @@ document load_module
 end
 
 define load_all_modules
+   shell rm -f .segments.tmp
set $this = grub_dl_head
while ($this != 0)
dump_module_sections $this
-- 
2.34.1


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


[PATCH v6 13/14] gdb: Modify gdb prompt when running gdb_grub script

2023-01-10 Thread Glenn Washburn
This will let users know that the GDB session is using the GRUB gdb scripts.

Signed-off-by: Glenn Washburn 
---
 grub-core/gdb_helper.py.in | 4 
 1 file changed, 4 insertions(+)

diff --git a/grub-core/gdb_helper.py.in b/grub-core/gdb_helper.py.in
index 8d5ee1d292..5ed9eab0f5 100644
--- a/grub-core/gdb_helper.py.in
+++ b/grub-core/gdb_helper.py.in
@@ -2,6 +2,10 @@ import os
 import re
 import subprocess
 
+def prompt_hook (current_prompt):
+  return "(grub gdb) "
+gdb.prompt_hook = prompt_hook
+
 # Convenience functions #
 
 class IsGrubLoaded (gdb.Function):
-- 
2.34.1


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


[PATCH v6 11/14] gdb: Fix issue with breakpoints defined before the GRUB image is loaded

2023-01-10 Thread Glenn Washburn
On some platforms, notably x86, software breakpoints set in GDB before
the GRUB image is loaded will be cleared when the image is loaded. This
is because the breakpoints work by overwriting the memory of the break-
point location with a special instruction which when hit will cause the
debugger to stop execution. Just before execution is resumed by the
debugger, the original instruction bytes are put back. When a breakpoint
is set before the GRUB image is loaded, the special debugger instruction
will be written to memory and when the GRUB image is loaded by the
firmware, which has no knowledge of the debugger, the debugger instruction
is overwritten. To the GDB user, GDB will show the breakpoint as set, but
it will never be hit. Furthermore, GDB now becomes confused, such that
even deleting and re-setting the breakpoint after the GRUB image is loaded
will not allow for a working breakpoint.

To work around this, in run_on_start, first a watchpoint is set on _start,
which will be triggered when the firmware starts loading the GRUB image.
When the _start watchpoint is hit, the current breakpoints are saved to a
file and then deleted by GDB before they can be overwritten by the firmware
and confuse GDB. Then a temporary software breakpoint is set on _start,
which will get triggered when the firmware hands off to GRUB to execute. In
that breakpoint load the previously saved and deleted breakpoints now that
there is no worry of them getting overwritten by the firmware.

Note that watchpoints are generally types of hardware breakpoints on x86, so
its deleted as soon as it gets triggered so that a minimal set of hardware
breakpoints are used, allowing more for the user.

Signed-off-by: Glenn Washburn 
---
 grub-core/gdb_grub.in | 33 +++--
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index 281dfb5927..8e89bbf368 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -59,14 +59,35 @@ end
 define run_on_start
# TODO: Add check to see if _start symbol is defined, if not, then
# the symbols have not yet been loaded and this command will not work.
-   thbreak _start
+   watch *_start
+   set $break_efi_start_bpnum = $bpnum
commands
silent
-
-   runtime_load_module
-
-   if $is_user_command("onstart")
-   onstart
+   delete $break_efi_start_bpnum
+
+   # Save the breakpoints here before the GRUB image is loaded
+   # into memory, then delete them. Later they will be reloaded
+   # once the GRUB image has been loaded. This avoids the issue
+   # where the loading of the GRUB image overwrites the software
+   # breakpoints, thus confusing GDB and effectively clearing
+   # those breakpoints.
+   save breakpoints .early-breakpoints.gdb
+   delete breakpoints
+
+   tbreak _start
+   commands
+   silent
+
+   # Reload the breakpoints now that the GRUB image has
+   # finished being loaded into memory.
+   source .early-breakpoints.gdb
+
+   runtime_load_module
+
+   if $is_user_command("onstart")
+   onstart
+   end
+   continue
end
continue
end
-- 
2.34.1


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


[PATCH v6 06/14] gdb: Only connect to remote target once when first sourced

2023-01-10 Thread Glenn Washburn
The gdb_grub script was originally meant to be run once when GDB first
starts up via the -x argument. So it runs commands unconditionally
assuming that the script has not been run before. Its nice to be able
to source the script again when developing the script to modify/add
commands. So only run the commands not defined in user-defined commands,
if a variable $runonce has already been set and when those commands have
been run to set $runonce.

Signed-off-by: Glenn Washburn 
---
 grub-core/gdb_grub.in | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index 620d1def72..4ca939f69a 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -94,12 +94,15 @@ set confirm off
 
 set $platform_efi = $_streq("@platform@", "efi")
 
-if $platform_efi
-   # Only load the executable file, not the symbols
-   exec-file kernel.exec
-else
-   file kernel.exec
-   runtime_load_module
-end
+if ! $runonce
+   if $platform_efi
+   # Only load the executable file, not the symbols
+   exec-file kernel.exec
+   else
+   file kernel.exec
+   runtime_load_module
+   end
 
-target remote :1234
+   target remote :1234
+   set $runonce = 1
+end
-- 
2.34.1


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


[PATCH v6 09/14] gdb: Add more support for debugging on EFI platforms

2023-01-10 Thread Glenn Washburn
If the configure option --enable-efi-debug is given, then enable the
printing early in EFI startup of the command needed to load symbols for
the GRUB EFI kernel. This is needed because EFI firmware determines where
to load the GRUB EFI at runtime, and so the relevant addresses are not
known ahead of time. This is not printed when secure boot is enabled.

The command is a custom command defined in the gdb_grub GDB script. So
GDB should be started with the script as an argument to the -x option or
sourced into an active GDB session before running the outputted command.

Also a command named "gdbinfo" is enabled which allows the user to print
the gdb command string on-demand, which can be valuable as the printing
early in EFI startup is quickly replaced by other text. So if using a
physical screen it may appear too briefly to be registered.

Co-developed-by: Peter Jones 
Signed-off-by: Glenn Washburn 
---
 config.h.in |  3 +++
 configure.ac| 11 ++
 grub-core/Makefile.core.def |  1 +
 grub-core/kern/efi/debug.c  | 40 ++
 grub-core/kern/efi/efi.c|  4 ++--
 grub-core/kern/efi/init.c   |  7 +-
 include/grub/efi/debug.h| 43 +
 include/grub/efi/efi.h  |  2 +-
 8 files changed, 107 insertions(+), 4 deletions(-)
 create mode 100644 grub-core/kern/efi/debug.c
 create mode 100644 include/grub/efi/debug.h

diff --git a/config.h.in b/config.h.in
index 4d1e50eba7..9f88be1593 100644
--- a/config.h.in
+++ b/config.h.in
@@ -13,6 +13,9 @@
 #define MM_DEBUG @MM_DEBUG@
 #endif
 
+/* Define to 1 to enable printing of gdb command to load module symbols.  */
+#define PRINT_GDB_SYM_LOAD_CMD @EFI_DEBUG@
+
 /* Define to 1 to enable disk cache statistics.  */
 #define DISK_CACHE_STATS @DISK_CACHE_STATS@
 #define BOOT_TIME_STATS @BOOT_TIME_STATS@
diff --git a/configure.ac b/configure.ac
index 93626b7982..4bf477b072 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1521,6 +1521,17 @@ else
 fi
 AC_SUBST([MM_DEBUG])
 
+# EFI debugging.
+AC_ARG_ENABLE([efi-debug],
+ AS_HELP_STRING([--enable-efi-debug],
+ [include aides for debugging the EFI binary]))
+if test x$enable_efi_debug = xyes; then
+  EFI_DEBUG=1
+else
+  EFI_DEBUG=0
+fi
+AC_SUBST([EFI_DEBUG])
+
 AC_ARG_ENABLE([cache-stats],
  AS_HELP_STRING([--enable-cache-stats],
  [enable disk cache statistics collection]))
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index d216a79e83..0ca102ad79 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -206,6 +206,7 @@ kernel = {
 
   efi = disk/efi/efidisk.c;
   efi = kern/efi/efi.c;
+  efi = kern/efi/debug.c;
   efi = kern/efi/init.c;
   efi = kern/efi/mm.c;
   efi = term/efi/console.c;
diff --git a/grub-core/kern/efi/debug.c b/grub-core/kern/efi/debug.c
new file mode 100644
index 00..c4f5561587
--- /dev/null
+++ b/grub-core/kern/efi/debug.c
@@ -0,0 +1,40 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2022  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 .
+ */
+/* debug.c - aides for debugging the EFI application */
+
+#include 
+#include 
+#include 
+
+__attribute__ ((unused)) static grub_err_t
+grub_cmd_gdbinfo (struct grub_command *cmd __attribute__ ((unused)),
+ int argc __attribute__ ((unused)),
+ char **args __attribute__ ((unused)))
+{
+  grub_efi_print_gdb_info ();
+  return 0;
+}
+
+void
+grub_efi_register_debug_commands (void)
+{
+#if PRINT_GDB_SYM_LOAD_CMD
+  grub_register_command_lockdown ("gdbinfo", grub_cmd_gdbinfo, 0,
+ N_("Print infomation useful for GDB 
debugging"));
+#endif
+}
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index cf49d6357e..17bd06f7e6 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -299,7 +299,7 @@ grub_efi_get_variable (const char *var, const 
grub_efi_guid_t *guid,
 /* Search the mods section from the PE32/PE32+ image. This code uses
a PE32 header, but should work with PE32+ as well.  */
 grub_addr_t
-grub_efi_modules_addr (void)
+grub_efi_section_addr (const char *section_name)
 {
   grub_efi_loaded_image_t *image;
   struct grub_msdos_image_header *header;
@@ -328,7 +328,7 @@ grub_efi_modules_addr (void)
 

[PATCH v6 02/14] gdb: Prevent wrapping when writing to .segments.tmp

2023-01-10 Thread Glenn Washburn
GDB logging is redirected to write .segments.tmp, which means that GDB
will wrap lines longer than what it thinks is the screen width
(typically 80 characters). When wrapping does occur it causes gmodule.pl
to misbehave. So disable line wrapping by using GDB's "with" command so
that its guaranteed to return the width to the previous value upon
command completion.

Also disable command tracing when dumping the module sections because
that output will go to .segments.tmp and thus cause gmodule.pl to
misbehave.

Signed-off-by: Glenn Washburn 
---
 grub-core/gdb_grub.in | 4 
 1 file changed, 4 insertions(+)

diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
index 4e45ad5622..edb5a8872c 100644
--- a/grub-core/gdb_grub.in
+++ b/grub-core/gdb_grub.in
@@ -22,6 +22,10 @@ define dump_module_sections_helper
 end
 
 define dump_module_sections
+   # Set unlimited width so that lines don't get wrapped writing
+   # to .segments.tmp
+   with width 0 -- \
+   with trace-commands off -- \
pipe dump_module_sections_helper $arg0 | sh -c 'cat >>.segments.tmp'
 end
 document dump_module_sections
-- 
2.34.1


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


[PATCH 0/6] Cryptomount testing

2023-01-10 Thread Glenn Washburn
This patch series adds a variety of functional cryptomount LUKS1/2 tests by
creating a LUKS container on the host and verifying that data inside can be
read accurately from a virtualized GRUB. This should be especially useful
when we eventually get around to upgrading the gcrypt library.

Glenn

Glenn Washburn (6):
  grub-shell: Set exit status to qemu exit status
  grub-shell: Only cleanup working directory file if QEMU does not fail
or timeout
  grub-shell: Allow specifying non-default trim line contents
  grub-shell: Trim line should always be matched from the beginning of
the line
  grub-shell: Add halt_cmd variable to testcase namespace
  tests: Add cryptomount functional test

 Makefile.util.def|  12 +
 tests/grub_cmd_cryptomount.in| 185 ++
 tests/util/grub-shell-luks-tester.in | 366 +++
 tests/util/grub-shell.in |  49 +++-
 4 files changed, 600 insertions(+), 12 deletions(-)
 create mode 100644 tests/grub_cmd_cryptomount.in
 create mode 100644 tests/util/grub-shell-luks-tester.in

-- 
2.34.1


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


[PATCH 2/6] grub-shell: Only cleanup working directory file if QEMU does not fail or timeout

2023-01-10 Thread Glenn Washburn
This keeps the generated files to aid in diagnosing the source of the
failure.

Signed-off-by: Glenn Washburn 
---
 tests/util/grub-shell.in | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index 2c0e654c1f..e5d34d1d35 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -537,8 +537,6 @@ elif [ x$boot = xemu ]; then
 (cd "$rootdir"; tar cf "$roottar" .)
 setup_qemu_logger
 "${builddir}/grub-core/grub-emu" -m "$device_map" --memdisk "$roottar" -r 
memdisk -d "/boot/grub" > "$work_directory/qemu-pipe" || ret=$?
-test -n "$debug" || rm -rf "$rootdir"
-test -n "$debug" || rm -f "$roottar"
 else
 setup_qemu_logger
 timeout -s KILL $timeout "${qemu}" ${qemuopts} ${serial_null} -serial 
file:/dev/stdout -${device}"${isofile}" ${bootdev} > 
"$work_directory/qemu-pipe" || ret=$?
@@ -547,8 +545,16 @@ fi
 wait
 rm -f "$work_directory/qemu-pipe"
 
+if [ "$ret" -ne 0 ]; then
+# If QEMU failure, keep generated files to reproduce
+exit $ret
+fi
+
 if [ x$boot = xcoreboot ]; then
 test -n "$debug" || rm -f "${imgfile}"
+elif [ x$boot = xemu ]; then
+test -n "$debug" || rm -rf "$rootdir"
+test -n "$debug" || rm -f "$roottar"
 fi
 test -n "$debug" || rm -f "${isofile}"
 test -n "$debug" || rm -rf "${rom_directory}"
-- 
2.34.1


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


[PATCH 6/6] tests: Add cryptomount functional test

2023-01-10 Thread Glenn Washburn
The grub_cmd_cryptomount make check test performs some functional testing
of cryptomount and by extension the underlying cryptodisk infrastructure.

A utility test script named grub-shell-luks-tester is created to handle the
complexities of the testing, making it simpler to add new test cases in
grub_cmd_cryptomount.

Signed-off-by: Glenn Washburn 
---
 Makefile.util.def|  12 +
 tests/grub_cmd_cryptomount.in| 185 ++
 tests/util/grub-shell-luks-tester.in | 366 +++
 3 files changed, 563 insertions(+)
 create mode 100644 tests/grub_cmd_cryptomount.in
 create mode 100644 tests/util/grub-shell-luks-tester.in

diff --git a/Makefile.util.def b/Makefile.util.def
index d919c562c4..22dabba358 100644
--- a/Makefile.util.def
+++ b/Makefile.util.def
@@ -743,6 +743,12 @@ script = {
   installdir = noinst;
 };
 
+script = {
+  name = grub-shell-luks-tester;
+  common = tests/util/grub-shell-luks-tester.in;
+  installdir = noinst;
+};
+
 script = {
   name = grub-fs-tester;
   common = tests/util/grub-fs-tester.in;
@@ -1039,6 +1045,12 @@ script = {
   common = tests/grub_script_return.in;
 };
 
+script = {
+  testcase = nonnative;
+  name = grub_cmd_cryptomount;
+  common = tests/grub_cmd_cryptomount.in;
+};
+
 script = {
   testcase = nonnative;
   name = grub_cmd_regexp;
diff --git a/tests/grub_cmd_cryptomount.in b/tests/grub_cmd_cryptomount.in
new file mode 100644
index 00..b05cd3800e
--- /dev/null
+++ b/tests/grub_cmd_cryptomount.in
@@ -0,0 +1,185 @@
+#! @BUILD_SHEBANG@ -e
+
+# Run all grub cryptomount tests in a Qemu instance
+# Copyright (C) 2023  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 .
+
+if [ "x$EUID" = "x" ] ; then
+  EUID=`id -u`
+fi
+
+if [ "$EUID" != 0 ] ; then
+   echo "not root; cannot test cryptomount."
+   exit 77
+fi
+
+if ! which cryptsetup >/dev/null 2>&1; then
+   echo "cryptsetup not installed; cannot test cryptomount."
+   exit 77
+fi
+
+if ! which mkfs.vfat >/dev/null 2>&1; then
+   echo "mkfs.vfat not installed; cannot test cryptomount."
+   exit 77
+fi
+
+COMMON_OPTS='${V:+--debug=$V} --cs-opts="--pbkdf-force-iterations 1000"'
+
+_testcase() {
+local EXPECTEDRES=$1
+local LOGPREFIX=$2
+local res=0
+local output
+shift 2
+
+# Create a subdir in TMPDIR for each testcase
+_TMPDIR=$TMPDIR
+TMPDIR=$TMPDIR/`echo -n "$(date +%s).$LOGPREFIX" | sed -e 's,[ /],_,g' -e 
's,:$,,g'`
+mkdir -p "$TMPDIR"
+
+output=`"$@" 2>&1` || res=$?
+TMPDIR=$_TMPDIR
+
+if [ "$res" -eq "$EXPECTEDRES" ]; then
+if [ "$res" -eq 0 ]; then
+echo $LOGPREFIX PASS
+else
+echo $LOGPREFIX XFAIL
+fi
+else
+echo "Error[$res]: $output"
+if [ "$res" -eq 0 ]; then
+echo $LOGPREFIX XPASS
+elif [ "$res" -eq 1 ]; then
+echo $LOGPREFIX FAIL
+else
+# Any exit code other than 1 or 0, indicates a hard error,
+# not a test error
+echo $LOGPREFIX ERROR
+return 99
+fi
+return 1
+fi
+}
+
+testcase() { _testcase 0 "$@"; }
+testcase_fail() { _testcase 1 "$@"; }
+
+### LUKS1 tests
+eval testcase "'LUKS1 test cryptsetup defaults:'" \
+@builddir@/grub-shell-luks-tester --luks=1 $COMMON_OPTS
+
+eval testcase "'LUKS1 test with twofish cipher:'" \
+@builddir@/grub-shell-luks-tester --luks=1 $COMMON_OPTS \
+"--cs-opts='--cipher twofish-xts-plain64'"
+
+eval testcase "'LUKS1 test key file support:'" \
+@builddir@/grub-shell-luks-tester --luks=1 $COMMON_OPTS \
+--keyfile
+
+eval testcase "'LUKS1 test key file with offset:'" \
+@builddir@/grub-shell-luks-tester --luks=1 $COMMON_OPTS \
+--keyfile --cs-opts="--keyfile-offset=237"
+
+eval testcase "'LUKS1 test key file with offset and size:'" \
+@builddir@/grub-shell-luks-tester --luks=1 $COMMON_OPTS \
+--keyfile "--cs-opts='--keyfile-offset=237 --keyfile-size=1023'"
+
+eval testcase "'LUKS1 test detached header support:'" \
+@builddir@/grub-shell-luks-tester --luks=1 $COMMON_OPTS \
+--detached-header
+
+eval testcase "'LUKS1 test both detached header and key file:'" \
+@builddir@/grub-shell-luks-tester --luks=1 $COMMON_OPTS \
+--keyfile --detached-header
+
+### LUKS2 tests (mirroring the LUKS1 tests above)
+LUKS2_COMMON_OPTS="--luks=2 --cs-opts=--

[PATCH 5/6] grub-shell: Add halt_cmd variable to testcase namespace

2023-01-10 Thread Glenn Washburn
This allows test case scripts to use the appropriate halt command for the
built architecture to end execution early. Otherwise, test case scripts
have no way to know the appropriate mechanism for halting the test case
early.

Signed-off-by: Glenn Washburn 
---
 tests/util/grub-shell.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index f41e1a0b68..8db6ba86ec 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -331,6 +331,8 @@ fi
 cfgfile="$work_directory/grub.cfg"
 cat <${cfgfile}
 grubshell=yes
+halt_cmd=${halt_cmd}
+export halt_cmd
 enable_progress_indicator=0
 export enable_progress_indicator
 EOF
-- 
2.34.1


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


[PATCH 4/6] grub-shell: Trim line should always be matched from the beginning of the line

2023-01-10 Thread Glenn Washburn
When turning on shell tracing the trim line will be output before we
actually want to start the trim. However, in this case the trim line never
starts from the beginning of the line. So start trimming from the correct
line by matching from the beginning of the line.

Signed-off-by: Glenn Washburn 
---
 tests/util/grub-shell.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index 585f0d066e..f41e1a0b68 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -360,7 +360,7 @@ terminal_output ${term}
 EOF
 
 if [ $trim = 1 ]; then
-echo "echo $trim_head" >>${cfgfile}
+echo "echo; echo $trim_head" >>${cfgfile}
 fi
 
 rom_directory="$work_directory/rom_directory"
@@ -484,7 +484,7 @@ fi
 do_trim ()
 {
 if [ $trim = 1 ] || [ $trim = 2 ]; then
-   awk '{ if (have_head == 1) print $0; } /'"$trim_head"'/ { have_head=1; 
}'
+   awk '{ if (have_head == 1) print $0; } /^'"$trim_head"'/ { have_head=1; 
}'
 else
cat
 fi
-- 
2.34.1


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


[PATCH 1/6] grub-shell: Set exit status to qemu exit status

2023-01-10 Thread Glenn Washburn
This allows us to test if unexpected output in test scripts is because of a
bug in grub, because there was an error in qemu, or qemu was killed due to a
timeout.

Signed-off-by: Glenn Washburn 
---
 tests/util/grub-shell.in | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index 43672e0804..2c0e654c1f 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -502,6 +502,12 @@ copy_extra_files() {
 done
 }
 
+setup_qemu_logger() {
+cat < "$work_directory/qemu-pipe" | tr -d "\r" | tee "${goutfile}" | 
do_trim &
+}
+
+ret=0
+mkfifo "$work_directory/qemu-pipe"
 if [ x$boot = xnet ]; then
 netdir="$work_directory/netdir"
 mkdir -p "$netdir"
@@ -509,7 +515,8 @@ if [ x$boot = xnet ]; then
 cp "${cfgfile}" "$netdir/boot/grub/grub.cfg"
 cp "${source}" "$netdir/boot/grub/testcase.cfg"
 [ -z "$files" ] || copy_extra_files "$netdir" $files
-timeout -s KILL $timeout "${qemu}" ${qemuopts} ${serial_null} -serial 
file:/dev/stdout -boot n -net 
"user,tftp=$netdir,bootfile=/boot/grub/${grub_modinfo_target_cpu}-${grub_modinfo_platform}/core.$netbootext"
  -net nic  | cat | tr -d "\r" | tee "${goutfile}" | do_trim
+setup_qemu_logger
+timeout -s KILL $timeout "${qemu}" ${qemuopts} ${serial_null} -serial 
file:/dev/stdout -boot n -net 
"user,tftp=$netdir,bootfile=/boot/grub/${grub_modinfo_target_cpu}-${grub_modinfo_platform}/core.$netbootext"
  -net nic > "$work_directory/qemu-pipe" || ret=$?
 elif [ x$boot = xemu ]; then
 rootdir="$work_directory/rootdir"
 grubdir="$rootdir/boot/grub"
@@ -528,12 +535,18 @@ elif [ x$boot = xemu ]; then
 [ -z "$files" ] || copy_extra_files "$rootdir" $files
 roottar="$work_directory/root.tar"
 (cd "$rootdir"; tar cf "$roottar" .)
-"${builddir}/grub-core/grub-emu" -m "$device_map" --memdisk "$roottar" -r 
memdisk -d "/boot/grub" | tr -d "\r" | tee "${goutfile}" | do_trim
+setup_qemu_logger
+"${builddir}/grub-core/grub-emu" -m "$device_map" --memdisk "$roottar" -r 
memdisk -d "/boot/grub" > "$work_directory/qemu-pipe" || ret=$?
 test -n "$debug" || rm -rf "$rootdir"
 test -n "$debug" || rm -f "$roottar"
 else
-timeout -s KILL $timeout "${qemu}" ${qemuopts} ${serial_null} -serial 
file:/dev/stdout -${device}"${isofile}" ${bootdev} | cat | tr -d "\r" | tee 
"${goutfile}" | do_trim
+setup_qemu_logger
+timeout -s KILL $timeout "${qemu}" ${qemuopts} ${serial_null} -serial 
file:/dev/stdout -${device}"${isofile}" ${bootdev} > 
"$work_directory/qemu-pipe" || ret=$?
 fi
+
+wait
+rm -f "$work_directory/qemu-pipe"
+
 if [ x$boot = xcoreboot ]; then
 test -n "$debug" || rm -f "${imgfile}"
 fi
@@ -541,6 +554,6 @@ test -n "$debug" || rm -f "${isofile}"
 test -n "$debug" || rm -rf "${rom_directory}"
 test -n "$debug" || rm -f "${tmpfile}" "${cfgfile}" "${goutfile}"
 
-exit 0
+exit $ret
 
 
-- 
2.34.1


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


[PATCH 3/6] grub-shell: Allow specifying non-default trim line contents

2023-01-10 Thread Glenn Washburn
This will be useful for tests that have unwanted output from setup. This is
not documented because its only intended to be internal at the moment. Also,
--no-trim is allowed to explicitly turn off trim.

Signed-off-by: Glenn Washburn 
---
 tests/util/grub-shell.in | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index e5d34d1d35..585f0d066e 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -32,6 +32,7 @@ PATH="${builddir}:$PATH"
 export PATH
 
 trim=0
+trim_head=664cbea8-132f-4770-8aa4-1696d59ac35c
 
 # Usage: usage
 # Print the usage.
@@ -226,8 +227,13 @@ for option in "$@"; do
echo "$0 (GNU GRUB ${PACKAGE_VERSION})"
exit 0 ;;
 --trim)
-   trim=1
+   trim=1 ;;
+--trim=*)
+   trim=2
+   trim_head=`echo "$option" | sed -e 's/--trim=//' -e 's/,/ /g'`
;;
+--no-trim)
+   trim=0 ;;
 --debug)
 debug=1 ;;
 --modules=*)
@@ -353,8 +359,6 @@ terminal_input ${term}
 terminal_output ${term}
 EOF
 
-trim_head=664cbea8-132f-4770-8aa4-1696d59ac35c
-
 if [ $trim = 1 ]; then
 echo "echo $trim_head" >>${cfgfile}
 fi
@@ -479,8 +483,8 @@ fi
 
 do_trim ()
 {
-if [ $trim = 1 ]; then
-   awk '{ if (have_head == 1) print $0; } 
/664cbea8-132f-4770-8aa4-1696d59ac35c/ { have_head=1; }'
+if [ $trim = 1 ] || [ $trim = 2 ]; then
+   awk '{ if (have_head == 1) print $0; } /'"$trim_head"'/ { have_head=1; 
}'
 else
cat
 fi
-- 
2.34.1


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