[PATCH] tpm: Enable boot despite unknown firmware failure

2023-06-14 Thread Michał Grzelak
Currently booting the system is prevented when call to EFI firmware
hash_log_extend_event() returns unknown error. Solve this by following
convention used in commit a4356538d (commands/tpm: Don't propagate
measurement failures to the verifiers layer).

Let the system to be bootable by default when unknown TPM error is
encountered. Make grub_tpm_is_fail_fatal() global and move its body to
efi/tpm.c. Check environment variable tpm_fail_fatal to fallback to
previous behaviour.

Signed-off-by: Michał Grzelak 
---
 grub-core/commands/efi/tpm.c | 10 +-
 grub-core/commands/tpm.c | 11 ++-
 include/grub/tpm.h   |  1 +
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/grub-core/commands/efi/tpm.c b/grub-core/commands/efi/tpm.c
index c616768f9..05f3064f7 100644
--- a/grub-core/commands/efi/tpm.c
+++ b/grub-core/commands/efi/tpm.c
@@ -18,6 +18,7 @@
  *  EFI TPM support code.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -146,7 +147,8 @@ grub_efi_log_event_status (grub_efi_status_t status)
 case GRUB_EFI_NOT_FOUND:
   return grub_error (GRUB_ERR_UNKNOWN_DEVICE, N_("TPM unavailable"));
 default:
-  return grub_error (GRUB_ERR_UNKNOWN_DEVICE, N_("unknown TPM error"));
+  return grub_error (grub_tpm_is_fail_fatal () ? GRUB_ERR_UNKNOWN_DEVICE
+ : GRUB_ERR_NONE, N_("unknown TPM error"));
 }
 }
 
@@ -266,6 +268,12 @@ grub_cc_log_event (unsigned char *buf, grub_size_t size, 
grub_uint8_t pcr,
 grub_efi_log_event_status (status);
 }
 
+bool
+grub_tpm_is_fail_fatal (void)
+{
+  return grub_env_get_bool ("tpm_fail_fatal", false);
+}
+
 grub_err_t
 grub_tpm_measure (unsigned char *buf, grub_size_t size, grub_uint8_t pcr,
const char *description)
diff --git a/grub-core/commands/tpm.c b/grub-core/commands/tpm.c
index 9f830916d..f86bdc81e 100644
--- a/grub-core/commands/tpm.c
+++ b/grub-core/commands/tpm.c
@@ -18,7 +18,6 @@
  *  Core TPM support code.
  */
 
-#include 
 #include 
 #include 
 #include 
@@ -40,12 +39,6 @@ grub_tpm_verify_init (grub_file_t io,
   return GRUB_ERR_NONE;
 }
 
-static inline bool
-is_tpm_fail_fatal (void)
-{
-  return grub_env_get_bool ("tpm_fail_fatal", false);
-}
-
 static grub_err_t
 grub_tpm_verify_write (void *context, void *buf, grub_size_t size)
 {
@@ -55,7 +48,7 @@ grub_tpm_verify_write (void *context, void *buf, grub_size_t 
size)
 return GRUB_ERR_NONE;
 
   grub_dprintf ("tpm", "Measuring buffer failed: %d\n", status);
-  return is_tpm_fail_fatal () ? status : GRUB_ERR_NONE;
+  return grub_tpm_is_fail_fatal () ? status : GRUB_ERR_NONE;
 }
 
 static grub_err_t
@@ -91,7 +84,7 @@ grub_tpm_verify_string (char *str, enum 
grub_verify_string_type type)
 return GRUB_ERR_NONE;
 
   grub_dprintf ("tpm", "Measuring string %s failed: %d\n", str, status);
-  return is_tpm_fail_fatal () ? status : GRUB_ERR_NONE;
+  return grub_tpm_is_fail_fatal () ? status : GRUB_ERR_NONE;
 }
 
 struct grub_file_verifier grub_tpm_verifier = {
diff --git a/include/grub/tpm.h b/include/grub/tpm.h
index c19fcbd0a..aea145dd1 100644
--- a/include/grub/tpm.h
+++ b/include/grub/tpm.h
@@ -36,5 +36,6 @@
 
 grub_err_t grub_tpm_measure (unsigned char *buf, grub_size_t size,
 grub_uint8_t pcr, const char *description);
+bool grub_tpm_is_fail_fatal (void);
 int grub_tpm_present (void);
 #endif
-- 
2.37.3


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


Re: [PATCH v4 3/5] efi: Drop all uses of efi_call_XX wrappers

2023-06-14 Thread Daniel Kiper
On Mon, Jun 12, 2023 at 10:53:09PM +0200, Robin Candau wrote:
> Le 12/06/2023 à 21:05, Daniel Kiper a écrit :
> > On Mon, Jun 12, 2023 at 07:19:53PM +0200, Christian Hesse wrote:
> > > Ard Biesheuvel  on Tue, 2023/05/23 17:31:
> > > > Now that GCC can generate function calls using the correct calling
> > > > convention for us, we can stop using the efi_call_XX () wrappers, and
> > > > just dereference the function pointers directly.
> > > >
> > > > This avoids the untyped variadic wrapper routines, which means better
> > > > type checking for the method calls.
> > > Building an Arch package from current git master makes the system 
> > > unbootable
> > > with just a black screen. We did biscect this and found this commit
> > > (bb4aa6e06ee3877110a1dc0eb0d766ffac978993 in master) to be the first bad 
> > > one.
> > >
> > > As gcc is mentioned in the commit message... We are building with gcc
> > > 13.1.1-1. Any idea what is going on here?
> > Could you provide us more details? On which architecture/target do
> > you run tests? Does GRUB start? If yes which kernel version do you
> > try to boot? Could you send us a link to the GRUB and kernel packages
> > and sources? Etc...
> >
> > Daniel
>
> Hi,
>
> Tests have been ran on a x86_64 laptop (but the issue has been reproduced on
> several other x86_64 machines). GRUB does not start, we don't even reach the
> GRUB menu, nothing more than a black screen at boot.
> The kernel we're trying to boot is `linux 6.3.7-arch1-1` [1] (package [2]),
> but I doubt the kernel itself is in cause as we're able to boot it with a
> GRUB package built without the bb4aa6e06ee3877110a1dc0eb0d766ffac978993
> commit. Here's a working GRUB package [3] and a faulty one [4].
> Sources are taken from https://git.savannah.gnu.org/git/grub.git [5].
>
> I remain available if you need specific tests or additional information.
>
> [1] https://archlinux.org/packages/core/x86_64/linux/
> [2] 
> http://www.gtlib.gatech.edu/pub/archlinux/core/os/x86_64/linux-6.3.7.arch1-1-x86_64.pkg.tar.zst
> [3] 
> https://pkgbuild.com/~eworm/grub/grub-2:2.06.r535.g6a080b9cd-2-x86_64.pkg.tar.zst
> [4] 
> https://pkgbuild.com/~eworm/grub/grub-2:2.06.r554.gc016a969d-2-x86_64.pkg.tar.zst
> [5] 
> https://gitlab.archlinux.org/archlinux/packaging/packages/grub/-/blob/main/PKGBUILD#L60

It looks I was able to reproduce this problem. I will be checking what
is going on in following days...

Daniel

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


[PATCH v4 1/1] loongarch: disable relaxation relocations

2023-06-14 Thread Xiaotian Wu
A working grub cannot be built with upcoming binutils and gcc, because linker
relaxation was added [1] causing new unsupported relocations to appear in 
modules.

So we pass -mno-relax to gcc if it is supported, to disable relaxation and make
grub forward-compatible with new toolchains.

[1]: 
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=56576f4a722b7398d35802ecf7d4185c27d6d69b

Signed-off-by: Xiaotian Wu 
---
 configure.ac | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/configure.ac b/configure.ac
index d9f088d12..a0ea5beae 100644
--- a/configure.ac
+++ b/configure.ac
@@ -874,6 +874,29 @@ if test "x$target_cpu" = xloongarch64; then
 TARGET_CFLAGS="$TARGET_CFLAGS -mno-explicit-relocs -fno-plt"
 TARGET_CCASFLAGS="$TARGET_CCASFLAGS -mno-explicit-relocs -fno-plt"
   fi
+
+  AC_CACHE_CHECK([for no-relax options], grub_cv_target_cc_mno_relax, [
+grub_cv_target_cc_mno_relax=no
+for cand in "-mno-relax" "-Wa,-mno-relax"; do
+  if test x"$grub_cv_target_cc_mno_relax" != xno ; then
+break
+  fi
+  CFLAGS="$TARGET_CFLAGS $cand -Werror"
+  AC_LINK_IFELSE([AC_LANG_PROGRAM([[
+   asm (".globl start; start:");
+   void __main (void);
+   void __main (void) {}
+   int main (void);
+   ]], [[]])], [grub_cv_target_cc_mno_relax="$cand"], [])
+done
+  ])
+  CFLAGS="$TARGET_CFLAGS"
+
+  if test x"$grub_cv_target_cc_mno_relax" != xno ; then
+TARGET_CFLAGS="$TARGET_CFLAGS $grub_cv_target_cc_mno_relax"
+TARGET_CCASFLAGS="$TARGET_CCASFLAGS $grub_cv_target_cc_mno_relax"
+  fi
+
   TARGET_CFLAGS="$TARGET_CFLAGS -Wa,-mla-global-with-abs"
   TARGET_CCASFLAGS="$TARGET_CCASFLAGS -Wa,-mla-global-with-abs"
 fi
-- 
2.40.1


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


[PATCH v4 0/1] loongarch: Disable relaxation for grub

2023-06-14 Thread Xiaotian Wu
A working grub cannot be built with upcoming binutils and gcc, because linker
relaxation was added [1] causing new unsupported relocations to appear in 
modules.

So we pass -mno-relax to gcc if it is supported, to disable relaxation and make
grub forward-compatible with new toolchains.

[1]: 
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=56576f4a722b7398d35802ecf7d4185c27d6d69b

v3->v4:
- rebase on the latest master branch
- check "-mno-relax" and "-Wa,-mno-relax"

v2->v3:
- Use the -mno-relax cflags for gcc

v1->v2:
- split patch
- drop cast code


Xiaotian Wu (1):
  loongarch: disable relaxation relocations

 configure.ac | 23 +++
 1 file changed, 23 insertions(+)

-- 
2.40.1


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


Re: [PATCH v3 5/5] loongarch: Use the -mno-relax cflags for gcc

2023-06-14 Thread Xiaotian Wu
在 2023-06-13星期二的 20:09 +0800,Xi Ruoyao via Grub-devel写道:
> On Tue, 2023-06-13 at 13:39 +0200, Daniel Kiper wrote:
> > On Tue, Jun 13, 2023 at 05:06:35PM +0800, Xiaotian Wu wrote:
> > > Because the binutils of the loongarch architecture adds
> > > relaxation
> > > support [1],
> > > the next version of binutils will not be able to build grub.
> > > 
> > > So we use the -mno-relax cflags to disable gcc to generate
> > > relaxation
> > > relocations to enhance the compatibility of grub.
> > > 
> > > [1]:
> > > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=56576f4a722b7398d35802ecf7d4185c27d6d69b
> > > 
> > > Signed-off-by: Xiaotian Wu 
> > > ---
> > >  configure.ac | 12 
> > >  1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/configure.ac b/configure.ac
> > > index d9f088d12..cd3cc6ba8 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -874,6 +874,18 @@ if test "x$target_cpu" = xloongarch64; then
> > >  TARGET_CFLAGS="$TARGET_CFLAGS -mno-explicit-relocs -fno-plt"
> > >  TARGET_CCASFLAGS="$TARGET_CCASFLAGS -mno-explicit-relocs -
> > > fno-
> > > plt"
> > >    fi
> > > +
> > > +  AC_CACHE_CHECK([whether _mno_relax works],
> > > [grub_cv_cc_mno_relax], [
> > > +    CFLAGS="$TARGET_CFLAGS -mno-relax -Werror"
> > > +    AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
> > > +   [grub_cv_cc_mno_relax=yes],
> > > +   [grub_cv_cc_mno_relax=no])
> > > +  ])
> > > +  if test "x$grub_cv_cc_mno_relax" = xyes; then
> > > +    TARGET_CFLAGS="$TARGET_CFLAGS -mno-relax"
> > > +    TARGET_CCASFLAGS="$TARGET_CCASFLAGS -mno-relax"
> > > +  fi
> > 
> > Could not you reuse sparc64 code which is ~100 lines below? If not
> > please make loongarch code as similar as possible to the sparc64
> > one.
> 
> More important reason is we need to try -Wa,-mno-relax along with -
> mno-
> relax, like SPARC64.  GCC 13 and earlier does not accept -mno-relax,
> but
> the assembler itself can still generate R_LARCH_RELAX etc.  So if a
> distro uses GCC 13 alongside Binutils 2.41, we'll need -Wa,-mno-
> relax.
> 
> Note that Binutils 2.41 will be released in Feb 2024 but GCC 14 will
> be
> released in mid 2024 (Apr to Jun) so it can be expected that some
> distros will really use Binutils 2.41 and GCC 13.

Code for sparc64 cannot be reused.
Because sparc64 sets LDFLAGS, and loongarch needs to set CFLAGS.
If only LDFLAGS is used on loongarch, gcc still generates .o files with
RELAX.

This point was also mentioned in mengqinggang's email:
https://lists.gnu.org/archive/html/grub-devel/2023-06/msg00062.html

> 
> -- 
> Xi Ruoyao 
> School of Aerospace Science and Technology, Xidian University
> 

-- 
Best Regards
Xiaotian Wu


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


Re: [PATCH v3] docs: Add debugging chapter to development documentation

2023-06-14 Thread Oskari Pirhonen
Oops, apologies for the late reply. Reading through it again, I found a
few more small nits:

On Tue, Jun 06, 2023 at 00:48:39 -0500, Glenn Washburn wrote:
> 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 
> ---
> Changes from v1:
>  * Add gdbinfo section
> ---
> Interdiff against v2:
>   diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi
>   index 188ca9c7ca6e..72470b42c61a 100644
>   --- a/docs/grub-dev.texi
>   +++ b/docs/grub-dev.texi
>   @@ -638,7 +638,7 @@ various targets using @command{gdb} and the 
> @samp{gdb_grub} GDB script.
>@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
>   +because in some respects it's 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:
>   @@ -688,11 +688,11 @@ it does add the module symbols with the appropriate 
> offset.
>@section x86_64-efi
>
>Using GDB to debug GRUB2 for the x86_64-efi target has some similarities 
> with
>   -the i386-pc target. Please read be familiar with the @ref{i386-pc} section
>   -when reading this one. Extra care must be used to run QEMU such that it 
> boots
>   -a UEFI firmware. This usually involves either using the @samp{-bios} option
>   -with a UEFI firmware blob (eg. @file{OVMF.fd}) or loading the firmware via
>   -pflash. This document will not go further into how to do this as there are
>   +the i386-pc target. Please read and familiarize yourself with the 
> @ref{i386-pc}
>   +section when reading this one. Extra care must be used to run QEMU such 
> that it
>   +boots a UEFI firmware. This usually involves either using the @samp{-bios}
>   +option with a UEFI firmware blob (eg. @file{OVMF.fd}) or loading the 
> firmware
>   +via pflash. This document will not go further into how to do this as there 
> are
>ample resource on the web.
>
>Like all EFI implementations, on x86_64-efi the (U)EFI firmware that loads
>   @@ -700,7 +700,7 @@ the GRUB2 EFI application determines at runtime where 
> the application will
>be loaded. This means that we do not know where to tell GDB to load the
>symbols for the GRUB2 core until the (U)EFI firmware determines it. There 
> are
>two good ways of figuring this out when running in QEMU: use a @ref{OVMF 
> debug log,
>   -debug build of OVMF} and check the debug log or have GRUB2 say where it is
>   +debug build of OVMF} and check the debug log, or have GRUB2 say where it is
>loaded. Neither of these are ideal because they both generally give the
>information after GRUB2 is already running, which makes debugging early 
> boot
>infeasible. Technically, the first method does give the load address before
>   @@ -734,11 +734,11 @@ application must be run via QEMU at least once prior 
> in order to get the
>load address. Two methods for obtaining the load address are described in
>two subsections below. Generally speaking, the load address does not change
>between QEMU runs. There are exceptions to this, namely that different
>   -GRUB2 EFI applications can be run at different addresses. Also, its been
>   +GRUB2 EFI applications can be run at different addresses. Also, it has been
>observed that after running the EFI application for the first time, the
>second run will some times have a different load address, but subsequent
>runs of the same EFI application will have the same load address as the
>   -second run. And its a near certainty that if the GRUB EFI binary has 
> changed,
>   +second run. And it's a near certainty that if the GRUB EFI binary has 
> changed,
>eg. been recompiled, the load address will also be different.
>
>This ability to predict what the load address will be allows one to assume
>   @@ -752,7 +752,7 @@ gdb -x gdb_grub -ex 'dynamic_load_symbols @var{address 
> of .text section}'
>@end example
>
>If you load the symbols in this manner and, after continuing execution, do
>   -not see output showing the loading of modules symbol, then its very likely
>   +not see output showing the loading of modules symbol, then it is very 
> likely
>that the load address was incorrect.
>
>Another thing to be aware of is how the loading of the GRUB image by the
>   @@ -760,8 +760,8 @@ firmware affects previously set software breakpoints. 
> On x86 platforms,
>software breakpoints are implemented by GDB by writing a special processor
>instruction at the location of the desired breakpoint. This special 
> instruction
>when executed will stop the program execution and hand control to the
>   -debugger, GDB. GDB will first saves