backport patches form upstream
This commit is contained in:
parent
255c1ac3e1
commit
a66ca38513
110
backport-Correctly-free-memory-allocated-in-handle_image.patch
Normal file
110
backport-Correctly-free-memory-allocated-in-handle_image.patch
Normal file
@ -0,0 +1,110 @@
|
||||
From 1e985a3a238100ca5f4bda3e269a9eaec9bda74b Mon Sep 17 00:00:00 2001
|
||||
From: Dennis Tseng <dennis.tseng@suse.com>
|
||||
Date: Sun, 25 Jun 2023 15:07:39 +0800
|
||||
Subject: [PATCH] Correctly free memory allocated in handle_image()
|
||||
|
||||
Currently pe's handle_image() function has two related issues, which are
|
||||
a memory leak in most error paths and an incorrect FreePool() call in
|
||||
some error paths.
|
||||
|
||||
This patch adds the correct FreePages() calls to most error paths, and
|
||||
switches the FreePool() call to match them.
|
||||
|
||||
[commit message re-written to be more informative by pjones]
|
||||
|
||||
Signed-off-by: Dennis Tseng <dennis.tseng@suse.com>
|
||||
---
|
||||
pe.c | 13 +++++++++++--
|
||||
1 file changed, 11 insertions(+), 2 deletions(-)
|
||||
|
||||
diff --git a/pe.c b/pe.c
|
||||
index 37753d2..e15b89f 100644
|
||||
--- a/pe.c
|
||||
+++ b/pe.c
|
||||
@@ -747,6 +747,7 @@ handle_image (void *data, unsigned int datasize,
|
||||
(Section->Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE) &&
|
||||
(mok_policy & MOK_POLICY_REQUIRE_NX)) {
|
||||
perror(L"Section %d is writable and executable\n", i);
|
||||
+ BS->FreePages(*alloc_address, *alloc_pages);
|
||||
return EFI_UNSUPPORTED;
|
||||
}
|
||||
|
||||
@@ -772,6 +773,7 @@ handle_image (void *data, unsigned int datasize,
|
||||
if (CompareMem(Section->Name, ".reloc\0\0", 8) == 0) {
|
||||
if (RelocSection) {
|
||||
perror(L"Image has multiple relocation sections\n");
|
||||
+ BS->FreePages(*alloc_address, *alloc_pages);
|
||||
return EFI_UNSUPPORTED;
|
||||
}
|
||||
/* If it has nonzero sizes, and our bounds check
|
||||
@@ -785,6 +787,7 @@ handle_image (void *data, unsigned int datasize,
|
||||
RelocSection = Section;
|
||||
} else {
|
||||
perror(L"Relocation section is invalid \n");
|
||||
+ BS->FreePages(*alloc_address, *alloc_pages);
|
||||
return EFI_UNSUPPORTED;
|
||||
}
|
||||
}
|
||||
@@ -795,10 +798,12 @@ handle_image (void *data, unsigned int datasize,
|
||||
|
||||
if (!base) {
|
||||
perror(L"Section %d has invalid base address\n", i);
|
||||
+ BS->FreePages(*alloc_address, *alloc_pages);
|
||||
return EFI_UNSUPPORTED;
|
||||
}
|
||||
if (!end) {
|
||||
perror(L"Section %d has zero size\n", i);
|
||||
+ BS->FreePages(*alloc_address, *alloc_pages);
|
||||
return EFI_UNSUPPORTED;
|
||||
}
|
||||
|
||||
@@ -806,6 +811,7 @@ handle_image (void *data, unsigned int datasize,
|
||||
(Section->VirtualAddress < context.SizeOfHeaders ||
|
||||
Section->PointerToRawData < context.SizeOfHeaders)) {
|
||||
perror(L"Section %d is inside image headers\n", i);
|
||||
+ BS->FreePages(*alloc_address, *alloc_pages);
|
||||
return EFI_UNSUPPORTED;
|
||||
}
|
||||
|
||||
@@ -814,6 +820,7 @@ handle_image (void *data, unsigned int datasize,
|
||||
} else {
|
||||
if (Section->PointerToRawData < context.SizeOfHeaders) {
|
||||
perror(L"Section %d is inside image headers\n", i);
|
||||
+ BS->FreePages(*alloc_address, *alloc_pages);
|
||||
return EFI_UNSUPPORTED;
|
||||
}
|
||||
|
||||
@@ -831,7 +838,7 @@ handle_image (void *data, unsigned int datasize,
|
||||
|
||||
if (context.NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC) {
|
||||
perror(L"Image has no relocation entry\n");
|
||||
- FreePool(buffer);
|
||||
+ BS->FreePages(*alloc_address, *alloc_pages);
|
||||
return EFI_UNSUPPORTED;
|
||||
}
|
||||
|
||||
@@ -844,7 +851,7 @@ handle_image (void *data, unsigned int datasize,
|
||||
|
||||
if (EFI_ERROR(efi_status)) {
|
||||
perror(L"Relocation failed: %r\n", efi_status);
|
||||
- FreePool(buffer);
|
||||
+ BS->FreePages(*alloc_address, *alloc_pages);
|
||||
return efi_status;
|
||||
}
|
||||
}
|
||||
@@ -910,10 +917,12 @@ handle_image (void *data, unsigned int datasize,
|
||||
|
||||
if (!found_entry_point) {
|
||||
perror(L"Entry point is not within sections\n");
|
||||
+ BS->FreePages(*alloc_address, *alloc_pages);
|
||||
return EFI_UNSUPPORTED;
|
||||
}
|
||||
if (found_entry_point > 1) {
|
||||
perror(L"%d sections contain entry point\n", found_entry_point);
|
||||
+ BS->FreePages(*alloc_address, *alloc_pages);
|
||||
return EFI_UNSUPPORTED;
|
||||
}
|
||||
|
||||
--
|
||||
2.27.0
|
||||
|
||||
@ -0,0 +1,161 @@
|
||||
From 89972ae25c133df31290f394413c19ea903219ad Mon Sep 17 00:00:00 2001
|
||||
From: Long Qin <qin.long@intel.com>
|
||||
Date: Wed, 1 Nov 2017 16:10:04 +0800
|
||||
Subject: [PATCH] CryptoPkg/BaseCryptLib: Fix buffer overflow issue in realloc
|
||||
wrapper
|
||||
|
||||
There is one long-standing problem in CRT realloc wrapper, which will
|
||||
cause the obvious buffer overflow issue when re-allocating one bigger
|
||||
memory block:
|
||||
void *realloc (void *ptr, size_t size)
|
||||
{
|
||||
//
|
||||
// BUG: hardcode OldSize == size! We have no any knowledge about
|
||||
// memory size of original pointer ptr.
|
||||
//
|
||||
return ReallocatePool ((UINTN) size, (UINTN) size, ptr);
|
||||
}
|
||||
This patch introduces one extra header to record the memory buffer size
|
||||
information when allocating memory block from malloc routine, and re-wrap
|
||||
the realloc() and free() routines to remove this BUG.
|
||||
|
||||
Cc: Laszlo Ersek <lersek@redhat.com>
|
||||
Cc: Ting Ye <ting.ye@intel.com>
|
||||
Cc: Jian J Wang <jian.j.wang@intel.com>
|
||||
Contributed-under: TianoCore Contribution Agreement 1.0
|
||||
Signed-off-by: Qin Long <qin.long@intel.com>
|
||||
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
|
||||
Validated-by: Jian J Wang <jian.j.wang@intel.com>
|
||||
|
||||
Cherry picked from https://github.com/tianocore/edk2.git, commit
|
||||
cf8197a39d07179027455421a182598bd6989999. Changes:
|
||||
* `SIGNATURE_32` -> `EFI_SIGNATURE_32`
|
||||
* Added definition of `MIN`
|
||||
|
||||
Fixes https://github.com/rhboot/shim/issues/538
|
||||
|
||||
Signed-off-by: Nicholas Bishop <nicholasbishop@google.com>
|
||||
---
|
||||
Cryptlib/SysCall/BaseMemAllocation.c | 85 +++++++++++++++++++++++++---
|
||||
1 file changed, 78 insertions(+), 7 deletions(-)
|
||||
|
||||
diff --git a/Cryptlib/SysCall/BaseMemAllocation.c b/Cryptlib/SysCall/BaseMemAllocation.c
|
||||
index 792b29e..7a565ff 100644
|
||||
--- a/Cryptlib/SysCall/BaseMemAllocation.c
|
||||
+++ b/Cryptlib/SysCall/BaseMemAllocation.c
|
||||
@@ -15,6 +15,20 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
|
||||
|
||||
#include <OpenSslSupport.h>
|
||||
|
||||
+//
|
||||
+// Extra header to record the memory buffer size from malloc routine.
|
||||
+//
|
||||
+#define CRYPTMEM_HEAD_SIGNATURE EFI_SIGNATURE_32('c','m','h','d')
|
||||
+typedef struct {
|
||||
+ UINT32 Signature;
|
||||
+ UINT32 Reserved;
|
||||
+ UINTN Size;
|
||||
+} CRYPTMEM_HEAD;
|
||||
+
|
||||
+#define CRYPTMEM_OVERHEAD sizeof(CRYPTMEM_HEAD)
|
||||
+
|
||||
+#define MIN(a, b) ({(a) < (b) ? (a) : (b);})
|
||||
+
|
||||
//
|
||||
// -- Memory-Allocation Routines --
|
||||
//
|
||||
@@ -22,27 +36,84 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
|
||||
/* Allocates memory blocks */
|
||||
void *malloc (size_t size)
|
||||
{
|
||||
- return AllocatePool ((UINTN) size);
|
||||
+ CRYPTMEM_HEAD *PoolHdr;
|
||||
+ UINTN NewSize;
|
||||
+ VOID *Data;
|
||||
+
|
||||
+ //
|
||||
+ // Adjust the size by the buffer header overhead
|
||||
+ //
|
||||
+ NewSize = (UINTN)(size) + CRYPTMEM_OVERHEAD;
|
||||
+
|
||||
+ Data = AllocatePool (NewSize);
|
||||
+ if (Data != NULL) {
|
||||
+ PoolHdr = (CRYPTMEM_HEAD *)Data;
|
||||
+ //
|
||||
+ // Record the memory brief information
|
||||
+ //
|
||||
+ PoolHdr->Signature = CRYPTMEM_HEAD_SIGNATURE;
|
||||
+ PoolHdr->Size = size;
|
||||
+
|
||||
+ return (VOID *)(PoolHdr + 1);
|
||||
+ } else {
|
||||
+ //
|
||||
+ // The buffer allocation failed.
|
||||
+ //
|
||||
+ return NULL;
|
||||
+ }
|
||||
}
|
||||
|
||||
/* Reallocate memory blocks */
|
||||
void *realloc (void *ptr, size_t size)
|
||||
{
|
||||
- //
|
||||
- // BUG: hardcode OldSize == size! We have no any knowledge about
|
||||
- // memory size of original pointer ptr.
|
||||
- //
|
||||
- return ReallocatePool (ptr, (UINTN) size, (UINTN) size);
|
||||
+ CRYPTMEM_HEAD *OldPoolHdr;
|
||||
+ CRYPTMEM_HEAD *NewPoolHdr;
|
||||
+ UINTN OldSize;
|
||||
+ UINTN NewSize;
|
||||
+ VOID *Data;
|
||||
+
|
||||
+ NewSize = (UINTN)size + CRYPTMEM_OVERHEAD;
|
||||
+ Data = AllocatePool (NewSize);
|
||||
+ if (Data != NULL) {
|
||||
+ NewPoolHdr = (CRYPTMEM_HEAD *)Data;
|
||||
+ NewPoolHdr->Signature = CRYPTMEM_HEAD_SIGNATURE;
|
||||
+ NewPoolHdr->Size = size;
|
||||
+ if (ptr != NULL) {
|
||||
+ //
|
||||
+ // Retrieve the original size from the buffer header.
|
||||
+ //
|
||||
+ OldPoolHdr = (CRYPTMEM_HEAD *)ptr - 1;
|
||||
+ ASSERT (OldPoolHdr->Signature == CRYPTMEM_HEAD_SIGNATURE);
|
||||
+ OldSize = OldPoolHdr->Size;
|
||||
+
|
||||
+ //
|
||||
+ // Duplicate the buffer content.
|
||||
+ //
|
||||
+ CopyMem ((VOID *)(NewPoolHdr + 1), ptr, MIN (OldSize, size));
|
||||
+ FreePool ((VOID *)OldPoolHdr);
|
||||
+ }
|
||||
+
|
||||
+ return (VOID *)(NewPoolHdr + 1);
|
||||
+ } else {
|
||||
+ //
|
||||
+ // The buffer allocation failed.
|
||||
+ //
|
||||
+ return NULL;
|
||||
+ }
|
||||
}
|
||||
|
||||
/* De-allocates or frees a memory block */
|
||||
void free (void *ptr)
|
||||
{
|
||||
+ CRYPTMEM_HEAD *PoolHdr;
|
||||
+
|
||||
//
|
||||
// In Standard C, free() handles a null pointer argument transparently. This
|
||||
// is not true of FreePool() below, so protect it.
|
||||
//
|
||||
if (ptr != NULL) {
|
||||
- FreePool (ptr);
|
||||
+ PoolHdr = (CRYPTMEM_HEAD *)ptr - 1;
|
||||
+ ASSERT (PoolHdr->Signature == CRYPTMEM_HEAD_SIGNATURE);
|
||||
+ FreePool (PoolHdr);
|
||||
}
|
||||
}
|
||||
--
|
||||
2.27.0
|
||||
|
||||
56
backport-CryptoPkg-BaseCryptLib-fix-NULL-dereference.patch
Normal file
56
backport-CryptoPkg-BaseCryptLib-fix-NULL-dereference.patch
Normal file
@ -0,0 +1,56 @@
|
||||
From 53509eaf2253e23bfb552e9386fd0877abe592b4 Mon Sep 17 00:00:00 2001
|
||||
From: Jian J Wang <jian.j.wang@intel.com>
|
||||
Date: Thu, 25 Apr 2019 23:42:16 +0800
|
||||
Subject: [PATCH] CryptoPkg/BaseCryptLib: fix NULL dereference
|
||||
|
||||
AuthenticodeVerify() calls OpenSSLs d2i_PKCS7() API to parse asn encoded
|
||||
signed authenticode pkcs#7 data. when this successfully returns, a type
|
||||
check is done by calling PKCS7_type_is_signed() and then
|
||||
Pkcs7->d.sign->contents->type is used. It is possible to construct an asn1
|
||||
blob that successfully decodes and have d2i_PKCS7() return a valid pointer
|
||||
and have PKCS7_type_is_signed() also return success but have Pkcs7->d.sign
|
||||
be a NULL pointer.
|
||||
|
||||
Looking at how PKCS7_verify() [inside of OpenSSL] implements checking for
|
||||
pkcs7 structs it does the following:
|
||||
- call PKCS7_type_is_signed()
|
||||
- call PKCS7_get_detached()
|
||||
Looking into how PKCS7_get_detatched() is implemented, it checks to see if
|
||||
p7->d.sign is NULL or if p7->d.sign->contents->d.ptr is NULL.
|
||||
|
||||
As such, the fix is to do the same as OpenSSL after calling d2i_PKCS7().
|
||||
- Add call to PKS7_get_detached() to existing error handling
|
||||
|
||||
Cc: Chao Zhang <chao.b.zhang@intel.com>
|
||||
Cc: Jiewen Yao <jiewen.yao@intel.com>
|
||||
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
|
||||
Cherry-picked-from: https://github.com/tianocore/edk2/commit/26442d11e620a9e81c019a24a4ff38441c64ba10
|
||||
---
|
||||
Cryptlib/Pk/CryptAuthenticode.c | 4 ++--
|
||||
1 file changed, 2 insertions(+), 2 deletions(-)
|
||||
|
||||
diff --git a/Cryptlib/Pk/CryptAuthenticode.c b/Cryptlib/Pk/CryptAuthenticode.c
|
||||
index 74e50a2..f6f988b 100644
|
||||
--- a/Cryptlib/Pk/CryptAuthenticode.c
|
||||
+++ b/Cryptlib/Pk/CryptAuthenticode.c
|
||||
@@ -9,7 +9,7 @@
|
||||
AuthenticodeVerify() will get PE/COFF Authenticode and will do basic check for
|
||||
data structure.
|
||||
|
||||
-Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.<BR>
|
||||
+Copyright (c) 2011 - 2019, Intel Corporation. All rights reserved.<BR>
|
||||
This program and the accompanying materials
|
||||
are licensed and made available under the terms and conditions of the BSD License
|
||||
which accompanies this distribution. The full text of the license may be found at
|
||||
@@ -106,7 +106,7 @@ AuthenticodeVerify (
|
||||
//
|
||||
// Check if it's PKCS#7 Signed Data (for Authenticode Scenario)
|
||||
//
|
||||
- if (!PKCS7_type_is_signed (Pkcs7)) {
|
||||
+ if (!PKCS7_type_is_signed (Pkcs7) || PKCS7_get_detached (Pkcs7)) {
|
||||
goto _Exit;
|
||||
}
|
||||
|
||||
--
|
||||
2.27.0
|
||||
|
||||
70
backport-Discard-load-options-that-start-with-a-NUL.patch
Normal file
70
backport-Discard-load-options-that-start-with-a-NUL.patch
Normal file
@ -0,0 +1,70 @@
|
||||
From 14d63398298c8de23036a4cf61594108b7345863 Mon Sep 17 00:00:00 2001
|
||||
From: Robbie Harwood <rharwood@redhat.com>
|
||||
Date: Tue, 23 Aug 2022 12:07:16 -0400
|
||||
Subject: [PATCH] Discard load-options that start with a NUL
|
||||
|
||||
In 6c8d08c0af4768c715b79c8ec25141d56e34f8b4 ("shim: Ignore UEFI
|
||||
LoadOptions that are just NUL characters."), a check was added to
|
||||
discard load options that are entirely NUL. We now see some firmwares
|
||||
that start LoadOptions with a NUL, and then follow it with garbage (path
|
||||
to directory containing loaders). Widen the check to just discard
|
||||
anything that starts with a NUL.
|
||||
|
||||
Resolves: #490
|
||||
Related: #95
|
||||
See-also: https://bugzilla.redhat.com/show_bug.cgi?id=2113005
|
||||
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
|
||||
---
|
||||
include/ucs2.h | 18 ------------------
|
||||
load-options.c | 7 ++++++-
|
||||
2 files changed, 6 insertions(+), 19 deletions(-)
|
||||
|
||||
diff --git a/include/ucs2.h b/include/ucs2.h
|
||||
index ee038ce..87eab32 100644
|
||||
--- a/include/ucs2.h
|
||||
+++ b/include/ucs2.h
|
||||
@@ -63,22 +63,4 @@ StrCSpn(const CHAR16 *s, const CHAR16 *reject)
|
||||
return ret;
|
||||
}
|
||||
|
||||
-/*
|
||||
- * Test if an entire buffer is nothing but NUL characters. This
|
||||
- * implementation "gracefully" ignores the difference between the
|
||||
- * UTF-8/ASCII 1-byte NUL and the UCS-2 2-byte NUL.
|
||||
- */
|
||||
-static inline bool
|
||||
-__attribute__((__unused__))
|
||||
-is_all_nuls(UINT8 *data, UINTN data_size)
|
||||
-{
|
||||
- UINTN i;
|
||||
-
|
||||
- for (i = 0; i < data_size; i++) {
|
||||
- if (data[i] != 0)
|
||||
- return false;
|
||||
- }
|
||||
- return true;
|
||||
-}
|
||||
-
|
||||
#endif /* SHIM_UCS2_H */
|
||||
diff --git a/load-options.c b/load-options.c
|
||||
index c6bb742..a8c6e1a 100644
|
||||
--- a/load-options.c
|
||||
+++ b/load-options.c
|
||||
@@ -404,8 +404,13 @@ parse_load_options(EFI_LOADED_IMAGE *li)
|
||||
|
||||
/*
|
||||
* Apparently sometimes we get L"\0\0"? Which isn't useful at all.
|
||||
+ *
|
||||
+ * Possibly related, but some boards have additional data before the
|
||||
+ * size which is garbage (it's a weird path to the directory
|
||||
+ * containing the loaders). Known boards that do this: Kontron VX3040
|
||||
+ * (AMI), ASUS B85M-E, and at least one "older Dell laptop".
|
||||
*/
|
||||
- if (is_all_nuls(li->LoadOptions, li->LoadOptionsSize))
|
||||
+ if (((CHAR16 *)li->LoadOptions)[0] == 0)
|
||||
return EFI_SUCCESS;
|
||||
|
||||
/*
|
||||
--
|
||||
2.27.0
|
||||
|
||||
@ -0,0 +1,60 @@
|
||||
From f23883ccf78f1f605a272f9e5700f47e5494a71d Mon Sep 17 00:00:00 2001
|
||||
From: =?UTF-8?q?Renaud=20M=C3=A9trich?= <rmetrich@redhat.com>
|
||||
Date: Mon, 16 Jan 2023 07:49:44 +0100
|
||||
Subject: [PATCH] Don't loop forever in load_certs() with buggy firmware
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
On DELL R350 booting DVD through RFS with BIOS 1.4.2 in Secure Boot,
|
||||
firmware returns EFI_BUFFER_TOO_SMALL but with new buffersize set to 0,
|
||||
which causes the load_certs() code to loop forever:
|
||||
|
||||
while (1) {
|
||||
efi_status = dir->Read(dir, &buffersize, buffer);
|
||||
if (efi_status == EFI_BUFFER_TOO_SMALL) {
|
||||
...
|
||||
continue;
|
||||
}
|
||||
...
|
||||
}
|
||||
|
||||
This commit prevents such infinite loop.
|
||||
|
||||
Signed-off-by: Renaud Métrich <rmetrich@redhat.com>
|
||||
---
|
||||
shim.c | 16 +++++++++++++---
|
||||
1 file changed, 13 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/shim.c b/shim.c
|
||||
index 4437898..27a8c11 100644
|
||||
--- a/shim.c
|
||||
+++ b/shim.c
|
||||
@@ -1483,11 +1483,21 @@ load_certs(EFI_HANDLE image_handle)
|
||||
}
|
||||
|
||||
while (1) {
|
||||
- int old = buffersize;
|
||||
+ UINTN old = buffersize;
|
||||
efi_status = dir->Read(dir, &buffersize, buffer);
|
||||
if (efi_status == EFI_BUFFER_TOO_SMALL) {
|
||||
- buffer = ReallocatePool(buffer, old, buffersize);
|
||||
- continue;
|
||||
+ if (buffersize != old) {
|
||||
+ buffer = ReallocatePool(buffer, old, buffersize);
|
||||
+ if (buffer == NULL) {
|
||||
+ perror(L"Failed to read directory %s - %r\n",
|
||||
+ PathName, EFI_OUT_OF_RESOURCES);
|
||||
+ goto done;
|
||||
+ }
|
||||
+ continue;
|
||||
+ }
|
||||
+ perror(L"Failed to read directory %s - buggy firmware\n",
|
||||
+ PathName);
|
||||
+ goto done;
|
||||
} else if (EFI_ERROR(efi_status)) {
|
||||
perror(L"Failed to read directory %s - %r\n", PathName,
|
||||
efi_status);
|
||||
--
|
||||
2.27.0
|
||||
|
||||
@ -0,0 +1,67 @@
|
||||
From cf59f3452d478455c5f3d83790b37a372d2837ea Mon Sep 17 00:00:00 2001
|
||||
From: Pete Batard <pete@akeo.ie>
|
||||
Date: Fri, 31 Mar 2023 15:39:54 +0200
|
||||
Subject: [PATCH] Further improve load_certs() for non-compliant
|
||||
drivers/firmwares
|
||||
|
||||
Following the discovery of more problematic firmwares and drivers
|
||||
affected by the issue f23883ccf78f1f605a272f9e5700f47e5494a71d is
|
||||
designed to address (e.g. https://github.com/rhboot/shim/issues/558),
|
||||
this patch further improves the code so that, instead of simply bailing
|
||||
out, we progressively increase the buffer sizes, until either success
|
||||
or a maximum size limit is reached.
|
||||
|
||||
In most cases, this workaround should be enough to ensure completion
|
||||
of the directory read and thus provide full shim functionality (while
|
||||
still warning the user about the non-compliance of their environment).
|
||||
|
||||
Signed-off-by: Pete Batard <pete@akeo.ie>
|
||||
---
|
||||
shim.c | 29 +++++++++++++++++++----------
|
||||
1 file changed, 19 insertions(+), 10 deletions(-)
|
||||
|
||||
diff --git a/shim.c b/shim.c
|
||||
index b3a4fe6..c31e97f 100644
|
||||
--- a/shim.c
|
||||
+++ b/shim.c
|
||||
@@ -1472,18 +1472,27 @@ load_certs(EFI_HANDLE image_handle)
|
||||
UINTN old = buffersize;
|
||||
efi_status = dir->Read(dir, &buffersize, buffer);
|
||||
if (efi_status == EFI_BUFFER_TOO_SMALL) {
|
||||
- if (buffersize != old) {
|
||||
- buffer = ReallocatePool(buffer, old, buffersize);
|
||||
- if (buffer == NULL) {
|
||||
- perror(L"Failed to read directory %s - %r\n",
|
||||
- PathName, EFI_OUT_OF_RESOURCES);
|
||||
+ if (buffersize == old) {
|
||||
+ /*
|
||||
+ * Some UEFI drivers or firmwares are not compliant with
|
||||
+ * the EFI_FILE_PROTOCOL.Read() specs and do not return the
|
||||
+ * required buffer size along with EFI_BUFFER_TOO_SMALL.
|
||||
+ * Work around this by progressively increasing the buffer
|
||||
+ * size, up to a certain point, until the call succeeds.
|
||||
+ */
|
||||
+ perror(L"Error reading directory %s - non-compliant UEFI driver or firmware!\n",
|
||||
+ PathName);
|
||||
+ buffersize = (buffersize < 4) ? 4 : buffersize * 2;
|
||||
+ if (buffersize > 1024)
|
||||
goto done;
|
||||
- }
|
||||
- continue;
|
||||
}
|
||||
- perror(L"Failed to read directory %s - buggy firmware\n",
|
||||
- PathName);
|
||||
- goto done;
|
||||
+ buffer = ReallocatePool(buffer, old, buffersize);
|
||||
+ if (buffer == NULL) {
|
||||
+ perror(L"Failed to read directory %s - %r\n",
|
||||
+ PathName, EFI_OUT_OF_RESOURCES);
|
||||
+ goto done;
|
||||
+ }
|
||||
+ continue;
|
||||
} else if (EFI_ERROR(efi_status)) {
|
||||
perror(L"Failed to read directory %s - %r\n", PathName,
|
||||
efi_status);
|
||||
--
|
||||
2.27.0
|
||||
|
||||
@ -0,0 +1,50 @@
|
||||
From 0bfc3978f4a6a10e4427fdab222b0e50c3c7283c Mon Sep 17 00:00:00 2001
|
||||
From: Alberto Perez <aperezguevar@microsoft.com>
|
||||
Date: Mon, 30 Jan 2023 14:52:15 -0600
|
||||
Subject: [PATCH] Work around malformed path delimiters in file paths from DHCP
|
||||
|
||||
shim uses path delimiters to determine the file path for the second
|
||||
stage. Currently only / (slash) is detected, but some DHCP
|
||||
implementations supply \ (backslash) as the path specifier.
|
||||
|
||||
This patch changes it to accept either delimiter.
|
||||
|
||||
Fixes issue #524.
|
||||
|
||||
Signed-off-by: Alberto Perez <aperezguevar@microsoft.com>
|
||||
---
|
||||
netboot.c | 11 ++++++++++-
|
||||
1 file changed, 10 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/netboot.c b/netboot.c
|
||||
index cf5882c..832deec 100644
|
||||
--- a/netboot.c
|
||||
+++ b/netboot.c
|
||||
@@ -263,7 +263,7 @@ static EFI_STATUS parseDhcp4()
|
||||
UINT8 *dir = pkt_v4->BootpBootFile;
|
||||
|
||||
for (i = dir_len; i >= 0; i--) {
|
||||
- if (dir[i] == '/')
|
||||
+ if ((dir[i] == '/') || (dir[i] == '\\'))
|
||||
break;
|
||||
}
|
||||
dir_len = (i >= 0) ? i + 1 : 0;
|
||||
@@ -277,6 +277,15 @@ static EFI_STATUS parseDhcp4()
|
||||
strncpy(full_path, (CHAR8 *)dir, dir_len);
|
||||
if (full_path[dir_len-1] == '/' && template[0] == '/')
|
||||
full_path[dir_len-1] = '\0';
|
||||
+ /*
|
||||
+ * If the path from DHCP is using backslash instead of slash,
|
||||
+ * accept that and use it in the template in the same position
|
||||
+ * as well.
|
||||
+ */
|
||||
+ if (full_path[dir_len-1] == '\\' && template[0] == '/') {
|
||||
+ full_path[dir_len-1] = '\0';
|
||||
+ template[0] = '\\';
|
||||
+ }
|
||||
}
|
||||
if (dir_len == 0 && dir[0] != '/' && template[0] == '/')
|
||||
template_ofs++;
|
||||
--
|
||||
2.27.0
|
||||
|
||||
46
backport-load_cert_file-Fix-stack-issue.patch
Normal file
46
backport-load_cert_file-Fix-stack-issue.patch
Normal file
@ -0,0 +1,46 @@
|
||||
From 2d4ebb5a798aafd3b06d2c3cb9c9840c1caa41ef Mon Sep 17 00:00:00 2001
|
||||
From: Eric Snowberg <eric.snowberg@oracle.com>
|
||||
Date: Wed, 2 Nov 2022 10:39:43 -0600
|
||||
Subject: [PATCH] load_cert_file: Fix stack issue
|
||||
|
||||
0214cd9cef5a fixes a NULL pointer dereference problem, it introduces two
|
||||
new problems. First it incorrectly assumes li.FilePath is a string.
|
||||
Second, it puts EFI_LOADED_IMAGE li on the stack. It has been found
|
||||
that not all archectures can handle this being on the stack.
|
||||
|
||||
The shim_li variable will be setup properly from the read_image
|
||||
call. Use the global shim_li variable instead when calling
|
||||
verify_image.
|
||||
|
||||
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
|
||||
---
|
||||
shim.c | 6 +-----
|
||||
1 file changed, 1 insertion(+), 5 deletions(-)
|
||||
|
||||
diff --git a/shim.c b/shim.c
|
||||
index 27b74ce..0d919ce 100644
|
||||
--- a/shim.c
|
||||
+++ b/shim.c
|
||||
@@ -1395,7 +1395,6 @@ EFI_STATUS
|
||||
load_cert_file(EFI_HANDLE image_handle, CHAR16 *filename, CHAR16 *PathName)
|
||||
{
|
||||
EFI_STATUS efi_status;
|
||||
- EFI_LOADED_IMAGE li;
|
||||
PE_COFF_LOADER_IMAGE_CONTEXT context;
|
||||
EFI_IMAGE_SECTION_HEADER *Section;
|
||||
EFI_SIGNATURE_LIST *certlist;
|
||||
@@ -1410,10 +1409,7 @@ load_cert_file(EFI_HANDLE image_handle, CHAR16 *filename, CHAR16 *PathName)
|
||||
if (EFI_ERROR(efi_status))
|
||||
return efi_status;
|
||||
|
||||
- memset(&li, 0, sizeof(li));
|
||||
- memcpy(&li.FilePath[0], filename, MIN(StrSize(filename), sizeof(li.FilePath)));
|
||||
-
|
||||
- efi_status = verify_image(data, datasize, &li, &context);
|
||||
+ efi_status = verify_image(data, datasize, shim_li, &context);
|
||||
if (EFI_ERROR(efi_status))
|
||||
return efi_status;
|
||||
|
||||
--
|
||||
2.27.0
|
||||
|
||||
38
backport-mok-remove-MokListTrusted-from-PCR-7.patch
Normal file
38
backport-mok-remove-MokListTrusted-from-PCR-7.patch
Normal file
@ -0,0 +1,38 @@
|
||||
From aa1b289a1a16774afc3143b8948d97261f0872d0 Mon Sep 17 00:00:00 2001
|
||||
From: Arthur Gautier <arthur.gautier@arista.com>
|
||||
Date: Fri, 21 Oct 2022 13:20:45 -0700
|
||||
Subject: [PATCH] mok: remove MokListTrusted from PCR 7
|
||||
|
||||
MokListTrusted was added by mistake to PCR 7 in 4e513405. The value of
|
||||
MokListTrusted does not alter the behavior of secure boot so, as per
|
||||
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05_v23_pub.pdf#page=36
|
||||
(section 3.3.4 PCR usage) so it should not be factored in the value of
|
||||
PCR 7.
|
||||
|
||||
See:
|
||||
https://github.com/rhboot/shim/pull/423
|
||||
https://github.com/rhboot/shim/commit/4e513405b4f1641710115780d19dcec130c5208f
|
||||
|
||||
Fixes https://github.com/rhboot/shim/issues/484
|
||||
Fixes https://github.com/rhboot/shim/issues/492
|
||||
|
||||
Signed-off-by: Arthur Gautier <arthur.gautier@arista.com>
|
||||
---
|
||||
mok.c | 1 -
|
||||
1 file changed, 1 deletion(-)
|
||||
|
||||
diff --git a/mok.c b/mok.c
|
||||
index 63ddfca..9811b35 100644
|
||||
--- a/mok.c
|
||||
+++ b/mok.c
|
||||
@@ -178,7 +178,6 @@ struct mok_state_variable mok_state_variable_data[] = {
|
||||
EFI_VARIABLE_NON_VOLATILE,
|
||||
.no_attr = EFI_VARIABLE_RUNTIME_ACCESS,
|
||||
.flags = MOK_MIRROR_DELETE_FIRST |
|
||||
- MOK_VARIABLE_MEASURE |
|
||||
MOK_VARIABLE_INVERSE |
|
||||
MOK_VARIABLE_LOG,
|
||||
.pcr = 14,
|
||||
--
|
||||
2.27.0
|
||||
|
||||
@ -0,0 +1,36 @@
|
||||
From c7b305152802c8db688605654f75e1195def9fd6 Mon Sep 17 00:00:00 2001
|
||||
From: Nicholas Bishop <nicholasbishop@google.com>
|
||||
Date: Mon, 19 Dec 2022 18:56:13 -0500
|
||||
Subject: [PATCH] pe: Align section size up to page size for mem attrs
|
||||
|
||||
Setting memory attributes is generally done at page granularity, and
|
||||
this is enforced by checks in `get_mem_attrs` and
|
||||
`update_mem_attrs`. But unlike the section address, the section size
|
||||
isn't necessarily aligned to 4KiB. Round up the section size to fix
|
||||
this.
|
||||
|
||||
Signed-off-by: Nicholas Bishop <nicholasbishop@google.com>
|
||||
---
|
||||
pe.c | 6 +++++-
|
||||
1 file changed, 5 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/pe.c b/pe.c
|
||||
index 9a3679e..5ad0914 100644
|
||||
--- a/pe.c
|
||||
+++ b/pe.c
|
||||
@@ -1372,7 +1372,11 @@ handle_image (void *data, unsigned int datasize,
|
||||
+ Section->Misc.VirtualSize - 1);
|
||||
|
||||
addr = (uintptr_t)base;
|
||||
- length = (uintptr_t)end - (uintptr_t)base + 1;
|
||||
+ // Align the length up to PAGE_SIZE. This is required because
|
||||
+ // platforms generally set memory attributes at page
|
||||
+ // granularity, but the section length (unlike the section
|
||||
+ // address) is not required to be aligned.
|
||||
+ length = ALIGN_VALUE((uintptr_t)end - (uintptr_t)base + 1, PAGE_SIZE);
|
||||
|
||||
if (Section->Characteristics & EFI_IMAGE_SCN_MEM_WRITE) {
|
||||
set_attrs |= MEM_ATTR_W;
|
||||
--
|
||||
2.27.0
|
||||
|
||||
36
backport-pe-Fix-image-section-entry-point-validation.patch
Normal file
36
backport-pe-Fix-image-section-entry-point-validation.patch
Normal file
@ -0,0 +1,36 @@
|
||||
From 17f02339ed1be9e90738603fe3c95ae7dc300061 Mon Sep 17 00:00:00 2001
|
||||
From: Ilya Okomin <ilya.okomin@oracle.com>
|
||||
Date: Fri, 7 Oct 2022 16:52:08 -0400
|
||||
Subject: [PATCH] pe: Fix image section entry-point validation
|
||||
|
||||
Seen mokmanager image load failure '2 sections contain entry point'
|
||||
for shim built on Oracle Linux 9 aarch64. found_entry_point counter in
|
||||
handle_image() uses SizeOfRawData to calculate section boundary.
|
||||
PE spec defines VirtualSize for the total size of the section when loaded
|
||||
into memory. SizeOfRawData is the size of the section (for object files)
|
||||
or the size of the initialized data on disk.
|
||||
|
||||
Fix this issue by updating section in-memory size limit to VirtualSize.
|
||||
|
||||
Resolves: #517
|
||||
Signed-off-by: Ilya Okomin <ilya.okomin@oracle.com>
|
||||
---
|
||||
pe.c | 2 +-
|
||||
1 file changed, 1 insertion(+), 1 deletion(-)
|
||||
|
||||
diff --git a/pe.c b/pe.c
|
||||
index f94530a..9a3679e 100644
|
||||
--- a/pe.c
|
||||
+++ b/pe.c
|
||||
@@ -1259,7 +1259,7 @@ handle_image (void *data, unsigned int datasize,
|
||||
}
|
||||
|
||||
if (Section->VirtualAddress <= context.EntryPoint &&
|
||||
- (Section->VirtualAddress + Section->SizeOfRawData - 1)
|
||||
+ (Section->VirtualAddress + Section->Misc.VirtualSize - 1)
|
||||
> context.EntryPoint)
|
||||
found_entry_point++;
|
||||
|
||||
--
|
||||
2.27.0
|
||||
|
||||
@ -0,0 +1,55 @@
|
||||
From a8b0b600ddcf02605da8582b4eac1932a3bb13fa Mon Sep 17 00:00:00 2001
|
||||
From: Mike Beaton <mjsbeaton@gmail.com>
|
||||
Date: Mon, 10 Apr 2023 07:25:51 +0000
|
||||
Subject: [PATCH] pe: only process RelocDir->Size of reloc section
|
||||
|
||||
Previously processing full padding-aligned Section->Misc.VirtualSize
|
||||
relied on padding reloc entries being inserted by GenFw, which is
|
||||
not required by spec.
|
||||
|
||||
This changes it to only process the amount referenced by Size, rather
|
||||
than VirtualSize which may be bigger than the data present.
|
||||
|
||||
Signed-off-by: Mike Beaton <mjsbeaton@gmail.com>
|
||||
---
|
||||
pe.c | 9 ++++++---
|
||||
1 file changed, 6 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/pe.c b/pe.c
|
||||
index 85b64c0..18f3e8f 100644
|
||||
--- a/pe.c
|
||||
+++ b/pe.c
|
||||
@@ -87,7 +87,7 @@ relocate_coff (PE_COFF_LOADER_IMAGE_CONTEXT *context,
|
||||
/* RelocBaseEnd here is the address of the first entry /past/ the
|
||||
* table. */
|
||||
RelocBaseEnd = ImageAddress(orig, size, Section->PointerToRawData +
|
||||
- Section->Misc.VirtualSize);
|
||||
+ context->RelocDir->Size);
|
||||
|
||||
if (!RelocBase && !RelocBaseEnd)
|
||||
return EFI_SUCCESS;
|
||||
@@ -741,7 +741,7 @@ read_header(void *data, unsigned int datasize,
|
||||
context->NumberOfSections = PEHdr->Pe32.FileHeader.NumberOfSections;
|
||||
|
||||
if (EFI_IMAGE_NUMBER_OF_DIRECTORY_ENTRIES < context->NumberOfRvaAndSizes) {
|
||||
- perror(L"Image header too small\n");
|
||||
+ perror(L"Image header too large\n");
|
||||
return EFI_UNSUPPORTED;
|
||||
}
|
||||
|
||||
@@ -1277,8 +1277,11 @@ handle_image (void *data, unsigned int datasize,
|
||||
Section->Misc.VirtualSize &&
|
||||
base && end &&
|
||||
RelocBase == base &&
|
||||
- RelocBaseEnd == end) {
|
||||
+ RelocBaseEnd <= end) {
|
||||
RelocSection = Section;
|
||||
+ } else {
|
||||
+ perror(L"Relocation section is invalid \n");
|
||||
+ return EFI_UNSUPPORTED;
|
||||
}
|
||||
}
|
||||
|
||||
--
|
||||
2.27.0
|
||||
|
||||
@ -0,0 +1,56 @@
|
||||
From 5c537b3d0cf8c393dad2e61d49aade68f3af1401 Mon Sep 17 00:00:00 2001
|
||||
From: dann frazier <dann.frazier@canonical.com>
|
||||
Date: Tue, 6 Sep 2022 09:28:22 -0600
|
||||
Subject: [PATCH] shim: Flush the memory region from i-cache before execution
|
||||
|
||||
We've seen crashes in early GRUB code on an ARM Cortex-A72-based
|
||||
platform that point at seemingly harmless instructions. Flushing
|
||||
the i-cache of those instructions prior to executing has been
|
||||
shown to avoid the problem, which has parallels with this story:
|
||||
https://www.mail-archive.com/osv-dev@googlegroups.com/msg06203.html
|
||||
|
||||
Add a cache flushing utility function and provide an implementation
|
||||
using a GCC intrinsic. This will need to be extended to support other
|
||||
compilers. Note that this intrinsic is a no-op for x86 platforms.
|
||||
|
||||
This fixes issue #498.
|
||||
|
||||
Signed-off-by: dann frazier <dann.frazier@canonical.com>
|
||||
---
|
||||
include/compiler.h | 6 ++++++
|
||||
pe.c | 3 +++
|
||||
2 files changed, 9 insertions(+)
|
||||
|
||||
diff --git a/include/compiler.h b/include/compiler.h
|
||||
index b4bf103..b0d595f 100644
|
||||
--- a/include/compiler.h
|
||||
+++ b/include/compiler.h
|
||||
@@ -192,5 +192,11 @@
|
||||
*/
|
||||
#define unreachable() __builtin_unreachable()
|
||||
|
||||
+#if defined(__GNUC__)
|
||||
+#define cache_invalidate(begin, end) __builtin___clear_cache(begin, end)
|
||||
+#else /* __GNUC__ */
|
||||
+#error shim has no cache_invalidate() implementation for this compiler
|
||||
+#endif /* __GNUC__ */
|
||||
+
|
||||
#endif /* !COMPILER_H_ */
|
||||
// vim:fenc=utf-8:tw=75:et
|
||||
diff --git a/pe.c b/pe.c
|
||||
index ba3e2bb..f94530a 100644
|
||||
--- a/pe.c
|
||||
+++ b/pe.c
|
||||
@@ -1196,6 +1196,9 @@ handle_image (void *data, unsigned int datasize,
|
||||
|
||||
CopyMem(buffer, data, context.SizeOfHeaders);
|
||||
|
||||
+ /* Flush the instruction cache for the region holding the image */
|
||||
+ cache_invalidate(buffer, buffer + context.ImageSize);
|
||||
+
|
||||
*entry_point = ImageAddress(buffer, context.ImageSize, context.EntryPoint);
|
||||
if (!*entry_point) {
|
||||
perror(L"Entry point is invalid\n");
|
||||
--
|
||||
2.27.0
|
||||
|
||||
18
shim.spec
18
shim.spec
@ -25,7 +25,7 @@
|
||||
|
||||
Name: shim
|
||||
Version: 15.6
|
||||
Release: 14
|
||||
Release: 15
|
||||
Summary: First-stage UEFI bootloader
|
||||
ExclusiveArch: x86_64 aarch64
|
||||
License: BSD
|
||||
@ -58,6 +58,19 @@ Patch20:backport-CVE-2021-23840.patch
|
||||
Patch21:backport-CVE-2023-0464.patch
|
||||
Patch22:backport-CVE-2023-3817.patch
|
||||
Patch23:backport-CVE-2023-40546.patch
|
||||
Patch24:backport-CryptoPkg-BaseCryptLib-Fix-buffer-overflow-issue-in-.patch
|
||||
Patch25:backport-shim-Flush-the-memory-region-from-i-cache-before-exe.patch
|
||||
Patch26:backport-pe-Align-section-size-up-to-page-size-for-mem-attrs.patch
|
||||
Patch27:backport-load_cert_file-Fix-stack-issue.patch
|
||||
Patch28:backport-mok-remove-MokListTrusted-from-PCR-7.patch
|
||||
Patch29:backport-Don-t-loop-forever-in-load_certs-with-buggy-firmware.patch
|
||||
Patch30:backport-CryptoPkg-BaseCryptLib-fix-NULL-dereference.patch
|
||||
Patch31:backport-Discard-load-options-that-start-with-a-NUL.patch
|
||||
Patch32:backport-pe-Fix-image-section-entry-point-validation.patch
|
||||
Patch33:backport-Further-improve-load_certs-for-non-compliant-drivers.patch
|
||||
Patch34:backport-Work-around-malformed-path-delimiters-in-file-paths-.patch
|
||||
Patch35:backport-pe-only-process-RelocDir-Size-of-reloc-section.patch
|
||||
Patch36:backport-Correctly-free-memory-allocated-in-handle_image.patch
|
||||
|
||||
# Feature for shim SMx support
|
||||
Patch9000:Feature-shim-openssl-add-ec-support.patch
|
||||
@ -192,6 +205,9 @@ make test
|
||||
/usr/src/debug/%{name}-%{version}-%{release}/*
|
||||
|
||||
%changelog
|
||||
* Thu Dec 7 2023 huangzq6 <huangzhenqiang2@huawei.com> - 15.6-15
|
||||
- backport patches form upstream
|
||||
|
||||
* Thu Nov 16 2023 huangzq6 <huangzhenqiang2@huawei.com> - 15.6-14
|
||||
- add signature for secureboot
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user