[PATCH v3 0/5] Fix coverity bugs and add checks for elf values in grub-core

[Alec Brown]

v3: Added check for e_shoff, made starting words lowercase in error messages,
and added comment to why return pointers are set to 0.

Coverity identified several untrusted loop bounds and untrusted allocation size
bugs in grub-core/loader/i386/bsdXX.c and grub-core/loader/multiboot_elfXX.c.
Upon review of these bugs, I found that specific checks weren't being made to
various elf header values based on the elf manual page. This patch series
addresses the coverity bugs, as well as adds functions to check for the correct
elf header values.

The Coverity bugs being addressed are:
CID 314018
CID 314030
CID 314031
CID 314039

Alec Brown (5):
  grub-core/loader/i386/bsdXX.c: Avoid downcasting (char *) to (Elf_Shdr *)
  elf: Validate number of elf section header table entries
  elf: Validate elf section header table index for section name string table
  elf: Validate number of elf program header table entries
  util/grub-module-verifierXX.c: Add e_shoff check in get_shdr()

 grub-core/kern/elf.c   |  15 +++
 grub-core/kern/elfXX.c | 101 
+
 grub-core/loader/i386/bsdXX.c  | 137 
+
 grub-core/loader/multiboot_elfxx.c |  76 
+++-
 include/grub/elf.h |  18 ++
 util/grub-module-verifierXX.c  |   3 +++
 6 files changed, 273 insertions(+), 77 deletions(-)


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

[PATCH v3 4/5] elf: Validate number of elf program header table entries

[Alec Brown]

In bsdXX.c and multiboot_elfXX.c, e_phnum is used to obtain the number of
program header table entries, but it wasn't being checked if the value was
there.

According to the elf(5) manual page,
"If the number of entries in the program header table is larger than or equal to
PN_XNUM (0x), this member holds PN_XNUM (0x) and the real number of
entries in the program header table is held in the sh_info member of the
initial entry in section header table.  Otherwise, the sh_info member of the
initial entry contains the value zero."

Since this check wasn't being made, grub_elfXX_get_phnum() is being added to
elfXX.c to make this check and use e_phnum if it doesn't have PN_XNUM as a
value, else use sh_info. We also need to make sure e_phnum isn't greater than
PN_XNUM and sh_info isn't less than PN_XNUM.

Note that even though elf.c and elfXX.c are located in grub-core/kern, they are
compiled as modules and don't need the EXPORT_FUNC macro to define the functions
in elf.h.

Signed-off-by: Alec Brown 
---
 grub-core/kern/elf.c   |  3 +++
 grub-core/kern/elfXX.c | 34 ++
 grub-core/loader/i386/bsdXX.c  | 11 +++---
 grub-core/loader/multiboot_elfxx.c | 19 +++--
 include/grub/elf.h |  5 +
 5 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/grub-core/kern/elf.c b/grub-core/kern/elf.c
index 19440a318..ff5655206 100644
--- a/grub-core/kern/elf.c
+++ b/grub-core/kern/elf.c
@@ -176,6 +176,7 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #define grub_elfXX_check_endianess_and_bswap_ehdr 
grub_elf32_check_endianess_and_bswap_ehdr
 #define grub_elfXX_get_shnum grub_elf32_get_shnum
 #define grub_elfXX_get_shstrndx grub_elf32_get_shstrndx
+#define grub_elfXX_get_phnum grub_elf32_get_phnum
 
 #include "elfXX.c"
 
@@ -198,6 +199,7 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #undef grub_elfXX_check_endianess_and_bswap_ehdr
 #undef grub_elfXX_get_shnum
 #undef grub_elfXX_get_shstrndx
+#undef grub_elfXX_get_phnum
 
 
 /* 64-bit */
@@ -220,5 +222,6 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #define grub_elfXX_check_endianess_and_bswap_ehdr 
grub_elf64_check_endianess_and_bswap_ehdr
 #define grub_elfXX_get_shnum grub_elf64_get_shnum
 #define grub_elfXX_get_shstrndx grub_elf64_get_shstrndx
+#define grub_elfXX_get_phnum grub_elf64_get_phnum
 
 #include "elfXX.c"
diff --git a/grub-core/kern/elfXX.c b/grub-core/kern/elfXX.c
index d09b37213..a917e7cce 100644
--- a/grub-core/kern/elfXX.c
+++ b/grub-core/kern/elfXX.c
@@ -272,3 +272,37 @@ grub_elfXX_get_shstrndx (ElfXX_Ehdr *e, grub_uint32_t 
*shstrndx)
 }
   return GRUB_ERR_NONE;
 }
+
+grub_err_t
+grub_elfXX_get_phnum (ElfXX_Ehdr *e, grub_uint32_t *phnum)
+{
+  ElfXX_Shdr *s;
+
+  if (phnum == NULL)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for 
phnum"));
+
+  /* Set *phnum to 0 so that phnum doesn't return junk on error */
+  *phnum = 0;
+
+  if (e == NULL)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for elf 
header"));
+
+  *phnum = e->e_phnum;
+  if (*phnum == PN_XNUM)
+{
+  if (e->e_shoff == 0)
+   return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid section header 
table offset in e_shoff"));
+
+  s = (ElfXX_Shdr *) ((grub_uint8_t *) e + e->e_shoff);
+  *phnum = s->sh_info;
+  if (*phnum < PN_XNUM)
+   return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid number of program 
header table entries in sh_info: %d"), *phnum);
+}
+  else
+{
+  if (*phnum >= PN_XNUM)
+   return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid number of program 
header table entries in e_phnum: %d"), *phnum);
+}
+
+  return GRUB_ERR_NONE;
+}
diff --git a/grub-core/loader/i386/bsdXX.c b/grub-core/loader/i386/bsdXX.c
index 7d9b65d1a..eb1164dd7 100644
--- a/grub-core/loader/i386/bsdXX.c
+++ b/grub-core/loader/i386/bsdXX.c
@@ -185,6 +185,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator 
*relocator,
   Elf_Shdr *s;
   Elf_Shdr *shdr = NULL;
   Elf_Word shnum;
+  grub_uint32_t phnum;
   grub_addr_t curload, module;
   grub_err_t err;
   grub_size_t chunk_size = 0;
@@ -200,6 +201,10 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct 
grub_relocator *relocator,
   if (err != GRUB_ERR_NONE)
 goto out;
 
+  err = grub_elf_get_phnum (&e, &phnum);
+  if (err != GRUB_ERR_NONE)
+goto out;
+
   for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + shnum * 
e.e_shentsize);
s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
 {
@@ -214,7 +219,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator 
*relocator,
 
   if (chunk_size < sizeof (e))
 chunk_size = sizeof (e);
-  chunk_size += (grub_uint32_t) e.e_phnum * e.e_phentsize;
+  chunk_size += (grub_size_t) phnum * e.e_phentsize;
   chunk_size += (grub_size_t) shnum * e.e_shentsize;
 
   {
@@ -271,9 +276,9 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_re

[PATCH v3 2/5] elf: Validate number of elf section header table entries

[Alec Brown]

In bsdXX.c and multiboot_elfXX.c, e_shnum is used to obtain the number of
section header table entries, but it wasn't being checked if the value was
there.

According to the elf(5) manual page,
"If the number of entries in the section header table is larger than or equal to
SHN_LORESERVE (0xff00), e_shnum holds the value zero and the real number of
entries in the section header table is held in the sh_size member of the intial
entry in section header table. Otherwise, the sh_size member of the initial
entry in the section header table holds the value zero."

Since this check wasn't being made, grub_elfXX_get_shnum() is being added to
elfXX.c to make this check and use whichever member doesn't have a value of
zero. If both are zero, then we must return an error. We also need to make sure
that e_shnum doesn't have a value greater than or equal to SHN_LORESERVE and
sh_size isn't less than SHN_LORESERVE.

Note that even though elf.c and elfXX.c are located in grub-core/kern, they are
compiled as modules and don't need the EXPORT_FUNC macro to define the functions
in elf.h.

Signed-off-by: Alec Brown 
---
 grub-core/kern/elf.c   |  9 
 grub-core/kern/elfXX.c | 34 +
 grub-core/loader/i386/bsdXX.c  | 82 ++
 grub-core/loader/multiboot_elfxx.c | 49 +++---
 include/grub/elf.h |  9 
 5 files changed, 142 insertions(+), 41 deletions(-)

diff --git a/grub-core/kern/elf.c b/grub-core/kern/elf.c
index 9d7149b38..5be770583 100644
--- a/grub-core/kern/elf.c
+++ b/grub-core/kern/elf.c
@@ -167,11 +167,14 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #define grub_elfXX_load_phdrs grub_elf32_load_phdrs
 #define ElfXX_Phdr Elf32_Phdr
 #define ElfXX_Ehdr Elf32_Ehdr
+#define ElfXX_Shdr Elf32_Shdr
+#define ElfXX_Word Elf32_Word
 #define grub_uintXX_t grub_uint32_t
 #define grub_swap_bytes_addrXX grub_swap_bytes32
 #define grub_swap_bytes_offXX grub_swap_bytes32
 #define grub_swap_bytes_XwordXX grub_swap_bytes32
 #define grub_elfXX_check_endianess_and_bswap_ehdr 
grub_elf32_check_endianess_and_bswap_ehdr
+#define grub_elfXX_get_shnum grub_elf32_get_shnum
 
 #include "elfXX.c"
 
@@ -185,11 +188,14 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #undef grub_elfXX_load_phdrs
 #undef ElfXX_Phdr
 #undef ElfXX_Ehdr
+#undef ElfXX_Shdr
+#undef ElfXX_Word
 #undef grub_uintXX_t
 #undef grub_swap_bytes_addrXX
 #undef grub_swap_bytes_offXX
 #undef grub_swap_bytes_XwordXX
 #undef grub_elfXX_check_endianess_and_bswap_ehdr
+#undef grub_elfXX_get_shnum
 
 
 /* 64-bit */
@@ -203,10 +209,13 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #define grub_elfXX_load_phdrs grub_elf64_load_phdrs
 #define ElfXX_Phdr Elf64_Phdr
 #define ElfXX_Ehdr Elf64_Ehdr
+#define ElfXX_Shdr Elf64_Shdr
+#define ElfXX_Word Elf64_Word
 #define grub_uintXX_t grub_uint64_t
 #define grub_swap_bytes_addrXX grub_swap_bytes64
 #define grub_swap_bytes_offXX grub_swap_bytes64
 #define grub_swap_bytes_XwordXX grub_swap_bytes64
 #define grub_elfXX_check_endianess_and_bswap_ehdr 
grub_elf64_check_endianess_and_bswap_ehdr
+#define grub_elfXX_get_shnum grub_elf64_get_shnum
 
 #include "elfXX.c"
diff --git a/grub-core/kern/elfXX.c b/grub-core/kern/elfXX.c
index 1859d1880..ed2dc7075 100644
--- a/grub-core/kern/elfXX.c
+++ b/grub-core/kern/elfXX.c
@@ -205,3 +205,37 @@ grub_elfXX_check_endianess_and_bswap_ehdr (grub_elf_t elf)
 
   return 0;
 }
+
+grub_err_t
+grub_elfXX_get_shnum (ElfXX_Ehdr *e, ElfXX_Word *shnum)
+{
+  ElfXX_Shdr *s;
+
+  if (shnum == NULL)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for 
shnum"));
+
+  /* Set *shnum to 0 so that shnum doesn't return junk on error */
+  *shnum = 0;
+
+  if (e == NULL)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for elf 
header"));
+
+  *shnum = e->e_shnum;
+  if (*shnum == SHN_UNDEF)
+{
+  if (e->e_shoff == 0)
+   return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid section header 
table offset in e_shoff"));
+
+  s = (ElfXX_Shdr *) ((grub_uint8_t *) e + e->e_shoff);
+  *shnum = s->sh_size;
+  if (*shnum < SHN_LORESERVE)
+   return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid number of section 
header table entries in sh_size: %d"), *shnum);
+}
+  else
+{
+  if (*shnum >= SHN_LORESERVE)
+   return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid number of section 
header table entries in e_shnum: %d"), *shnum);
+}
+
+  return GRUB_ERR_NONE;
+}
diff --git a/grub-core/loader/i386/bsdXX.c b/grub-core/loader/i386/bsdXX.c
index 6946cecbb..7d9b65d1a 100644
--- a/grub-core/loader/i386/bsdXX.c
+++ b/grub-core/loader/i386/bsdXX.c
@@ -26,7 +26,10 @@ load (grub_file_t file, const char *filename, void *where, 
grub_off_t off, grub_
 static inline grub_err_t
 read_headers (grub_file_t file, const char *filename, Elf_Ehdr *e, Elf_Shdr 
**shdr)
 {
- if (grub_file_seek (file, 0) == (grub_off_t) -1)
+  Elf_Word 

[PATCH v3 1/5] grub-core/loader/i386/bsdXX.c: Avoid downcasting (char *) to (Elf_Shdr *)

[Alec Brown]

In bsdXX.c, a couple of untrusted loop bound and untrusted allocation size bugs
were flagged by Coverity in the functions grub_openbsd_find_ramdisk() and
grub_freebsd_load_elfmodule(). These bugs were flagged by coverity because the
variable shdr was downcasting from a char pointer to an Elf_Shdr pointer
whenever it was used to set the base value in for loops. To avoid this, we need
to set shdr as an Elf_Shdr pointer where it is initialized.

In the function read_headers(), the function is reading elf section header data
from a file and passing it to the variable shdr as data for a char pointer. If
we switch the type of shdr to an Elf_Shdr pointer in read_headers() as well as
other functions, then we won't need to downcast to an Elf_Shdr pointer and won't
have to worry about tainted data later in the code.

Fixes: CID 314018
Fixes: CID 314030
Fixes: CID 314031
Fixes: CID 314039

Signed-off-by: Alec Brown 
---
 grub-core/loader/i386/bsdXX.c | 66 +++
 1 file changed, 28 insertions(+), 38 deletions(-)

diff --git a/grub-core/loader/i386/bsdXX.c b/grub-core/loader/i386/bsdXX.c
index e4f4aa365..6946cecbb 100644
--- a/grub-core/loader/i386/bsdXX.c
+++ b/grub-core/loader/i386/bsdXX.c
@@ -24,7 +24,7 @@ load (grub_file_t file, const char *filename, void *where, 
grub_off_t off, grub_
 }
 
 static inline grub_err_t
-read_headers (grub_file_t file, const char *filename, Elf_Ehdr *e, char **shdr)
+read_headers (grub_file_t file, const char *filename, Elf_Ehdr *e, Elf_Shdr 
**shdr)
 {
  if (grub_file_seek (file, 0) == (grub_off_t) -1)
 return grub_errno;
@@ -78,7 +78,7 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct 
grub_relocator *relocator,
 {
   Elf_Ehdr e;
   Elf_Shdr *s;
-  char *shdr = 0;
+  Elf_Shdr *shdr = NULL;
   grub_addr_t curload, module;
   grub_err_t err;
   grub_size_t chunk_size = 0;
@@ -90,9 +90,8 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct 
grub_relocator *relocator,
   if (err)
 goto out;
 
-  for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) ((char *) shdr
-   + e.e_shnum * e.e_shentsize);
-   s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+  for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * 
e.e_shentsize);
+   s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
 {
   if (s->sh_size == 0)
continue;
@@ -113,9 +112,8 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct 
grub_relocator *relocator,
 chunk_src = get_virtual_current_address (ch);
   }
 
-  for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) ((char *) shdr
-   + e.e_shnum * e.e_shentsize);
-   s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+  for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * 
e.e_shentsize);
+   s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
 {
   if (s->sh_size == 0)
continue;
@@ -173,7 +171,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator 
*relocator,
 {
   Elf_Ehdr e;
   Elf_Shdr *s;
-  char *shdr = 0;
+  Elf_Shdr *shdr = NULL;
   grub_addr_t curload, module;
   grub_err_t err;
   grub_size_t chunk_size = 0;
@@ -185,9 +183,8 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator 
*relocator,
   if (err)
 goto out;
 
-  for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) ((char *) shdr
-   + e.e_shnum * e.e_shentsize);
-   s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+  for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * 
e.e_shentsize);
+   s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
 {
   if (s->sh_size == 0)
continue;
@@ -214,9 +211,8 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator 
*relocator,
 chunk_src = get_virtual_current_address (ch);
   }
 
-  for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) ((char *) shdr
-   + e.e_shnum * e.e_shentsize);
-   s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+  for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * 
e.e_shentsize);
+   s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
 {
   if (s->sh_size == 0)
continue;
@@ -285,7 +281,7 @@ SUFFIX (grub_freebsd_load_elf_meta) (struct grub_relocator 
*relocator,
   grub_err_t err;
   Elf_Ehdr e;
   Elf_Shdr *s;
-  char *shdr = 0;
+  Elf_Shdr *shdr = NULL;
   unsigned symoff, stroff, symsize, strsize;
   grub_freebsd_addr_t symstart, symend, symentsize, dynamic;
   Elf_Sym *sym;
@@ -306,13 +302,11 @@ SUFFIX (grub_freebsd_load_elf_meta) (struct 
grub_relocator *relocator,
   if (err)
 goto out;
 
-  for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) (shdr
-   + e.e_shnum * e.e_shentsize);
-   s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+  for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * 
e.e_shentsize);
+   s = (Elf_S

[PATCH v3 5/5] util/grub-module-verifierXX.c: Add e_shoff check in get_shdr()

[Alec Brown]

In util/grub-module-verifierXX.c, the function get_shdr() is used to obtain the
section header at a given index but isn't checking that there is an offset for
the section header table. To validate that there is, we can check that e_shoff
isn't 0.

Signed-off-by: Alec Brown 
---
 util/grub-module-verifierXX.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/util/grub-module-verifierXX.c b/util/grub-module-verifierXX.c
index 4e6cf133f..cf3ff0dfa 100644
--- a/util/grub-module-verifierXX.c
+++ b/util/grub-module-verifierXX.c
@@ -134,6 +134,9 @@ grub_target_to_host_real (const struct 
grub_module_verifier_arch *arch, grub_uin
 static Elf_Shdr *
 get_shdr (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e, Elf_Word 
index)
 {
+  if (grub_target_to_host (e->e_shoff) == 0)
+grub_util_error ("Invalid section header offset");
+
   return (Elf_Shdr *) ((char *) e + grub_target_to_host (e->e_shoff) +
   index * grub_target_to_host16 (e->e_shentsize));
 }
-- 
2.27.0


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

[PATCH v3 3/5] elf: Validate elf section header table index for section name string table

[Alec Brown]

In multiboot_elfXX.c, e_shstrndx is used to obtain the section header table
index of the section name string table, but it wasn't being checked if the value
was there.

According to the elf(5) manual page,
"If the index of section name string table section is larger than or equal to
SHN_LORESERVE (0xff00), this member holds SHN_XINDEX (0x) and the real
index of the section name string table section is held in the sh_link member of
the initial entry in section header table. Otherwise, the sh_link member of the
initial entry in section header table contains the value zero."

Since this check wasn't being made, grub_elfXX_get_shstrndx() is being added to
elfXX.c to make this check and use e_shstrndx if it doesn't have SHN_XINDEX as a
value, else use sh_link. We also need to make sure e_shstrndx isn't greater than
or equal to SHN_LORESERVE and sh_link isn't less than SHN_LORESERVE.

Note that even though elf.c and elfXX.c are located in grub-core/kern, they are
compiled as modules and don't need the EXPORT_FUNC macro to define the functions
in elf.h.

Signed-off-by: Alec Brown 
---
 grub-core/kern/elf.c   |  3 +++
 grub-core/kern/elfXX.c | 33 ++
 grub-core/loader/multiboot_elfxx.c | 10 -
 include/grub/elf.h |  4 
 4 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/grub-core/kern/elf.c b/grub-core/kern/elf.c
index 5be770583..19440a318 100644
--- a/grub-core/kern/elf.c
+++ b/grub-core/kern/elf.c
@@ -175,6 +175,7 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #define grub_swap_bytes_XwordXX grub_swap_bytes32
 #define grub_elfXX_check_endianess_and_bswap_ehdr 
grub_elf32_check_endianess_and_bswap_ehdr
 #define grub_elfXX_get_shnum grub_elf32_get_shnum
+#define grub_elfXX_get_shstrndx grub_elf32_get_shstrndx
 
 #include "elfXX.c"
 
@@ -196,6 +197,7 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #undef grub_swap_bytes_XwordXX
 #undef grub_elfXX_check_endianess_and_bswap_ehdr
 #undef grub_elfXX_get_shnum
+#undef grub_elfXX_get_shstrndx
 
 
 /* 64-bit */
@@ -217,5 +219,6 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #define grub_swap_bytes_XwordXX grub_swap_bytes64
 #define grub_elfXX_check_endianess_and_bswap_ehdr 
grub_elf64_check_endianess_and_bswap_ehdr
 #define grub_elfXX_get_shnum grub_elf64_get_shnum
+#define grub_elfXX_get_shstrndx grub_elf64_get_shstrndx
 
 #include "elfXX.c"
diff --git a/grub-core/kern/elfXX.c b/grub-core/kern/elfXX.c
index ed2dc7075..d09b37213 100644
--- a/grub-core/kern/elfXX.c
+++ b/grub-core/kern/elfXX.c
@@ -239,3 +239,36 @@ grub_elfXX_get_shnum (ElfXX_Ehdr *e, ElfXX_Word *shnum)
 
   return GRUB_ERR_NONE;
 }
+
+grub_err_t
+grub_elfXX_get_shstrndx (ElfXX_Ehdr *e, grub_uint32_t *shstrndx)
+{
+  ElfXX_Shdr *s;
+
+  if (shstrndx == NULL)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for 
shstrndx"));
+
+  /* Set *shstrndx to 0 so that shstrndx doesn't return junk on error */
+  *shstrndx = 0;
+
+  if (e == NULL)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for elf 
header"));
+
+  *shstrndx = e->e_shstrndx;
+  if (*shstrndx == SHN_XINDEX)
+{
+  if (e->e_shoff == 0)
+   return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid section header 
table offset in e_shoff"));
+
+  s = (ElfXX_Shdr *) ((grub_uint8_t *) e + e->e_shoff);
+  *shstrndx = s->sh_link;
+  if (*shstrndx < SHN_LORESERVE)
+   return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid section header 
table index in sh_link: %d"), *shstrndx);
+}
+  else
+{
+  if (*shstrndx >= SHN_LORESERVE)
+   return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid section header 
table index in e_shstrndx: %d"), *shstrndx);
+}
+  return GRUB_ERR_NONE;
+}
diff --git a/grub-core/loader/multiboot_elfxx.c 
b/grub-core/loader/multiboot_elfxx.c
index 1fe9be91b..54aaa24aa 100644
--- a/grub-core/loader/multiboot_elfxx.c
+++ b/grub-core/loader/multiboot_elfxx.c
@@ -25,6 +25,7 @@
 # define Elf_Shdr  Elf32_Shdr
 # define Elf_Word  Elf32_Word
 # define grub_elf_get_shnumgrub_elf32_get_shnum
+# define grub_elf_get_shstrndx grub_elf32_get_shstrndx
 #elif defined(MULTIBOOT_LOAD_ELF64)
 # define XX64
 # define E_MACHINE MULTIBOOT_ELF64_MACHINE
@@ -34,6 +35,7 @@
 # define Elf_Shdr  Elf64_Shdr
 # define Elf_Word  Elf64_Word
 # define grub_elf_get_shnumgrub_elf64_get_shnum
+# define grub_elf_get_shstrndx grub_elf64_get_shstrndx
 #else
 #error "I'm confused"
 #endif
@@ -62,6 +64,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
   grub_err_t err;
   grub_relocator_chunk_t ch;
   grub_uint32_t load_offset = 0, load_size;
+  grub_uint32_t shstrndx;
   Elf_Word shnum;
   unsigned int i;
   void *source = NULL;
@@ -84,6 +87,10 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
   if (err != GRUB_ERR_NONE)
 return err;
 
+  err 

[PATCH v3 00/15] Dynamic allocation of memory regions and IBM vTPM v2

[Daniel Axtens]

Hi,

This is a small update to v2
(https://lists.gnu.org/archive/html/grub-devel/2022-03/msg00242.html)

Changes since v2:

 - A fix to patch 11 that does IBM CAS, as it turns out my old approach
   caused some more extreme configurations to fail to boot. Oops.
 - Code style fixes (tabs vs spaces).
 - Add Daniel's R-Bs.

This series is also available on github.com/daxtens/grub branch
memrework+vtpm-202204

The v2 cover letter:

This is, at long last, an updated version of my series extending Patrick's
dynamic memory regions to ieee1275.

Noteworthy changes:

 - reworked debug prints as grub_dprintfs. Folded the ieee1275 ones into the
   ieee1275 patches.

 - reworked the ieee1275 runtime memory claiming to be more resilient and better
   documented.

 - fixed comment style and hopefully addressed all other change requests.

 - grub will now try asking for contiguous memory and then, if that fails, for
   discontiguous memory - in case region merging with the discontiguous memory
   is sufficient to allow the eventual allocation to succeed.

 - The ieee1275 code agressively rounds up the size of the region for a dynamic
   allocation - it will now retry with a more precise size if the larger
   allocation fails.

The memtool module is included as an RFC only and will require more work, as
discussed in the patch.

I've also included Stefan's vTPM patch - Stefan kindly tested that his patch
worked with my memory rework series. I have added his tested-by to relevant
Power-specific patches only.

Kind regards,
Daniel

Daniel Axtens (8):
  grub-shell: only pass SeaBIOS fw_opt in for x86 BIOS platforms
  mm: assert that we preserve header vs region alignment
  mm: when adding a region, merge with region after as well as before
  mm: debug support for region operations
  ieee1275: request memory with ibm,client-architecture-support
  ieee1275: drop len -= 1 quirk in heap_init
  ieee1275: support runtime memory claiming
  [RFC] Add memtool module with memory allocation stress-test

Patrick Steinhardt (6):
  mm: Drop unused unloading of modules on OOM
  mm: Allow dynamically requesting additional memory regions
  efi: mm: Always request a fixed number of pages on init
  efi: mm: Extract function to add memory regions
  efi: mm: Pass up errors from `add_memory_regions ()`
  efi: mm: Implement runtime addition of pages

Stefan Berger (1):
  ibmvtpm: Add support for trusted boot using a vTPM 2.0

 docs/grub-dev.texi|   7 +-
 docs/grub.texi|   3 +-
 grub-core/Makefile.core.def   |  12 +
 grub-core/commands/ieee1275/ibmvtpm.c | 152 ++
 grub-core/commands/memtools.c | 155 ++
 grub-core/kern/dl.c   |  20 --
 grub-core/kern/efi/mm.c   |  83 +++---
 grub-core/kern/ieee1275/cmain.c   |   3 +
 grub-core/kern/ieee1275/init.c| 410 +-
 grub-core/kern/mm.c   | 178 +++
 include/grub/dl.h |   1 -
 include/grub/ieee1275/ieee1275.h  |  11 +
 include/grub/mm.h |  16 +
 include/grub/mm_private.h |  23 ++
 tests/util/grub-shell.in  |   6 +-
 15 files changed, 942 insertions(+), 138 deletions(-)
 create mode 100644 grub-core/commands/ieee1275/ibmvtpm.c
 create mode 100644 grub-core/commands/memtools.c

-- 
2.32.0


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

[PATCH v3 01/15] grub-shell: only pass SeaBIOS fw_opt in for x86 BIOS platforms

[Daniel Axtens]

This breaks the tests on pseries - just restrict it to x86 platforms
that don't specify a BIOS.

Signed-off-by: Daniel Axtens 
Reviewed-by: Daniel Kiper 
---
v2: Thanks Daniel K and Glenn for feedback.
---
 tests/util/grub-shell.in | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index 33590baeb13c..4828afb7cc5a 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -376,7 +376,11 @@ if test -z "$debug"; then
   # workaround unfortunately causes qemu to issue a warning 'externally
   # provided fw_cfg item names should be prefixed with "opt/"', but there
   # doesn't seem to be a better option.
-  qemuopts="${qemuopts} -fw_cfg name=etc/sercon-port,string=0"
+  #
+  # SeaBIOS is used for i386, except on EFI.
+  if [ ${grub_modinfo_target_cpu} == 'i386' ] && [ ${grub_modinfo_platform} != 
'efi' ]; then
+qemuopts="${qemuopts} -fw_cfg name=etc/sercon-port,string=0"
+  fi
 fi
 
 if [ x$boot != xnet ] && [ x$boot != xemu ]; then
-- 
2.32.0


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

[PATCH v3 06/15] mm: Allow dynamically requesting additional memory regions

[Daniel Axtens]

From: Patrick Steinhardt 

Currently, all platforms will set up their heap on initialization of the
platform code. While this works mostly fine, it poses some limitations
on memory management on us. Most notably, allocating big chunks of
memory in the gigabyte range would require us to pre-request this many
bytes from the firmware and add it to the heap from the beginning on
some platforms like EFI. As this isn't needed for most configurations,
it is inefficient and may even negatively impact some usecases when,
e.g., chainloading. Nonetheless, allocating big chunks of memory is
required sometimes, where one example is the upcoming support for the
Argon2 key derival function in LUKS2.

In order to avoid pre-allocating big chunks of memory, this commit
implements a runtime mechanism to add more pages to the system. When a
given allocation cannot be currently satisfied, we'll call a given
callback set up by the platform's own memory management subsystem,
asking it to add a memory area with at least `n` bytes. If this
succeeds, we retry searching for a valid memory region, which should now
succeed.

If this fails, we try asking for `n` bytes, possibly spread across
multiple regions, in hopes that region merging means that we end up
with enough memory for things to work out.

Signed-off-by: Patrick Steinhardt 
[dja: add this to the documentation at the top of mm.c
  v2: fallback to non-contiguous]
Signed-off-by: Daniel Axtens 
Tested-by: Stefan Berger 
Reviewed-by: Daniel Kiper 
---
 grub-core/kern/mm.c | 30 ++
 include/grub/mm.h   | 16 
 2 files changed, 46 insertions(+)

diff --git a/grub-core/kern/mm.c b/grub-core/kern/mm.c
index 41e5ea07dc5a..ed782e8d1f56 100644
--- a/grub-core/kern/mm.c
+++ b/grub-core/kern/mm.c
@@ -28,6 +28,9 @@
   - multiple regions may be used as free space. They may not be
   contiguous.
 
+  - if existing regions are insufficient to satisfy an allocation, a new
+  region can be requested from firmware.
+
   Regions are managed by a singly linked list, and the meta information is
   stored in the beginning of each region. Space after the meta information
   is used to allocate memory.
@@ -81,6 +84,7 @@
 
 
 grub_mm_region_t grub_mm_base;
+grub_mm_add_region_func_t grub_mm_add_region_fn;
 
 /* Get a header from the pointer PTR, and set *P and *R to a pointer
to the header and a pointer to its region, respectively. PTR must
@@ -437,6 +441,32 @@ grub_memalign (grub_size_t align, grub_size_t size)
   count++;
   goto again;
 
+case 1:
+  /* Request additional pages, contiguous */
+  count++;
+
+  if (grub_mm_add_region_fn != NULL &&
+  grub_mm_add_region_fn (size, GRUB_MM_ADD_REGION_CONSECUTIVE) == 
GRUB_ERR_NONE)
+   goto again;
+
+  /* fallthrough  */
+
+case 2:
+  /* Request additional pages, anything at all */
+  count++;
+
+  if (grub_mm_add_region_fn != NULL)
+{
+  /*
+   * Try again even if this fails, in case it was able to partially
+   * satisfy the request
+   */
+  grub_mm_add_region_fn (size, GRUB_MM_ADD_REGION_NONE);
+  goto again;
+}
+
+  /* fallthrough */
+
 default:
   break;
 }
diff --git a/include/grub/mm.h b/include/grub/mm.h
index 44fde7cb9033..5d916809666c 100644
--- a/include/grub/mm.h
+++ b/include/grub/mm.h
@@ -20,6 +20,7 @@
 #ifndef GRUB_MM_H
 #define GRUB_MM_H  1
 
+#include 
 #include 
 #include 
 #include 
@@ -28,6 +29,21 @@
 # define NULL  ((void *) 0)
 #endif
 
+#define GRUB_MM_ADD_REGION_NONE0
+#define GRUB_MM_ADD_REGION_CONSECUTIVE (1 << 0)
+
+/*
+ * Function used to request memory regions of `grub_size_t` bytes. The second
+ * parameter is a bitfield of `GRUB_MM_ADD_REGION` flags.
+ */
+typedef grub_err_t (*grub_mm_add_region_func_t) (grub_size_t, unsigned int);
+
+/*
+ * Set this function pointer to enable adding memory-regions at runtime in case
+ * a memory allocation cannot be satisfied with existing regions.
+ */
+extern grub_mm_add_region_func_t EXPORT_VAR(grub_mm_add_region_fn);
+
 void grub_mm_init_region (void *addr, grub_size_t size);
 void *EXPORT_FUNC(grub_calloc) (grub_size_t nmemb, grub_size_t size);
 void *EXPORT_FUNC(grub_malloc) (grub_size_t size);
-- 
2.32.0


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

[PATCH v3 03/15] mm: when adding a region, merge with region after as well as before

[Daniel Axtens]

On x86_64-efi (at least) regions seem to be added from top down. The mm
code will merge a new region with an existing region that comes
immediately before the new region. This allows larger allocations to be
satisfied that would otherwise be the case.

On powerpc-ieee1275, however, regions are added from bottom up. So if
we add 3x 32MB regions, we can still only satisfy a 32MB allocation,
rather than the 96MB allocation we might otherwise be able to satisfy.

 * Define 'post_size' as being bytes lost to the end of an allocation
   due to being given weird sizes from firmware that are not multiples
   of GRUB_MM_ALIGN.

 * Allow merging of regions immediately _after_ existing regions, not
   just before. As with the other approach, we create an allocated
   block to represent the new space and the pass it to grub_free() to
   get the metadata right.

Signed-off-by: Daniel Axtens 
Tested-by: Stefan Berger 
Reviewed-by: Daniel Kiper 
---
v2: Thanks Daniel K for feedback.
v3: Fix alignment of comments (8 spaces vs tab)
---
 grub-core/kern/mm.c   | 123 +++---
 include/grub/mm_private.h |   9 +++
 2 files changed, 85 insertions(+), 47 deletions(-)

diff --git a/grub-core/kern/mm.c b/grub-core/kern/mm.c
index 079c28da7cdf..6e4e8f325a05 100644
--- a/grub-core/kern/mm.c
+++ b/grub-core/kern/mm.c
@@ -130,53 +130,81 @@ grub_mm_init_region (void *addr, grub_size_t size)
 
   /* Attempt to merge this region with every existing region */
   for (p = &grub_mm_base, q = *p; q; p = &(q->next), q = *p)
-/*
- * Is the new region immediately below an existing region? That
- * is, is the address of the memory we're adding now (addr) + size
- * of the memory we're adding (size) + the bytes we couldn't use
- * at the start of the region we're considering (q->pre_size)
- * equal to the address of q? In other words, does the memory
- * looks like this?
- *
- * addr  q
- *   |size-|-q->pre_size-||
- */
-if ((grub_uint8_t *) addr + size + q->pre_size == (grub_uint8_t *) q)
-  {
-   /*
-* Yes, we can merge the memory starting at addr into the
-* existing region from below. Align up addr to GRUB_MM_ALIGN
-* so that our new region has proper alignment.
-*/
-   r = (grub_mm_region_t) ALIGN_UP ((grub_addr_t) addr, GRUB_MM_ALIGN);
-   /* Copy the region data across */
-   *r = *q;
-   /* Consider all the new size as pre-size */
-   r->pre_size += size;
-
-   /*
-* If we have enough pre-size to create a block, create a
-* block with it. Mark it as allocated and pass it to
-* grub_free (), which will sort out getting it into the free
-* list.
-*/
-   if (r->pre_size >> GRUB_MM_ALIGN_LOG2)
- {
-   h = (grub_mm_header_t) (r + 1);
-   /* block size is pre-size converted to cells */
-   h->size = (r->pre_size >> GRUB_MM_ALIGN_LOG2);
-   h->magic = GRUB_MM_ALLOC_MAGIC;
-   /* region size grows by block size converted back to bytes */
-   r->size += h->size << GRUB_MM_ALIGN_LOG2;
-   /* adjust pre_size to be accurate */
-   r->pre_size &= (GRUB_MM_ALIGN - 1);
-   *p = r;
-   grub_free (h + 1);
- }
-   /* Replace the old region with the new region */
-   *p = r;
-   return;
-  }
+{
+  /*
+   * Is the new region immediately below an existing region? That
+   * is, is the address of the memory we're adding now (addr) + size
+   * of the memory we're adding (size) + the bytes we couldn't use
+   * at the start of the region we're considering (q->pre_size)
+   * equal to the address of q? In other words, does the memory
+   * looks like this?
+   *
+   * addr  q
+   *   |size-|-q->pre_size-||
+   */
+  if ((grub_uint8_t *) addr + size + q->pre_size == (grub_uint8_t *) q)
+{
+  /*
+   * Yes, we can merge the memory starting at addr into the
+   * existing region from below. Align up addr to GRUB_MM_ALIGN
+   * so that our new region has proper alignment.
+   */
+  r = (grub_mm_region_t) ALIGN_UP ((grub_addr_t) addr, GRUB_MM_ALIGN);
+  /* Copy the region data across */
+  *r = *q;
+  /* Consider all the new size as pre-size */
+  r->pre_size += size;
+
+  /*
+   * If we have enough pre-size to create a block, create a
+   * block with it. Mark it as allocated and pass it to
+   * grub_free (), which will sort out getting it into the free
+   * list.
+   */
+  if (r->pre_size >> GRUB_MM_ALIGN_LOG2)
+{
+  h = (grub_mm_header_t) (r + 1);
+  /* block size is pre-size converted to cells */
+  h->size = (r->pre_size >> GRUB_MM_ALIGN_LOG2);
+  h->m

[PATCH v3 02/15] mm: assert that we preserve header vs region alignment

[Daniel Axtens]

grub_mm_region_init() does:

  h = (grub_mm_header_t) (r + 1);

where h is a grub_mm_header_t and r is a grub_mm_region_t.

Cells are supposed to be GRUB_MM_ALIGN aligned, but while grub_mm_dump
ensures this vs the region header, grub_mm_region_init() does not.

It's better to be explicit than implicit here: rather than changing
grub_mm_region_init() to ALIGN_UP(), require that the struct is
explictly a multiple of the header size.

Signed-off-by: Daniel Axtens 
Reviewed-by: Daniel Kiper 

---
v2: Thanks Daniel K for feedback.
---
 include/grub/mm_private.h | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/include/grub/mm_private.h b/include/grub/mm_private.h
index 203533cc3d42..d3f2321e14fb 100644
--- a/include/grub/mm_private.h
+++ b/include/grub/mm_private.h
@@ -20,6 +20,7 @@
 #define GRUB_MM_PRIVATE_H  1
 
 #include 
+#include 
 
 /* For context, see kern/mm.c */
 
@@ -89,4 +90,17 @@ typedef struct grub_mm_region
 extern grub_mm_region_t EXPORT_VAR (grub_mm_base);
 #endif
 
+static inline void
+grub_mm_size_sanity_check (void) {
+  /* Ensure we preserve alignment when doing h = (grub_mm_header_t) (r + 1) */
+  COMPILE_TIME_ASSERT ((sizeof (struct grub_mm_region) %
+   sizeof (struct grub_mm_header)) == 0);
+
+  /*
+   * GRUB_MM_ALIGN is supposed to represent cell size, and a mm_header is
+   * supposed to be 1 cell.
+   */
+  COMPILE_TIME_ASSERT (sizeof (struct grub_mm_header) == GRUB_MM_ALIGN);
+}
+
 #endif
-- 
2.32.0


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

[PATCH v3 15/15] ibmvtpm: Add support for trusted boot using a vTPM 2.0

[Daniel Axtens]

From: Stefan Berger 

Add support for trusted boot using a vTPM 2.0 on the IBM IEEE1275
PowerPC platform. With this patch grub now measures text and binary data
into the TPM's PCRs 8 and 9 in the same way as the x86_64 platform
does.

This patch requires Daniel Axtens's patches for claiming more memory.

For vTPM support to work on PowerVM, system driver levels 1010.30
or 1020.00 are required.

Note: Previous versions of firmware levels with the 2hash-ext-log
API call have a bug that, once this API call is invoked, has the
effect of disabling the vTPM driver under Linux causing an error
message to be displayed in the Linux kernel log. Those users will
have to update their machines to the firmware levels mentioned
above.

Cc: Eric Snowberg 
Signed-off-by: Stefan Berger 
Signed-off-by: Daniel Axtens 
---
 docs/grub.texi|   3 +-
 grub-core/Makefile.core.def   |   7 ++
 grub-core/commands/ieee1275/ibmvtpm.c | 152 ++
 include/grub/ieee1275/ieee1275.h  |   3 +
 4 files changed, 164 insertions(+), 1 deletion(-)
 create mode 100644 grub-core/commands/ieee1275/ibmvtpm.c

diff --git a/docs/grub.texi b/docs/grub.texi
index 9835c878affc..fd3790929af4 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -6087,7 +6087,8 @@ tpm module is loaded. As such it is recommended that the 
tpm module be built
 into @file{core.img} in order to avoid a potential gap in measurement between
 @file{core.img} being loaded and the tpm module being loaded.
 
-Measured boot is currently only supported on EFI platforms.
+Measured boot is currently only supported on EFI and IBM IEEE1275 PowerPC
+platforms.
 
 @node Lockdown
 @section Lockdown when booting on a secure setup
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 795369155a43..b5bb10a5c302 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -1123,6 +1123,13 @@ module = {
   enable = powerpc_ieee1275;
 };
 
+module = {
+  name = tpm;
+  common = commands/tpm.c;
+  ieee1275 = commands/ieee1275/ibmvtpm.c;
+  enable = powerpc_ieee1275;
+};
+
 module = {
   name = terminal;
   common = commands/terminal.c;
diff --git a/grub-core/commands/ieee1275/ibmvtpm.c 
b/grub-core/commands/ieee1275/ibmvtpm.c
new file mode 100644
index ..e68b8448bc00
--- /dev/null
+++ b/grub-core/commands/ieee1275/ibmvtpm.c
@@ -0,0 +1,152 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2021  Free Software Foundation, Inc.
+ *  Copyright (C) 2021  IBM Corporation
+ *
+ *  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 .
+ *
+ *  IBM vTPM support code.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static grub_ieee1275_ihandle_t tpm_ihandle;
+static grub_uint8_t tpm_version;
+
+#define IEEE1275_IHANDLE_INVALID ((grub_ieee1275_ihandle_t)0)
+
+static void
+tpm_get_tpm_version (void)
+{
+  grub_ieee1275_phandle_t vtpm;
+  char buffer[20];
+
+  if (!grub_ieee1275_finddevice ("/vdevice/vtpm", &vtpm) &&
+  !grub_ieee1275_get_property (vtpm, "compatible", buffer,
+  sizeof (buffer), NULL) &&
+  !grub_strcmp (buffer, "IBM,vtpm20"))
+tpm_version = 2;
+}
+
+static grub_err_t
+tpm_init (void)
+{
+  static int init_success = 0;
+
+  if (!init_success)
+{
+  if (grub_ieee1275_open ("/vdevice/vtpm", &tpm_ihandle) < 0) {
+tpm_ihandle = IEEE1275_IHANDLE_INVALID;
+return GRUB_ERR_UNKNOWN_DEVICE;
+  }
+
+  init_success = 1;
+
+  tpm_get_tpm_version ();
+}
+
+  return GRUB_ERR_NONE;
+}
+
+static int
+ibmvtpm_2hash_ext_log (grub_uint8_t pcrindex,
+  grub_uint32_t eventtype,
+  const char *description,
+  grub_size_t description_size,
+  void *buf, grub_size_t size)
+{
+  struct tpm_2hash_ext_log
+  {
+struct grub_ieee1275_common_hdr common;
+grub_ieee1275_cell_t method;
+grub_ieee1275_cell_t ihandle;
+grub_ieee1275_cell_t size;
+grub_ieee1275_cell_t buf;
+grub_ieee1275_cell_t description_size;
+grub_ieee1275_cell_t description;
+grub_ieee1275_cell_t eventtype;
+grub_ieee1275_cell_t pcrindex;
+grub_ieee1275_cell_t catch_result;
+grub_ieee1275_cell_t rc;
+  }
+  args;
+
+  INIT_IEEE1275_COMMON (&args.common, "call-method", 8, 2);
+  args.method = (grub_ieee1275_cell_t) "2hash-ext-log";
+  

[PATCH v3 04/15] mm: debug support for region operations

[Daniel Axtens]

This is handy for debugging. Enable with `set debug=regions`.

Signed-off-by: Daniel Axtens 
Reviewed-by: Daniel Kiper 
---
 grub-core/kern/mm.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/grub-core/kern/mm.c b/grub-core/kern/mm.c
index 6e4e8f325a05..a1f47c0616a2 100644
--- a/grub-core/kern/mm.c
+++ b/grub-core/kern/mm.c
@@ -115,9 +115,8 @@ grub_mm_init_region (void *addr, grub_size_t size)
   grub_mm_header_t h;
   grub_mm_region_t r, *p, q;
 
-#if 0
-  grub_printf ("Using memory for heap: start=%p, end=%p\n", addr, addr + 
(unsigned int) size);
-#endif
+  grub_dprintf ("regions", "Using memory for heap: start=%p, end=%p\n",
+addr, (char *) addr + (unsigned int) size);
 
   /* Exclude last 4K to avoid overflows. */
   /* If addr + 0x1000 overflows then whole region is in excluded zone.  */
@@ -142,8 +141,14 @@ grub_mm_init_region (void *addr, grub_size_t size)
* addr  q
*   |size-|-q->pre_size-||
*/
+  grub_dprintf ("regions", "Can we extend into region above?"
+   " %p + %" PRIxGRUB_SIZE " + %" PRIxGRUB_SIZE " ?=? %p\n",
+   (grub_uint8_t *) addr, size, q->pre_size, (grub_uint8_t *) 
q);
   if ((grub_uint8_t *) addr + size + q->pre_size == (grub_uint8_t *) q)
 {
+ grub_dprintf ("regions", "Yes: extending a region: (%p -> %p) -> (%p 
-> %p)\n",
+   q, (grub_uint8_t *) q + sizeof (*q) + q->size,
+   addr, (grub_uint8_t *) q + sizeof (*q) + q->size);
   /*
* Yes, we can merge the memory starting at addr into the
* existing region from below. Align up addr to GRUB_MM_ALIGN
@@ -185,9 +190,15 @@ grub_mm_init_region (void *addr, grub_size_t size)
*   q   addr
*   ||-q->post_size-|size-|
*/
+  grub_dprintf ("regions", "Can we extend into region below?"
+" %p + %" PRIxGRUB_SIZE " + %" PRIxGRUB_SIZE " + %" 
PRIxGRUB_SIZE " ?=? %p\n",
+(grub_uint8_t *) q, sizeof(*q), q->size, q->post_size, 
(grub_uint8_t *) addr);
   if ((grub_uint8_t *)q + sizeof(*q) + q->size + q->post_size ==
  (grub_uint8_t *) addr)
{
+ grub_dprintf ("regions", "Yes: extending a region: (%p -> %p) -> (%p 
-> %p)\n",
+   q, (grub_uint8_t *) q + sizeof (*q) + q->size,
+   q, (grub_uint8_t *) addr + size);
  /*
   * Yes! Follow a similar pattern to above, but simpler.
   * Our header starts at address - post_size, which should align us
@@ -206,6 +217,8 @@ grub_mm_init_region (void *addr, grub_size_t size)
}
 }
 
+  grub_dprintf ("regions", "No: considering a new region at %p of size %" 
PRIxGRUB_SIZE "\n",
+   addr, size);
   /* Allocate a region from the head.  */
   r = (grub_mm_region_t) ALIGN_UP ((grub_addr_t) addr, GRUB_MM_ALIGN);
 
-- 
2.32.0


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

[PATCH v3 08/15] efi: mm: Extract function to add memory regions

[Daniel Axtens]

From: Patrick Steinhardt 

In preparation of support for runtime-allocating additional memory
region, this patch extracts the function to retrieve the EFI memory map
and add a subset of it to GRUB's own memory regions.

Signed-off-by: Patrick Steinhardt 
Signed-off-by: Daniel Axtens 
Reviewed-by: Daniel Kiper 
---
 grub-core/kern/efi/mm.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index 0bccd24f304f..c8d3968f7f68 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -556,8 +556,8 @@ print_memory_map (grub_efi_memory_descriptor_t *memory_map,
 }
 #endif
 
-void
-grub_efi_mm_init (void)
+static grub_err_t
+grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
 {
   grub_efi_memory_descriptor_t *memory_map;
   grub_efi_memory_descriptor_t *memory_map_end;
@@ -570,7 +570,7 @@ grub_efi_mm_init (void)
   /* Prepare a memory region to store two memory maps.  */
   memory_map = grub_efi_allocate_any_pages (2 * BYTES_TO_PAGES 
(MEMORY_MAP_SIZE));
   if (! memory_map)
-grub_fatal ("cannot allocate memory");
+return grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory for 
memory map");
 
   /* Obtain descriptors for available memory.  */
   map_size = MEMORY_MAP_SIZE;
@@ -588,14 +588,14 @@ grub_efi_mm_init (void)
 
   memory_map = grub_efi_allocate_any_pages (2 * BYTES_TO_PAGES (map_size));
   if (! memory_map)
-   grub_fatal ("cannot allocate memory");
+   return grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory for 
new memory map");
 
   mm_status = grub_efi_get_memory_map (&map_size, memory_map, 0,
   &desc_size, 0);
 }
 
   if (mm_status < 0)
-grub_fatal ("cannot get memory map");
+return grub_error (GRUB_ERR_OUT_OF_MEMORY, "error fetching memory map from 
EFI");
 
   memory_map_end = NEXT_MEMORY_DESCRIPTOR (memory_map, map_size);
 
@@ -610,7 +610,7 @@ grub_efi_mm_init (void)
 
   /* Allocate memory regions for GRUB's memory management.  */
   add_memory_regions (filtered_memory_map, desc_size,
- filtered_memory_map_end, BYTES_TO_PAGES 
(DEFAULT_HEAP_SIZE));
+ filtered_memory_map_end, BYTES_TO_PAGES (required_bytes));
 
 #if 0
   /* For debug.  */
@@ -628,6 +628,15 @@ grub_efi_mm_init (void)
   /* Release the memory maps.  */
   grub_efi_free_pages ((grub_addr_t) memory_map,
   2 * BYTES_TO_PAGES (MEMORY_MAP_SIZE));
+
+  return GRUB_ERR_NONE;
+}
+
+void
+grub_efi_mm_init (void)
+{
+  if (grub_efi_mm_add_regions (DEFAULT_HEAP_SIZE) != GRUB_ERR_NONE)
+grub_fatal ("%s", grub_errmsg);
 }
 
 #if defined (__aarch64__) || defined (__arm__) || defined (__riscv)
-- 
2.32.0


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

[PATCH v3 09/15] efi: mm: Pass up errors from `add_memory_regions ()`

[Daniel Axtens]

From: Patrick Steinhardt 

The function `add_memory_regions ()` is currently only called on system
initialization to allocate a fixed amount of pages. As such, it didn't
need to return any errors: in case it failed, we cannot proceed anyway.
This will change with the upcoming support for requesting more memory
from the firmware at runtime, where it doesn't make sense anymore to
fail hard.

Refactor the function to return an error to prepare for this. Note that
this does not change the behaviour when initializing the memory system
because `grub_efi_mm_init ()` knows to call `grub_fatal ()` in case
`grub_efi_mm_add_regions ()` returns an error.

Signed-off-by: Patrick Steinhardt 
[dja: clarify error messages]
Signed-off-by: Daniel Axtens 
Reviewed-by: Daniel Kiper 

---

v2: Thank you Glenn and Daniel K for the feedback.

This and all EFI changes were not tested on real hardware.
---
 grub-core/kern/efi/mm.c | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index c8d3968f7f68..4e594659a1ff 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -478,7 +478,7 @@ filter_memory_map (grub_efi_memory_descriptor_t *memory_map,
 }
 
 /* Add memory regions.  */
-static void
+static grub_err_t
 add_memory_regions (grub_efi_memory_descriptor_t *memory_map,
grub_efi_uintn_t desc_size,
grub_efi_memory_descriptor_t *memory_map_end,
@@ -506,9 +506,9 @@ add_memory_regions (grub_efi_memory_descriptor_t 
*memory_map,
   GRUB_EFI_ALLOCATE_ADDRESS,
   GRUB_EFI_LOADER_CODE);
   if (! addr)
-   grub_fatal ("cannot allocate conventional memory %p with %u pages",
-   (void *) ((grub_addr_t) start),
-   (unsigned) pages);
+   return grub_error (GRUB_ERR_OUT_OF_MEMORY,
+   "Memory starting at %p (%u pages) marked as free, 
but EFI would not allocate",
+   (void *) ((grub_addr_t) start), (unsigned) pages);
 
   grub_mm_init_region (addr, PAGES_TO_BYTES (pages));
 
@@ -518,7 +518,11 @@ add_memory_regions (grub_efi_memory_descriptor_t 
*memory_map,
 }
 
   if (required_pages > 0)
-grub_fatal ("too little memory");
+return grub_error (GRUB_ERR_OUT_OF_MEMORY,
+   "could not allocate all requested memory: %" 
PRIuGRUB_UINT64_T " pages still required after iterating EFI memory map",
+   required_pages);
+
+  return GRUB_ERR_NONE;
 }
 
 void
@@ -565,6 +569,7 @@ grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
   grub_efi_memory_descriptor_t *filtered_memory_map_end;
   grub_efi_uintn_t map_size;
   grub_efi_uintn_t desc_size;
+  grub_err_t err;
   int mm_status;
 
   /* Prepare a memory region to store two memory maps.  */
@@ -609,8 +614,11 @@ grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
   sort_memory_map (filtered_memory_map, desc_size, filtered_memory_map_end);
 
   /* Allocate memory regions for GRUB's memory management.  */
-  add_memory_regions (filtered_memory_map, desc_size,
- filtered_memory_map_end, BYTES_TO_PAGES (required_bytes));
+  err = add_memory_regions (filtered_memory_map, desc_size,
+   filtered_memory_map_end,
+   BYTES_TO_PAGES (required_bytes));
+  if (err != GRUB_ERR_NONE)
+return err;
 
 #if 0
   /* For debug.  */
-- 
2.32.0


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

[PATCH v3 11/15] ieee1275: request memory with ibm, client-architecture-support

[Daniel Axtens]

On PowerVM, the first time we boot a Linux partition, we may only get
256MB of real memory area, even if the partition has more memory.

This isn't enough to reliably verify a kernel. Fortunately, the Power
Architecture Platform Reference (PAPR) defines a method we can call to ask
for more memory: the broad and powerful ibm,client-architecture-support
(CAS) method.

CAS can do an enormous amount of things on a PAPR platform: as well as
asking for memory, you can set the supported processor level, the interrupt
controller, hash vs radix mmu, and so on.

If:

 - we are running under what we think is PowerVM (compatible property of /
   begins with "IBM"), and

 - the full amount of RMA is less than 512MB (as determined by the reg
   property of /memory)

then call CAS as follows: (refer to the Linux on Power Architecture
Reference, LoPAR, which is public, at B.5.2.3):

 - Use the "any" PVR value and supply 2 option vectors.

 - Set option vector 1 (PowerPC Server Processor Architecture Level)
   to "ignore".

 - Set option vector 2 with default or Linux-like options, including a
   min-rma-size of 512MB.

 - Set option vector 3 to request Floating Point, VMX and Decimal Floating
   point, but don't abort the boot if we can't get them.

 - Set option vector 4 to request a minimum VP percentage to 1%, which is
   what Linux requests, and is below the default of 10%. Without this,
   some systems with very large or very small configurations fail to boot.

This will cause a CAS reboot and the partition will restart with 512MB
of RMA. Importantly, grub will notice the 512MB and not call CAS again.

Notes about the choices of parameters:

 - A partition can be configured with only 256MB of memory, which would
   mean this request couldn't be satisfied, but PFW refuses to load with
   only 256MB of memory, so it's a bit moot. SLOF will run fine with 256MB,
   but we will never call CAS under qemu/SLOF because /compatible won't
   begin with "IBM".)

 - unspecified CAS vectors take on default values. Some of these values
   might restrict the ability of certain hardware configurations to boot.
   This is why we need to specify the VP percentage in vector 4, which is
   in turn why we need to specify vector 3.

Finally, we should have enough memory to verify a kernel, and we will
reach Linux. One of the first things Linux does while still running under
OpenFirmware is to call CAS with a much fuller set of options (including
asking for 512MB of memory). Linux includes a much more restrictive set of
PVR values and processor support levels, and this CAS invocation will likely
induce another reboot. On this reboot grub will again notice the higher RMA,
and not call CAS. We will get to Linux again, Linux will call CAS again, but
because the values are now set for Linux this will not induce another CAS
reboot and we will finally boot all the way to userspace.

On all subsequent boots, everything will be configured with 512MB of RMA,
so there will be no further CAS reboots from grub. (phyp is super sticky
with the RMA size - it persists even on cold boots. So if you've ever booted
Linux in a partition, you'll probably never have grub call CAS. It'll only
ever fire the first time a partition loads grub, or if you deliberately lower
the amount of memory your partition has below 512MB.)

Signed-off-by: Daniel Axtens 

---

v2: reformat
v3: extend to option vectors 3 & 4

I wrongly assumed that the most compatible way to perform CAS
negotiation was to only set the minimum number of vectors required
to ask for more memory. It turns out that this messes up booting
if the minimum VP capacity would be less than the default 10% in
vector 4.
---
 grub-core/kern/ieee1275/cmain.c  |   3 +
 grub-core/kern/ieee1275/init.c   | 152 +++
 include/grub/ieee1275/ieee1275.h |   8 ++
 3 files changed, 163 insertions(+)

diff --git a/grub-core/kern/ieee1275/cmain.c b/grub-core/kern/ieee1275/cmain.c
index 4442b6a83193..b707798ec3fb 100644
--- a/grub-core/kern/ieee1275/cmain.c
+++ b/grub-core/kern/ieee1275/cmain.c
@@ -123,6 +123,9 @@ grub_ieee1275_find_options (void)
  break;
}
}
+
+  if (grub_strncmp (tmp, "IBM,", 4) == 0)
+   grub_ieee1275_set_flag (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY);
 }
 
   if (is_smartfirmware)
diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
index 2adf4fdfc0e7..cf4bcf2cfbf5 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -200,11 +200,163 @@ heap_init (grub_uint64_t addr, grub_uint64_t len, 
grub_memory_type_t type,
   return 0;
 }
 
+/*
+ * How much memory does OF believe it has? (regardless of whether
+ * it's accessible or not)
+ */
+static grub_err_t
+grub_ieee1275_total_mem (grub_uint64_t *total)
+{
+  grub_ieee1275_phandle_t root;
+  grub_ieee1275_phandle_t memory;
+  grub_uint32_t reg[4];
+  grub_ssize_t reg_size;
+  grub_uint32_t address_cells = 1;
+  grub_uint32_t size_ce

[PATCH v3 05/15] mm: Drop unused unloading of modules on OOM

[Daniel Axtens]

From: Patrick Steinhardt 

In `grub_memalign ()`, there's a commented section which would allow for
unloading of unneeded modules in case where there is not enough free
memory available to satisfy a request. Given that this code is never
compiled in, let's remove it together with `grub_dl_unload_unneeded()`

Signed-off-by: Patrick Steinhardt 
Signed-off-by: Daniel Axtens 
Reviewed-by: Daniel Kiper 
---
 grub-core/kern/dl.c | 20 
 grub-core/kern/mm.c |  8 
 include/grub/dl.h   |  1 -
 3 files changed, 29 deletions(-)

diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c
index b1d3a103ec98..e447fd0fab4e 100644
--- a/grub-core/kern/dl.c
+++ b/grub-core/kern/dl.c
@@ -811,23 +811,3 @@ grub_dl_unload (grub_dl_t mod)
   grub_free (mod);
   return 1;
 }
-
-/* Unload unneeded modules.  */
-void
-grub_dl_unload_unneeded (void)
-{
-  /* Because grub_dl_remove modifies the list of modules, this
- implementation is tricky.  */
-  grub_dl_t p = grub_dl_head;
-
-  while (p)
-{
-  if (grub_dl_unload (p))
-   {
- p = grub_dl_head;
- continue;
-   }
-
-  p = p->next;
-}
-}
diff --git a/grub-core/kern/mm.c b/grub-core/kern/mm.c
index a1f47c0616a2..41e5ea07dc5a 100644
--- a/grub-core/kern/mm.c
+++ b/grub-core/kern/mm.c
@@ -437,14 +437,6 @@ grub_memalign (grub_size_t align, grub_size_t size)
   count++;
   goto again;
 
-#if 0
-case 1:
-  /* Unload unneeded modules.  */
-  grub_dl_unload_unneeded ();
-  count++;
-  goto again;
-#endif
-
 default:
   break;
 }
diff --git a/include/grub/dl.h b/include/grub/dl.h
index d0f4115fe736..acb4d42327d7 100644
--- a/include/grub/dl.h
+++ b/include/grub/dl.h
@@ -203,7 +203,6 @@ grub_dl_t EXPORT_FUNC(grub_dl_load) (const char *name);
 grub_dl_t grub_dl_load_core (void *addr, grub_size_t size);
 grub_dl_t EXPORT_FUNC(grub_dl_load_core_noinit) (void *addr, grub_size_t size);
 int EXPORT_FUNC(grub_dl_unload) (grub_dl_t mod);
-extern void grub_dl_unload_unneeded (void);
 extern int EXPORT_FUNC(grub_dl_ref) (grub_dl_t mod);
 extern int EXPORT_FUNC(grub_dl_unref) (grub_dl_t mod);
 extern int EXPORT_FUNC(grub_dl_ref_count) (grub_dl_t mod);
-- 
2.32.0


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

[PATCH v3 12/15] ieee1275: drop len -= 1 quirk in heap_init

[Daniel Axtens]

This was apparently 'required by some firmware': commit dc9468500919
("2007-02-12  Hollis Blanchard  ").

It's not clear what firmware that was, and what platform from 14 years ago
which exhibited the bug then is still both in use and buggy now.

It doesn't cause issues on qemu (mac99 or pseries) or under PFW for Power8.

I don't have access to old Mac hardware, but if anyone feels especially
strongly we can put it under some feature flag. I really want to disable
it under pseries because it will mess with region merging.

Signed-off-by: Daniel Axtens 
---
 grub-core/kern/ieee1275/init.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
index cf4bcf2cfbf5..60a49301ba8f 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -166,7 +166,6 @@ heap_init (grub_uint64_t addr, grub_uint64_t len, 
grub_memory_type_t type,
  addr = 0x18;
}
 }
-  len -= 1; /* Required for some firmware.  */
 
   /* Never exceed HEAP_MAX_SIZE  */
   if (*total + len > HEAP_MAX_SIZE)
-- 
2.32.0


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

[PATCH v3 07/15] efi: mm: Always request a fixed number of pages on init

[Daniel Axtens]

From: Patrick Steinhardt 

When initializing the EFI memory subsytem, we will by default request a
quarter of the available memory, bounded by a minimum/maximum value.
Given that we're about to extend the EFI memory system to dynamically
request additional pages from the firmware as required, this scaling of
requested memory based on available memory will not make a lot of sense
anymore.

Remove this logic as a preparatory patch such that we'll instead defer
to the runtime memory allocator. Note that ideally, we'd want to change
this after dynamic requesting of pages has been implemented for the EFI
platform. But because we'll need to split up initialization of the
memory subsystem and the request of pages from the firmware, we'd have
to duplicate quite some logic at first only to remove it afterwards
again. This seems quite pointless, so we instead have patches slightly
out of order.

Signed-off-by: Patrick Steinhardt 
Signed-off-by: Daniel Axtens 
Reviewed-by: Daniel Kiper 
---
 grub-core/kern/efi/mm.c | 35 +++
 1 file changed, 3 insertions(+), 32 deletions(-)

diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index d8e4114541a4..0bccd24f304f 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -38,9 +38,8 @@
a multiplier of 4KB.  */
 #define MEMORY_MAP_SIZE0x3000
 
-/* The minimum and maximum heap size for GRUB itself.  */
-#define MIN_HEAP_SIZE  0x10
-#define MAX_HEAP_SIZE  (1600 * 0x10)
+/* The default heap size for GRUB itself in bytes.  */
+#define DEFAULT_HEAP_SIZE  0x10
 
 static void *finish_mmap_buf = 0;
 static grub_efi_uintn_t finish_mmap_size = 0;
@@ -478,23 +477,6 @@ filter_memory_map (grub_efi_memory_descriptor_t 
*memory_map,
   return filtered_desc;
 }
 
-/* Return the total number of pages.  */
-static grub_efi_uint64_t
-get_total_pages (grub_efi_memory_descriptor_t *memory_map,
-grub_efi_uintn_t desc_size,
-grub_efi_memory_descriptor_t *memory_map_end)
-{
-  grub_efi_memory_descriptor_t *desc;
-  grub_efi_uint64_t total = 0;
-
-  for (desc = memory_map;
-   desc < memory_map_end;
-   desc = NEXT_MEMORY_DESCRIPTOR (desc, desc_size))
-total += desc->num_pages;
-
-  return total;
-}
-
 /* Add memory regions.  */
 static void
 add_memory_regions (grub_efi_memory_descriptor_t *memory_map,
@@ -583,8 +565,6 @@ grub_efi_mm_init (void)
   grub_efi_memory_descriptor_t *filtered_memory_map_end;
   grub_efi_uintn_t map_size;
   grub_efi_uintn_t desc_size;
-  grub_efi_uint64_t total_pages;
-  grub_efi_uint64_t required_pages;
   int mm_status;
 
   /* Prepare a memory region to store two memory maps.  */
@@ -624,22 +604,13 @@ grub_efi_mm_init (void)
   filtered_memory_map_end = filter_memory_map (memory_map, filtered_memory_map,
   desc_size, memory_map_end);
 
-  /* By default, request a quarter of the available memory.  */
-  total_pages = get_total_pages (filtered_memory_map, desc_size,
-filtered_memory_map_end);
-  required_pages = (total_pages >> 2);
-  if (required_pages < BYTES_TO_PAGES (MIN_HEAP_SIZE))
-required_pages = BYTES_TO_PAGES (MIN_HEAP_SIZE);
-  else if (required_pages > BYTES_TO_PAGES (MAX_HEAP_SIZE))
-required_pages = BYTES_TO_PAGES (MAX_HEAP_SIZE);
-
   /* Sort the filtered descriptors, so that GRUB can allocate pages
  from smaller regions.  */
   sort_memory_map (filtered_memory_map, desc_size, filtered_memory_map_end);
 
   /* Allocate memory regions for GRUB's memory management.  */
   add_memory_regions (filtered_memory_map, desc_size,
- filtered_memory_map_end, required_pages);
+ filtered_memory_map_end, BYTES_TO_PAGES 
(DEFAULT_HEAP_SIZE));
 
 #if 0
   /* For debug.  */
-- 
2.32.0


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

[PATCH v3 14/15] [RFC] Add memtool module with memory allocation stress-test

[Daniel Axtens]

When working on memory, it's nice to be able to test your work.

Add a memtest module. When compiled with --enable-mm-debug, it exposes
3 commands:

 * lsmem - print all allocations and free space in all regions
 * lsfreemem - print free space in all regions

 * stress_big_allocs - stress test large allocations:
  - how much memory can we allocate in one chunk?
  - how many 1MB chunks can we allocate?
  - check that gap-filling works with a 1MB aligned 900kB alloc + a
 100kB alloc.

Signed-off-by: Daniel Axtens 

---

I haven't addressed most of the change requests yet. This will need lots
of work to get to a mergable state, especially the ability to check
MM_DEBUG in template files so that it can only be built if
--enable-mm-debug is set, ('enable = mm_debug' or something) and that
involves a lot of slow and painful build system hacking. Perhaps later
on. But I have sorted out the copyright assignment so at least that
is clear if anyone else wants to hack on it.
---
 grub-core/Makefile.core.def   |   5 ++
 grub-core/commands/memtools.c | 155 ++
 grub-core/kern/mm.c   |   4 +
 3 files changed, 164 insertions(+)
 create mode 100644 grub-core/commands/memtools.c

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 7159948721e1..795369155a43 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -2532,3 +2532,8 @@ module = {
   common = commands/i386/wrmsr.c;
   enable = x86;
 };
+
+module = {
+  name = memtools;
+  common = commands/memtools.c;
+};
diff --git a/grub-core/commands/memtools.c b/grub-core/commands/memtools.c
new file mode 100644
index ..bb4ad359e013
--- /dev/null
+++ b/grub-core/commands/memtools.c
@@ -0,0 +1,155 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2021 Free Software Foundation, Inc.
+ *  Copyright (C) 2021 IBM Corporation
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+#ifdef MM_DEBUG
+
+static grub_err_t
+grub_cmd_lsmem (grub_command_t cmd __attribute__ ((unused)),
+int argc __attribute__ ((unused)),
+char **args __attribute__ ((unused)))
+
+{
+#ifndef GRUB_MACHINE_EMU
+  grub_mm_dump(0);
+#endif
+
+  return 0;
+}
+
+static grub_err_t
+grub_cmd_lsfreemem (grub_command_t cmd __attribute__ ((unused)),
+   int argc __attribute__ ((unused)),
+   char **args __attribute__ ((unused)))
+
+{
+#ifndef GRUB_MACHINE_EMU
+  grub_mm_dump_free();
+#endif
+
+  return 0;
+}
+
+
+static grub_err_t
+grub_cmd_stress_big_allocs (grub_command_t cmd __attribute__ ((unused)),
+   int argc __attribute__ ((unused)),
+   char **args __attribute__ ((unused)))
+{
+  int i, max_mb, blocks_alloced;
+  void *mem;
+  void **blocklist;
+
+  grub_printf ("Test 1: increasingly sized allocs to 1GB block\n");
+  for (i = 1; i < 1024; i++) {
+grub_printf ("%d MB . ", i);
+mem = grub_malloc (i * 1024 * 1024);
+if (mem == NULL)
+  {
+   grub_printf ("failed\n");
+   break;
+  }
+else
+  grub_free (mem);
+
+if (i % 10 == 0)
+  grub_printf ("\n");
+  }
+
+  max_mb = i - 1;
+  grub_printf ("Max sized allocation we did was %d MB\n", max_mb);
+  
+  grub_printf ("Test 2: 1MB at a time, max 4GB\n");
+  blocklist = grub_calloc (4096, sizeof (void *));
+  for (i = 0; i < 4096; i++)
+{
+  blocklist[i] = grub_malloc (1024 * 1024);
+  if (!blocklist[i])
+   {
+ grub_printf ("Ran out of memory at iteration %d\n", i);
+ break;
+   }
+}
+  blocks_alloced = i;
+  for (i = 0; i < blocks_alloced; i++)
+grub_free (blocklist[i]);
+
+  grub_printf ("\nTest 3: 1MB aligned 900kB + 100kB\n");
+  //grub_mm_debug=1;
+  for (i = 0; i < 4096; i += 2)
+{
+  blocklist[i] = grub_memalign (1024 * 1024, 900 * 1024);
+  if (!blocklist[i])
+   {
+ grub_printf ("Failed big allocation, iteration %d\n", i);
+ blocks_alloced = i;
+ break;
+   }
+
+  blocklist[i + 1] = grub_malloc (100 * 1024);
+  if (!blocklist[i + 1])
+   {
+ grub_printf ("Failed small allocation, iteration %d\n", i);
+ blocks_alloced = i + 1;
+ break;
+   }
+  grub_printf (".");
+ 

[PATCH v3 13/15] ieee1275: support runtime memory claiming

[Daniel Axtens]

On powerpc-ieee1275, we are running out of memory trying to verify
anything. This is because:

 - we have to load an entire file into memory to verify it. This is
   difficult to change with appended signatures.
 - We only have 32MB of heap.
 - Distro kernels are now often around 30MB.

So we want to be able to claim more memory from OpenFirmware for our heap
at runtime.

There are some complications:

 - The grub mm code isn't the only thing that will make claims on
   memory from OpenFirmware:

* PFW/SLOF will have claimed some for their own use.

* The ieee1275 loader will try to find other bits of memory that we
  haven't claimed to place the kernel and initrd when we go to boot.

* Once we load Linux, it will also try to claim memory. It claims
  memory without any reference to /memory/available, it just starts
  at min(top of RMO, 768MB) and works down. So we need to avoid this
  area. See arch/powerpc/kernel/prom_init.c as of v5.11.

 - The smallest amount of memory a ppc64 KVM guest can have is 256MB.
   It doesn't work with distro kernels but can work with custom kernels.
   We should maintain support for that. (ppc32 can boot with even less,
   and we shouldn't break that either.)

 - Even if a VM has more memory, the memory OpenFirmware makes available
   as Real Memory Area can be restricted. Even with our CAS work, an LPAR
   on a PowerVM box is likely to have only 512MB available to OpenFirmware
   even if it has many gigabytes of memory allocated.

What should we do?

We don't know in advance how big the kernel and initrd are going to be,
which makes figuring out how much memory we can take a bit tricky.

To figure out how much memory we should leave unused, I looked at:

 - an Ubuntu 20.04.1 ppc64le pseries KVM guest:
vmlinux: ~30MB
initrd:  ~50MB

 - a RHEL8.2 ppc64le pseries KVM guest:
vmlinux: ~30MB
initrd:  ~30MB

So to give us a little wriggle room, I think we want to leave at least
128MB for the loader to put vmlinux and initrd in memory and leave Linux
with space to satisfy its early allocations.

Allow other space to be allocated at runtime.

Tested-by: Stefan Berger 
Signed-off-by: Daniel Axtens 

---

v2: reformat, rework and explain better, add debug prints with
grub_dprintf
---
 docs/grub-dev.texi |   7 +-
 grub-core/kern/ieee1275/init.c | 267 ++---
 2 files changed, 254 insertions(+), 20 deletions(-)

diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi
index 8a20a9af2f4b..6f3f5fb1fb14 100644
--- a/docs/grub-dev.texi
+++ b/docs/grub-dev.texi
@@ -1059,7 +1059,10 @@ space is limited to 4GiB. GRUB allocates pages from EFI 
for its heap, at most
 1.6 GiB.
 
 On i386-ieee1275 and powerpc-ieee1275 GRUB uses same stack as IEEE1275.
-It allocates at most 32MiB for its heap.
+
+On i386-ieee1275 and powerpc-ieee1275, GRUB will allocate 32MiB for its heap on
+startup. It may allocate more at runtime, as long as at least 128MiB remain 
free
+in OpenFirmware.
 
 On sparc64-ieee1275 stack is 256KiB and heap is 2MiB.
 
@@ -1087,7 +1090,7 @@ In short:
 @item i386-qemu   @tab 60 KiB  @tab < 4 GiB
 @item *-efi   @tab ?   @tab < 1.6 GiB
 @item i386-ieee1275   @tab ?   @tab < 32 MiB
-@item powerpc-ieee1275@tab ?   @tab < 32 MiB
+@item powerpc-ieee1275@tab ?   @tab available memory - 128MiB
 @item sparc64-ieee1275@tab 256KiB  @tab 2 MiB
 @item arm-uboot   @tab 256KiB  @tab 2 MiB
 @item mips(el)-qemu_mips  @tab 2MiB@tab 253 MiB
diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
index 60a49301ba8f..ec591e6e13ec 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -45,13 +45,23 @@
 #include 
 #endif
 
-/* The maximum heap size we're going to claim */
+/* The maximum heap size we're going to claim at boot. Not used by sparc. */
 #ifdef __i386__
 #define HEAP_MAX_SIZE  (unsigned long) (64 * 1024 * 1024)
-#else
+#else // __powerpc__
 #define HEAP_MAX_SIZE  (unsigned long) (32 * 1024 * 1024)
 #endif
 
+/*
+ * The amount of OF space we will not claim here so as to leave space for
+ * the loader and linux to service early allocations.
+ *
+ * In 2021, Daniel Axtens claims that we should leave at least 128MB to
+ * ensure we can load a stock kernel and initrd on a pseries guest with
+ * a 512MB real memory area under PowerVM.
+ */
+#define RUNTIME_MIN_SPACE (128UL * 1024 * 1024)
+
 extern char _start[];
 extern char _end[];
 
@@ -145,16 +155,52 @@ grub_claim_heap (void)
 + GRUB_KERNEL_MACHINE_STACK_SIZE), 0x20);
 }
 #else
-/* Helper for grub_claim_heap.  */
+/* Helpers for mm on powerpc. */
+
+/*
+ * How much memory does OF believe exists in total?
+ *
+ * This isn't necessarily the true total. It can be the total memory
+ * accessible in real mode for a pseries guest, for example.
+ */
+static grub_uint64_t rmo_top;
+
 

[PATCH v3 10/15] efi: mm: Implement runtime addition of pages

[Daniel Axtens]

From: Patrick Steinhardt 

Adjust the interface of `grub_efi_mm_add_regions ()` to take a set of
`GRUB_MM_ADD_REGION_*` flags, which most notably is currently only the
`CONSECUTVE` flag. This allows us to set the function up as callback for
the memory subsystem and have it call out to us in case there's not
enough pages available in the current heap.

Signed-off-by: Patrick Steinhardt 
Signed-off-by: Daniel Axtens 
Reviewed-by: Daniel Kiper 
---
 grub-core/kern/efi/mm.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index 4e594659a1ff..b5160bdffc67 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -482,7 +482,8 @@ static grub_err_t
 add_memory_regions (grub_efi_memory_descriptor_t *memory_map,
grub_efi_uintn_t desc_size,
grub_efi_memory_descriptor_t *memory_map_end,
-   grub_efi_uint64_t required_pages)
+   grub_efi_uint64_t required_pages,
+   unsigned int flags)
 {
   grub_efi_memory_descriptor_t *desc;
 
@@ -496,6 +497,10 @@ add_memory_regions (grub_efi_memory_descriptor_t 
*memory_map,
 
   start = desc->physical_start;
   pages = desc->num_pages;
+
+  if (pages < required_pages && (flags & GRUB_MM_ADD_REGION_CONSECUTIVE))
+   continue;
+
   if (pages > required_pages)
{
  start += PAGES_TO_BYTES (pages - required_pages);
@@ -561,7 +566,7 @@ print_memory_map (grub_efi_memory_descriptor_t *memory_map,
 #endif
 
 static grub_err_t
-grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
+grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes, unsigned int flags)
 {
   grub_efi_memory_descriptor_t *memory_map;
   grub_efi_memory_descriptor_t *memory_map_end;
@@ -616,7 +621,8 @@ grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
   /* Allocate memory regions for GRUB's memory management.  */
   err = add_memory_regions (filtered_memory_map, desc_size,
filtered_memory_map_end,
-   BYTES_TO_PAGES (required_bytes));
+   BYTES_TO_PAGES (required_bytes),
+   flags);
   if (err != GRUB_ERR_NONE)
 return err;
 
@@ -643,8 +649,9 @@ grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
 void
 grub_efi_mm_init (void)
 {
-  if (grub_efi_mm_add_regions (DEFAULT_HEAP_SIZE) != GRUB_ERR_NONE)
+  if (grub_efi_mm_add_regions (DEFAULT_HEAP_SIZE, GRUB_MM_ADD_REGION_NONE) != 
GRUB_ERR_NONE)
 grub_fatal ("%s", grub_errmsg);
+  grub_mm_add_region_fn = grub_efi_mm_add_regions;
 }
 
 #if defined (__aarch64__) || defined (__arm__) || defined (__riscv)
-- 
2.32.0


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

Re: [PATCH v2 13/22] libtasn1: changes for grub compatibility

[Daniel Axtens]

Stefan Berger  writes:

> On 6/30/21 4:40 AM, Daniel Axtens wrote:
>> Do a few things to make libtasn1 compile as part of grub:
>>
>>   - redefine _asn1_strcat. grub removed strcat so replace it with the
>> appropriate calls to memcpy and strlen. Use this internally where
>> strcat was used.
>>
>>   - replace c_isdigit with grub_isdigit (and don't import c-ctype from
>> gnulib) grub_isdigit provides the same functionality as c_isdigit: it
>> determines if the input is an ASCII digit without regard for locale.
>>
>>   - replace GL_ATTRIBUTE_PURE with __attribute__((pure)) which been
>> supported since gcc-2.96. This avoids messing around with gnulib.
>>
>>   - adjust libtasn1.h: drop the ASN1_API logic, it's not needed for our
>> modules. Unconditionally support const and pure attributes and adjust
>> header paths.
>>
>>   - adjust header paths to "grub/libtasn1.h".
>>
>>   - replace a 64 bit division with a call to grub_divmod64, preventing
>> creation of __udivdi3 calls on 32 bit platforms.
>>
>> Signed-off-by: Daniel Axtens 
>>
>> ---
>>
>> v2: Clean up strcat handling, thanks Stefan Berger.
>> ---
>>   grub-core/lib/libtasn1/lib/decoding.c   | 11 ++-
>>   grub-core/lib/libtasn1/lib/element.c|  3 ++-
>>   grub-core/lib/libtasn1/lib/gstr.c   |  4 ++--
>>   grub-core/lib/libtasn1/lib/int.h|  4 ++--
>>   grub-core/lib/libtasn1/lib/parser_aux.c |  7 ---
>>   include/grub/libtasn1.h | 26 ++---
>>   6 files changed, 22 insertions(+), 33 deletions(-)
>>
>> diff --git a/grub-core/lib/libtasn1/lib/decoding.c 
>> b/grub-core/lib/libtasn1/lib/decoding.c
>> index 42f9a92b5d44..3406e1832746 100644
>> --- a/grub-core/lib/libtasn1/lib/decoding.c
>> +++ b/grub-core/lib/libtasn1/lib/decoding.c
>> @@ -32,7 +32,8 @@
>>   #include 
>>   #include 
>>   #include 
>> -#include 
>> +
>> +#define c_isdigit grub_isdigit
>>   
>>   #ifdef DEBUG
>>   # define warn() fprintf(stderr, "%s: %d\n", __func__, __LINE__)
>> @@ -2008,8 +2009,8 @@ asn1_expand_octet_string (asn1_node_const definitions, 
>> asn1_node * element,
>>(p2->type & CONST_ASSIGN))
>>  {
>>strcpy (name, definitions->name);
>> -  strcat (name, ".");
>> -  strcat (name, p2->name);
>> +  _asn1_strcat (name, ".");
>> +  _asn1_strcat (name, p2->name);
>>   
>>len = sizeof (value);
>>result = asn1_read_value (definitions, name, value, &len);
>> @@ -2026,8 +2027,8 @@ asn1_expand_octet_string (asn1_node_const definitions, 
>> asn1_node * element,
>>if (p2)
>>  {
>>strcpy (name, definitions->name);
>> -  strcat (name, ".");
>> -  strcat (name, p2->name);
>> +  _asn1_strcat (name, ".");
>> +  _asn1_strcat (name, p2->name);
>>   
>>result = asn1_create_element (definitions, name, &aux);
>>if (result == ASN1_SUCCESS)
>> diff --git a/grub-core/lib/libtasn1/lib/element.c 
>> b/grub-core/lib/libtasn1/lib/element.c
>> index 539008d8e949..ed761ff56bd9 100644
>> --- a/grub-core/lib/libtasn1/lib/element.c
>> +++ b/grub-core/lib/libtasn1/lib/element.c
>> @@ -30,9 +30,10 @@
>>   #include "parser_aux.h"
>>   #include 
>>   #include "structure.h"
>> -#include "c-ctype.h"
>>   #include "element.h"
>>   
>> +#define c_isdigit grub_isdigit
>> +
>>   void
>>   _asn1_hierarchical_name (asn1_node_const node, char *name, int name_size)
>>   {
>> diff --git a/grub-core/lib/libtasn1/lib/gstr.c 
>> b/grub-core/lib/libtasn1/lib/gstr.c
>> index e91a3a151c0d..a092c9a5a24b 100644
>> --- a/grub-core/lib/libtasn1/lib/gstr.c
>> +++ b/grub-core/lib/libtasn1/lib/gstr.c
>> @@ -36,13 +36,13 @@ _asn1_str_cat (char *dest, size_t dest_tot_size, const 
>> char *src)
>>   
>> if (dest_tot_size - dest_size > str_size)
>>   {
>> -  strcat (dest, src);
>> +  _asn1_strcat (dest, src);
>>   }
>> else
>>   {
>> if (dest_tot_size - dest_size > 0)
>>  {
>> -  strncat (dest, src, (dest_tot_size - dest_size) - 1);
>> +  memcpy (dest + dest_size, src, (dest_tot_size - dest_size) - 1);
>
>
> With dest_size = strlen(dest) this is following the 'pattern' of the 
> #define below.

Hmm, is it?

#define _asn1_strcat(a,b) memcpy((char *)a + strlen((const char *)a), (const 
char *)b, strlen((const char *)b) + 1)

so _asn1_strcat(dest, src) =>
 memcpy(dest + dest_size, src, src_size + 1)

I'm not sure that src_size + 1 = dest_tot_size - 1 ?

I've left it as is for now but I'm happy to be convinced (or more
accurately for my successor to be convinced) that I'm wrong...

Kind regards,
Daniel

>
>
>>dest[dest_tot_size - 1] = 0;
>>  }
>>   }
>> diff --git a/grub-core/lib/libtasn1/lib/int.h 
>> b/grub-core/lib/libtasn1/lib/int.h
>> index ea1625786c1b..4a568efee9c1 100644
>> --- a/grub-core/lib/libtasn1/lib/int.h
>> +++ b/grub-core/lib/libtasn1/lib/int.h
>> @@ -35,7 +35,7 @@
>>   #include 
>>   #endif
>>   
>> -#

Re: [PATCH v2 18/22] appended signatures: parse PKCS#7 signedData and X.509 certificates

[Daniel Axtens]

Stefan Berger  writes:

> On 6/30/21 4:40 AM, Daniel Axtens wrote:
>
>> This code allows us to parse:
>>
>>   - PKCS#7 signedData messages. Only a single signerInfo is supported,
>> which is all that the Linux sign-file utility supports creating
>> out-of-the-box. Only RSA, SHA-256 and SHA-512 are supported.
>> Any certificate embedded in the PKCS#7 message will be ignored.
>>
>>   - X.509 certificates: at least enough to verify the signatures on the
>> PKCS#7 messages. We expect that the certificates embedded in grub will
>> be leaf certificates, not CA certificates. The parser enforces this.
>>
>>   - X.509 certificates support the Extended Key Usage extension and handle
>> it by verifying that the certificate has a single purpose, that is code
>> signing. This is required because Red Hat certificates have both Key
>> Usage and Extended Key Usage extensions present.
>>
>> Signed-off-by: Javier Martinez Canillas  # EKU support
>> Signed-off-by: Daniel Axtens 
>
> A few comments below.
>
>
>>
>> ---
>>
>> v2 changes:
>>
>>   - Handle the Extended Key Usage extension
>>   - Fix 2 leaks in x509 cert parsing
>>   - Improve x509 parser function name
>>   - Constify the data parameter in parser signatures
>>   - Support multiple signers in a pkcs7 message. Accept any passing sig.
>>   - Allow padding after a pkcs7 message in an appended signature, required
>>  to support my model for signers separated in time.
>>   - Fix a test that used GRUB_ERR_NONE rather than ASN1_SUCCESS. They're
>>  both 0 so no harm was done, but better to be correct.
>>   - Various code and comment cleanups.
>>
>> Thanks to Nayna Jain and Stefan Berger for their reviews.
>>
>> revert
>>
>> Signed-off-by: Daniel Axtens 
>> ---
>>   grub-core/commands/appendedsig/appendedsig.h |  118 ++
>>   grub-core/commands/appendedsig/asn1util.c|  103 ++
>>   grub-core/commands/appendedsig/pkcs7.c   |  509 +
>>   grub-core/commands/appendedsig/x509.c| 1079 ++
>>   4 files changed, 1809 insertions(+)
>>   create mode 100644 grub-core/commands/appendedsig/appendedsig.h
>>   create mode 100644 grub-core/commands/appendedsig/asn1util.c
>>   create mode 100644 grub-core/commands/appendedsig/pkcs7.c
>>   create mode 100644 grub-core/commands/appendedsig/x509.c
>>
>> diff --git a/grub-core/commands/appendedsig/appendedsig.h 
>> b/grub-core/commands/appendedsig/appendedsig.h
>> new file mode 100644
>> index ..327d68ddb1b7
>> --- /dev/null
>> +++ b/grub-core/commands/appendedsig/appendedsig.h
>> @@ -0,0 +1,118 @@
>> +/*
>> + *  GRUB  --  GRand Unified Bootloader
>> + *  Copyright (C) 2020  IBM Corporation.
>> + *
>> + *  GRUB is free software: you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation, either version 3 of the License, or
>> + *  (at your option) any later version.
>> + *
>> + *  GRUB is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with GRUB.  If not, see .
>> + */
>> +
>> +#include 
>> +#include 
>> +
>> +extern asn1_node _gnutls_gnutls_asn;
>> +extern asn1_node _gnutls_pkix_asn;
>> +
>> +#define MAX_OID_LEN 32
>> +
>> +/*
>> + * One or more x509 certificates.
>> + *
>> + * We do limited parsing: extracting only the serial, CN and RSA public key.
>> + */
>> +struct x509_certificate
>> +{
>> +  struct x509_certificate *next;
>> +
>> +  grub_uint8_t *serial;
>> +  grub_size_t serial_len;
>> +
>> +  char *subject;
>> +  grub_size_t subject_len;
>> +
>> +  /* We only support RSA public keys. This encodes [modulus, 
>> publicExponent] */
>> +  gcry_mpi_t mpis[2];
>> +};
>> +
>> +/*
>> + * A PKCS#7 signedData signerInfo.
>> + */
>> +struct pkcs7_signerInfo
>> +{
>> +  const gcry_md_spec_t *hash;
>> +  gcry_mpi_t sig_mpi;
>> +};
>> +
>> +/*
>> + * A PKCS#7 signedData message.
>> + *
>> + * We make no attempt to match intelligently, so we don't save any info 
>> about
>> + * the signer.
>> + */
>> +struct pkcs7_signedData
>> +{
>> +  int signerInfo_count;
>> +  struct pkcs7_signerInfo *signerInfos;
>> +};
>> +
>> +
>> +/* Do libtasn1 init */
>> +int asn1_init (void);
>> +
>> +/*
>> + * Import a DER-encoded certificate at 'data', of size 'size'.
>> + *
>> + * Place the results into 'results', which must be already allocated.
>> + */
>> +grub_err_t
>> +parse_x509_certificate (const void *data, grub_size_t size,
>> +struct x509_certificate *results);
>> +
>> +/*
>> + * Release all the storage associated with the x509 certificate.
>> + * If the caller dynamically allocated the certificate, it must free it.
>> + * The caller is also respon