From c1ccaf8a0e204d9c86a80256c9999e4384c61f10 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Fri, 22 Mar 2019 14:24:05 +0100 Subject: [PATCH] Fix some BLS snippets not being displayed in the GRUB menu There was an error in the logic that stored the parsed BLS snippets in the sorted linked list that is used to populate the GRUB boot menu entries. Also add a fix found by coverity scan about a possible undefined behaviour due grub_efi_status_t having the wrong type. Resolves: rhbz#1691232 Signed-off-by: Javier Martinez Canillas --- 0287-Fix-the-type-of-grub_efi_status_t.patch | 79 +++++++++++++++++++ ...-grub_list_t-and-the-GRUB_AS_LIST-ma.patch | 76 ++++++++++++++++++ grub.patches | 2 + grub2.spec | 7 +- 4 files changed, 163 insertions(+), 1 deletion(-) create mode 100644 0287-Fix-the-type-of-grub_efi_status_t.patch create mode 100644 0288-blscfg-don-t-use-grub_list_t-and-the-GRUB_AS_LIST-ma.patch diff --git a/0287-Fix-the-type-of-grub_efi_status_t.patch b/0287-Fix-the-type-of-grub_efi_status_t.patch new file mode 100644 index 0000000..e937c27 --- /dev/null +++ b/0287-Fix-the-type-of-grub_efi_status_t.patch @@ -0,0 +1,79 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Peter Jones +Date: Thu, 21 Mar 2019 13:06:06 -0400 +Subject: [PATCH] Fix the type of grub_efi_status_t + +Currently, in some builds with some checkers, we see: + +1. grub-core/disk/efi/efidisk.c:601: error[shiftTooManyBitsSigned]: Shifting signed 64-bit value by 63 bits is undefined behaviour + +This is because grub_efi_status_t is defined as grub_efi_intn_t, which is +signed, and shifting into the sign bit is not defined behavior. UEFI fixed +this in the spec in 2.3: + +2.3 | Change the defined type of EFI_STATUS from INTN to UINTN | May 7, 2009 + +And the current EDK2 code has: +MdePkg/Include/Base.h-// +MdePkg/Include/Base.h-// Status codes common to all execution phases +MdePkg/Include/Base.h-// +MdePkg/Include/Base.h:typedef UINTN RETURN_STATUS; +MdePkg/Include/Base.h- +MdePkg/Include/Base.h-/** +MdePkg/Include/Base.h- Produces a RETURN_STATUS code with the highest bit set. +MdePkg/Include/Base.h- +MdePkg/Include/Base.h- @param StatusCode The status code value to convert into a warning code. +MdePkg/Include/Base.h- StatusCode must be in the range 0x00000000..0x7FFFFFFF. +MdePkg/Include/Base.h- +MdePkg/Include/Base.h- @return The value specified by StatusCode with the highest bit set. +MdePkg/Include/Base.h- +MdePkg/Include/Base.h-**/ +MdePkg/Include/Base.h-#define ENCODE_ERROR(StatusCode) ((RETURN_STATUS)(MAX_BIT | (StatusCode))) +MdePkg/Include/Base.h- +MdePkg/Include/Base.h-/** +MdePkg/Include/Base.h- Produces a RETURN_STATUS code with the highest bit clear. +MdePkg/Include/Base.h- +MdePkg/Include/Base.h- @param StatusCode The status code value to convert into a warning code. +MdePkg/Include/Base.h- StatusCode must be in the range 0x00000000..0x7FFFFFFF. +MdePkg/Include/Base.h- +MdePkg/Include/Base.h- @return The value specified by StatusCode with the highest bit clear. +MdePkg/Include/Base.h- +MdePkg/Include/Base.h-**/ +MdePkg/Include/Base.h-#define ENCODE_WARNING(StatusCode) ((RETURN_STATUS)(StatusCode)) +MdePkg/Include/Base.h- +MdePkg/Include/Base.h-/** +MdePkg/Include/Base.h- Returns TRUE if a specified RETURN_STATUS code is an error code. +MdePkg/Include/Base.h- +MdePkg/Include/Base.h- This function returns TRUE if StatusCode has the high bit set. Otherwise, FALSE is returned. +MdePkg/Include/Base.h- +MdePkg/Include/Base.h- @param StatusCode The status code value to evaluate. +MdePkg/Include/Base.h- +MdePkg/Include/Base.h- @retval TRUE The high bit of StatusCode is set. +MdePkg/Include/Base.h- @retval FALSE The high bit of StatusCode is clear. +MdePkg/Include/Base.h- +MdePkg/Include/Base.h-**/ +MdePkg/Include/Base.h-#define RETURN_ERROR(StatusCode) (((INTN)(RETURN_STATUS)(StatusCode)) < 0) +... +Uefi/UefiBaseType.h:typedef RETURN_STATUS EFI_STATUS; + +This patch makes grub's implementation match the Edk2 declaration with regards +to the signedness of the type. + +Signed-off-by: Peter Jones +--- + include/grub/efi/api.h | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h +index b337e1a193d..0c15cffa23c 100644 +--- a/include/grub/efi/api.h ++++ b/include/grub/efi/api.h +@@ -536,7 +536,7 @@ typedef grub_uint64_t grub_efi_uint64_t; + typedef grub_uint8_t grub_efi_char8_t; + typedef grub_uint16_t grub_efi_char16_t; + +-typedef grub_efi_intn_t grub_efi_status_t; ++typedef grub_efi_uintn_t grub_efi_status_t; + /* Make grub_efi_status_t reasonably printable. */ + #if GRUB_CPU_SIZEOF_VOID_P == 8 + #define PRIxGRUB_EFI_STATUS "lx" diff --git a/0288-blscfg-don-t-use-grub_list_t-and-the-GRUB_AS_LIST-ma.patch b/0288-blscfg-don-t-use-grub_list_t-and-the-GRUB_AS_LIST-ma.patch new file mode 100644 index 0000000..a05da65 --- /dev/null +++ b/0288-blscfg-don-t-use-grub_list_t-and-the-GRUB_AS_LIST-ma.patch @@ -0,0 +1,76 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Javier Martinez Canillas +Date: Fri, 22 Mar 2019 11:14:26 +0100 +Subject: [PATCH] blscfg: don't use grub_list_t and the GRUB_AS_LIST() macro + +We are not using GRUB's list functions anyways since we want to add new +items in the middle ot the list but GRUB's grub_list_push() only allows +to add new items at the beginning of the list. So don't use grub_list_t +and GRUB_AS_LIST() macro and just reference struct bls_entry * directly. + +We can't change GRUB lists API because we want the blscfg module to not +need external symbols so it can be updated without updating GRUB's core. + +This also solves a bug where the struct bls_entry .next field wasn't set +correctly which caused some entries to not be populated in the grub menu. + +Resolves: rhbz#1691232 + +Signed-off-by: Javier Martinez Canillas +--- + grub-core/commands/blscfg.c | 10 +++++----- + include/grub/menu.h | 2 +- + 2 files changed, 6 insertions(+), 6 deletions(-) + +diff --git a/grub-core/commands/blscfg.c b/grub-core/commands/blscfg.c +index 5635066e3eb..bb93b7f4904 100644 +--- a/grub-core/commands/blscfg.c ++++ b/grub-core/commands/blscfg.c +@@ -350,13 +350,13 @@ static int bls_cmp(const struct bls_entry *e0, const struct bls_entry *e1) + return r; + } + +-static void list_add_tail(grub_list_t head, grub_list_t item) ++static void list_add_tail(struct bls_entry *head, struct bls_entry *item) + { + item->next = head; + if (head->prev) +- (*head->prev)->next = item; ++ head->prev->next = item; + item->prev = head->prev; +- head->prev = &item; ++ head->prev = item; + } + + static int bls_add_entry(struct bls_entry *entry) +@@ -378,7 +378,7 @@ static int bls_add_entry(struct bls_entry *entry) + + if (rc == 1) { + grub_dprintf ("blscfg", "Add entry with id \"%s\"\n", entry->filename); +- list_add_tail (GRUB_AS_LIST (e), GRUB_AS_LIST (entry)); ++ list_add_tail (e, entry); + if (e == entries) { + entries = entry; + entry->prev = NULL; +@@ -391,7 +391,7 @@ static int bls_add_entry(struct bls_entry *entry) + if (last) { + grub_dprintf ("blscfg", "Add entry with id \"%s\"\n", entry->filename); + last->next = entry; +- entry->prev = &last; ++ entry->prev = last; + } + + return 0; +diff --git a/include/grub/menu.h b/include/grub/menu.h +index eea493f74b1..0acdc2aa6bf 100644 +--- a/include/grub/menu.h ++++ b/include/grub/menu.h +@@ -23,7 +23,7 @@ + struct bls_entry + { + struct bls_entry *next; +- struct bls_entry **prev; ++ struct bls_entry *prev; + struct keyval **keyvals; + int nkeyvals; + char *filename; diff --git a/grub.patches b/grub.patches index d53bdeb..c80c2b5 100644 --- a/grub.patches +++ b/grub.patches @@ -284,3 +284,5 @@ Patch0283: 0283-Check-if-blsdir-exists-before-attempting-to-get-it-s.patch Patch0284: 0284-blscfg-fallback-to-default_kernelopts-if-BLS-option-.patch Patch0285: 0285-grub-switch-to-blscfg-copy-increment.mod-for-legacy-.patch Patch0286: 0286-Only-set-blsdir-if-boot-loader-entries-is-in-a-btrfs.patch +Patch0287: 0287-Fix-the-type-of-grub_efi_status_t.patch +Patch0288: 0288-blscfg-don-t-use-grub_list_t-and-the-GRUB_AS_LIST-ma.patch diff --git a/grub2.spec b/grub2.spec index acbce0f..584ce09 100644 --- a/grub2.spec +++ b/grub2.spec @@ -7,7 +7,7 @@ Name: grub2 Epoch: 1 Version: 2.02 -Release: 73%{?dist} +Release: 74%{?dist} Summary: Bootloader with support for Linux, Multiboot and more License: GPLv3+ URL: http://www.gnu.org/software/grub/ @@ -480,6 +480,11 @@ rm -r /boot/grub2.tmp/ || : %endif %changelog +* Fri Mar 22 2019 Javier Martinez Canillas - 2.02-74 +- Fix some BLS snippets not being displayed in the GRUB menu + Resolves: rhbz#1691232 +- Fix possible undefined behaviour due wrong grub_efi_status_t type (pjones) + * Wed Mar 20 2019 Javier Martinez Canillas - 2.02-73 - Only set blsdir if /boot/loader/entries is in a btrfs or zfs partition Related: rhbz#1688453