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 <javierm@redhat.com>
This commit is contained in:
Javier Martinez Canillas 2019-03-22 14:24:05 +01:00
parent 242b306a29
commit c1ccaf8a0e
No known key found for this signature in database
GPG Key ID: C751E590D63F3D69
4 changed files with 163 additions and 1 deletions

View File

@ -0,0 +1,79 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
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 <pjones@redhat.com>
---
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"

View File

@ -0,0 +1,76 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas <javierm@redhat.com>
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 <javierm@redhat.com>
---
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;

View File

@ -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 Patch0284: 0284-blscfg-fallback-to-default_kernelopts-if-BLS-option-.patch
Patch0285: 0285-grub-switch-to-blscfg-copy-increment.mod-for-legacy-.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 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

View File

@ -7,7 +7,7 @@
Name: grub2 Name: grub2
Epoch: 1 Epoch: 1
Version: 2.02 Version: 2.02
Release: 73%{?dist} Release: 74%{?dist}
Summary: Bootloader with support for Linux, Multiboot and more Summary: Bootloader with support for Linux, Multiboot and more
License: GPLv3+ License: GPLv3+
URL: http://www.gnu.org/software/grub/ URL: http://www.gnu.org/software/grub/
@ -480,6 +480,11 @@ rm -r /boot/grub2.tmp/ || :
%endif %endif
%changelog %changelog
* Fri Mar 22 2019 Javier Martinez Canillas <javierm@redhat.com> - 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 <javierm@redhat.com> - 2.02-73 * Wed Mar 20 2019 Javier Martinez Canillas <javierm@redhat.com> - 2.02-73
- Only set blsdir if /boot/loader/entries is in a btrfs or zfs partition - Only set blsdir if /boot/loader/entries is in a btrfs or zfs partition
Related: rhbz#1688453 Related: rhbz#1688453