Re: Bad shim signature on kernel loading with patchset from 25.05.2023 and up

[Thomas Frauendorfer]

On Thu, Jun 22, 2023 at 3:00 PM Tobias Powalowski via Grub-devel
 wrote:
>
> Hi,
> just curious since the patchset from 25.05.2023, I cannot use Secure Boot on 
> my project anymore. The kernel hash cannot be validated anymore and shim 
> bails out with bad shim signature.
> Any help would be appreciated.
> greetings
> tpowa

According to shim documentation the shim lock protocol should use SysV
abi instead of MS abi.
So could you try if it solves your issue if you remove
"__grub_efi_api" from the verify function of
grub_efi_shim_lock_protocol in include/grub/efi/api.h in around line
1780

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

Re: Bad shim signature on kernel loading with patchset from 25.05.2023 and up

[Tobias Powalowski via Grub-devel]

Hi,
removing it removes the error, but boot does not happen then. It errors now
with: cannot load image.
greetings
tpowa

Am Mo., 26. Juni 2023 um 09:06 Uhr schrieb Thomas Frauendorfer <
thomas.frauendor...@gmail.com>:

> On Thu, Jun 22, 2023 at 3:00 PM Tobias Powalowski via Grub-devel
>  wrote:
> >
> > Hi,
> > just curious since the patchset from 25.05.2023, I cannot use Secure
> Boot on my project anymore. The kernel hash cannot be validated anymore and
> shim bails out with bad shim signature.
> > Any help would be appreciated.
> > greetings
> > tpowa
>
> According to shim documentation the shim lock protocol should use SysV
> abi instead of MS abi.
> So could you try if it solves your issue if you remove
> "__grub_efi_api" from the verify function of
> grub_efi_shim_lock_protocol in include/grub/efi/api.h in around line
> 1780
>


-- 
Tobias Powalowski
Arch Linux Developer & Package Maintainer (tpowa)
https://www.archlinux.org 
tp...@archlinux.org
Archboot Developer
https://archboot.com

St. Martin-Apotheke
Herzog-Georg-Str. 25
89415 Lauingen
https://www.st-martin-apo.de
i...@st-martin-apo.de
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Bad shim signature on kernel loading with patchset from 25.05.2023 and up

[Daniel Kiper]

Hey,

Thomas, good point. I completely missed that.

Tobias, please try Ard patch with the change suggested by Thomas. If you
have not done that yet...

Daniel

On Mon, Jun 26, 2023 at 09:49:19AM +0200, Tobias Powalowski via Grub-devel 
wrote:
> Hi,
> removing it removes the error, but boot does not happen then. It errors now
> with: cannot load image.
> greetings
> tpowa
>
> Am Mo., 26. Juni 2023 um 09:06 Uhr schrieb Thomas Frauendorfer
> :
>  On Thu, Jun 22, 2023 at 3:00 PM Tobias Powalowski via Grub-devel
>   wrote:
>  >
>  > Hi,
>  > just curious since the patchset from 25.05.2023, I cannot use
>  Secure Boot on my project anymore. The kernel hash cannot be
>  validated anymore and shim bails out with bad shim signature.
>  > Any help would be appreciated.
>  > greetings
>  > tpowa
>
>  According to shim documentation the shim lock protocol should use
>  SysV
>  abi instead of MS abi.
>  So could you try if it solves your issue if you remove
>  "__grub_efi_api" from the verify function of
>  grub_efi_shim_lock_protocol in include/grub/efi/api.h in around line
>  1780

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

Re: Bad shim signature on kernel loading with patchset from 25.05.2023 and up

[Tobias Powalowski via Grub-devel]

Am Mo., 26. Juni 2023 um 14:28 Uhr schrieb Daniel Kiper :

> Hey,
>
> Thomas, good point. I completely missed that.
>
> Tobias, please try Ard patch with the change suggested by Thomas. If you
> have not done that yet...
>
> Daniel
>
> Hi Daniel, Thomas and Ard,
yes both together work :)
Sorry haven't tested it earlier with both enabled.

Attached is my used patch.

greetings
tpowa

-- 
Tobias Powalowski
Arch Linux Developer & Package Maintainer (tpowa)
https://www.archlinux.org 
tp...@archlinux.org
Archboot Developer
https://archboot.com

St. Martin-Apotheke
Herzog-Georg-Str. 25
89415 Lauingen
https://www.st-martin-apo.de
i...@st-martin-apo.de
--- a/include/grub/efi/api.h	2023-06-26 09:22:48.229322948 +0200
+++ b/include/grub/efi/api.h	2023-06-26 09:22:55.046025291 +0200
@@ -1777,7 +1777,7 @@
 
 struct grub_efi_shim_lock_protocol
 {
-  grub_efi_status_t (__grub_efi_api *verify) (void *buffer, grub_uint32_t size);
+  grub_efi_status_t (*verify) (void *buffer, grub_uint32_t size);
 };
 typedef struct grub_efi_shim_lock_protocol grub_efi_shim_lock_protocol_t;
 
diff --git a/grub-core/loader/efi/linux.c b/grub-core/loader/efi/linux.c
index c1eef7c9865f5d0d..49b16933809dbf44 100644
--- a/grub-core/loader/efi/linux.c
+++ b/grub-core/loader/efi/linux.c
@@ -18,6 +18,7 @@

 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -458,6 +459,11 @@ grub_cmd_linux (grub_command_t cmd __attribute__((unused)),

   grub_dl_ref (my_mod);

+#if defined(__i386__)  || defined(__x86_64__)
+  if (grub_env_get ("shim_lock") != NULL)
+return grub_cmd_linux_x86_legacy (cmd, argc, argv);
+#endif
+
   if (argc == 0)
 {
   grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));

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

Re: Bad shim signature on kernel loading with patchset from 25.05.2023 and up

[Daniel Kiper]

On Mon, Jun 26, 2023 at 03:14:18PM +0200, Tobias Powalowski via Grub-devel 
wrote:
> Am Mo., 26. Juni 2023 um 14:28 Uhr schrieb Daniel Kiper :
>  Hey,
>
>  Thomas, good point. I completely missed that.
>
>  Tobias, please try Ard patch with the change suggested by Thomas. If
>  you
>  have not done that yet...
>
>  Daniel
>
> Hi Daniel, Thomas and Ard,
> yes both together work :)

Great! Thanks a lot for the report and testing. I will prepare fixes in
the following days. They can be different then the ones proposed here
due to other reports about LoadFile2 failures. You will be CC-ed.

> Sorry haven't tested it earlier with both enabled.

No worries.

Daniel

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

Re: Bad shim signature on kernel loading with patchset from 25.05.2023 and up

[Ard Biesheuvel]

On Mon, 26 Jun 2023 at 15:56, Daniel Kiper  wrote:
>
> On Mon, Jun 26, 2023 at 03:14:18PM +0200, Tobias Powalowski via Grub-devel 
> wrote:
> > Am Mo., 26. Juni 2023 um 14:28 Uhr schrieb Daniel Kiper 
> > :
> >  Hey,
> >
> >  Thomas, good point. I completely missed that.
> >
> >  Tobias, please try Ard patch with the change suggested by Thomas. If
> >  you
> >  have not done that yet...
> >

Which change is that? I am seeing only half of the discussion.


> >  Daniel
> >
> > Hi Daniel, Thomas and Ard,
> > yes both together work :)
>
> Great! Thanks a lot for the report and testing. I will prepare fixes in
> the following days. They can be different then the ones proposed here
> due to other reports about LoadFile2 failures. You will be CC-ed.
>
> > Sorry haven't tested it earlier with both enabled.
>
> No worries.
>
> Daniel

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

Re: Bad shim signature on kernel loading with patchset from 25.05.2023 and up

[Daniel Kiper]

On Mon, Jun 26, 2023 at 03:58:15PM +0200, Ard Biesheuvel wrote:
> On Mon, 26 Jun 2023 at 15:56, Daniel Kiper  wrote:
> >
> > On Mon, Jun 26, 2023 at 03:14:18PM +0200, Tobias Powalowski via Grub-devel 
> > wrote:
> > > Am Mo., 26. Juni 2023 um 14:28 Uhr schrieb Daniel Kiper 
> > > :
> > >  Hey,
> > >
> > >  Thomas, good point. I completely missed that.
> > >
> > >  Tobias, please try Ard patch with the change suggested by Thomas. If
> > >  you
> > >  have not done that yet...
> > >
>
> Which change is that? I am seeing only half of the discussion.

Sorry, removal of unnecessary __grub_efi_api from the verify function of
grub_efi_shim_lock_protocol. Here are details [1].

Daniel

[1] https://lists.gnu.org/archive/html/grub-devel/2023-06/msg00149.html

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

Re: Bad shim signature on kernel loading with patchset from 25.05.2023 and up

[Ard Biesheuvel]

On Mon, 26 Jun 2023 at 16:14, Daniel Kiper  wrote:
>
> On Mon, Jun 26, 2023 at 03:58:15PM +0200, Ard Biesheuvel wrote:
> > On Mon, 26 Jun 2023 at 15:56, Daniel Kiper  wrote:
> > >
> > > On Mon, Jun 26, 2023 at 03:14:18PM +0200, Tobias Powalowski via 
> > > Grub-devel wrote:
> > > > Am Mo., 26. Juni 2023 um 14:28 Uhr schrieb Daniel Kiper 
> > > > :
> > > >  Hey,
> > > >
> > > >  Thomas, good point. I completely missed that.
> > > >
> > > >  Tobias, please try Ard patch with the change suggested by Thomas. 
> > > > If
> > > >  you
> > > >  have not done that yet...
> > > >
> >
> > Which change is that? I am seeing only half of the discussion.
>
> Sorry, removal of unnecessary __grub_efi_api from the verify function of
> grub_efi_shim_lock_protocol. Here are details [1].
>

Ugh, really? EFI protocols using SysV calling convention? What a mess.

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

Re: [PATCH v2] docs: Minor edits to debugging chapter

[Glenn Washburn]

On Sun, 25 Jun 2023 14:27:57 -0500
Oskari Pirhonen  wrote:

> Small set of wording and grammatical edits which did not make it in time
> for the original review of the chapter.

LGTM.

Reviewed-by: Glenn Washburn 

Glenn

> 
> Signed-off-by: Oskari Pirhonen 
> ---
> v2:
> - apply Glenn's suggestions
> 
> Interdiff against v1:
>   diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi
>   index 05a19c26d..0834ca562 100644
>   --- a/docs/grub-dev.texi
>   +++ b/docs/grub-dev.texi
>   @@ -766,8 +766,8 @@ be loading the GRUB image into memory where every byte 
> is already set to 0.
>This means that if a breakpoint is set before GRUB is loaded, GDB will save
>the 0-byte(s) where the the special instruction will go. Then when the 
> firmware
>loads the GRUB image and because it is unaware of the debugger, it will
>   -write the GRUB image to memory, overwriting anything that was there 
> previously.
>   -Notably in this case the instruction that implements the software 
> breakpoint.
>   +write the GRUB image to memory, overwriting anything that was there 
> previously ---
>   +notably in this case the instruction that implements the software 
> breakpoint.
>This will be confusing for the person using GDB because GDB will show the
>breakpoint as set, but the brekapoint will never be hit. Furthermore, GDB
>then becomes confused, such that even deleting an recreating the breakpoint
>   @@ -783,8 +783,8 @@ the CPU. So they can always be set freely without 
> regard to whether GRUB has
>been loaded or not. The reason that hardware breakpoints aren't always used
>is because there are a limited number of them, usually around 4 on various
>CPUs, and specifically exactly 4 for x86 CPUs. The @file{gdb_grub} script 
> goes
>   -out of its way to avoid using hardware breakpoints internally, and when 
> needed,
>   -uses them as briefly as possible, thus allowing the user to have a maximal
>   +out of its way to avoid using hardware breakpoints internally and uses 
> them as
>   +briefly as possible when needed, thus allowing the user to have a maximal
>number at their disposal.
>
>@node OVMF debug log
> 
>  docs/grub-dev.texi | 37 ++---
>  1 file changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi
> index 72470b42c..0834ca562 100644
> --- a/docs/grub-dev.texi
> +++ b/docs/grub-dev.texi
> @@ -658,11 +658,11 @@ 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}.
> +script which does much of the setup. This file is at 
> @file{grub-core/gdb_grub}
> +in 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
> +@samp{grub-pc-dbg} package. The GDB script is intended to be used
>  like so, assuming:
>  
>  @example
> @@ -719,14 +719,13 @@ expected when breaking on functions, but, for instance, 
> global variables
>  will point to the wrong address in memory and thus give incorrect values
>  (which can be difficult to debug).
>  
> -The calculating of the correct offsets for sections when loading symbol
> -files are taken care of when loading the kernel symbols via the user-defined
> -GDB command @command{dynamic_load_kernel_exec_symbols}, which takes one
> -argument, the address where the text section is loaded, as determined by
> -one of the methods above. Alternatively, the command 
> @command{dynamic_load_symbols}
> -with the text section address as an agrument can be called to load the
> -kernel symbols and setup loading the module symbols as they are loaded at
> -runtime.
> +Calculating the correct offsets for sections is taken care of automatically
> +when loading the kernel symbols via the user-defined GDB command
> +@command{dynamic_load_kernel_exec_symbols}, which takes one argument, the
> +address where the text section is loaded as determined by one of the methods
> +above. Alternatively, the command @command{dynamic_load_symbols} with the 
> text
> +section address as an agrument can be called to load the kernel symbols and 
> set
> +up loading the module symbols as they are loaded at runtime.
>  
>  In the author's experience, when debugging with QEMU and OVMF, to have
>  debugging symbols loaded at the start of GRUB2 execution the GRUB2 EFI
> @@ -736,7 +735,7 @@ two subsections below. Generally speaking, the load 
> address does not change
>  between QEMU runs.