| Age | Commit message (Collapse) | Author |
|---|
|
The MOK variable mirroring makes use of variable_create_esl, which
can only create a well-formed EFI_SIGNATURE_LIST containing a single
signature. Fix fill_esl and variable_create_esl to support creating
a EFI_SIGNATURE_LIST with one or more supplied EFI_SIGNATURE_DATA
structures.
Introduce variable_create_esl_with_one_signature and
fill_esl_with_one_signature for code that does want to create a
EFI_SIGNATURE_LIST containing a single signature constructed from
a supplied signature data buffer and owner GUID.
|
|
If the LoadOptions string count is zero, then it's assumed that it is an
EFI_LOAD_OPTION and the OptionalData field attempt to be parsed. If that
fails as well, in the second stage was set to the default loader path.
But this behaviour was changed by the commit 018b74d2 ("shim: attempt to
improve the argument handling"), and not in that case the LoadOptions is
attempted to be used as a single string. This breaks some firmwares that
return something in the LoadOptions but are not a proper EFI device path.
Instead of making assumptions about the LoadOptions if can't be parsed
correctly, just use the default loader as it was done before that commit.
This fixes booting on a Gigabyte GA-Z97X-SLI mainboard that contains the
following bytes as LoadOptions: 0x41 0x4d 0x42 0x4f ('AMBO').
Reported-by: Thomas Frauendorfer | Miray Software <tf@miray.de>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
|
|
Similar to x86_64, the .sbat section is aligned to 4096, so we should
include the aligned part in SizeOfRawData as objcopy does for x86_64.
For VirtualSize, _sbat_vsize is used to reflect the actually size of
sbat.
This also fixes a strange hash mismatching in openSUSE build service
when attaching signature to AArch64 EFI images from shim package.
Signed-off-by: Gary Lin <glin@suse.com>
|
|
The order in which the foreach() returns files differes from
Debian on WSL1 and Debian running natively.
When shim is build on these two platforms the resulting binaries differ.
This patch manually sorts the input file list to create identical binaries.
Signed-off-by: Thomas Frauendorfer | Miray Software <tf@miray.de>
|
|
If the file Make.local exists, use it as a source of local build
configuration by including it in Make.defaults.
(cherry picked from commit 57e38a1ebf73 in the shim-15.2 branch)
Signed-off-by: Paul Moore <pmoore2@cisco.com>
|
|
This fixes the SizeOfImage and SizeOfInitializedData headers on arm and
aa64.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Don't check SHIM_UNIT_TEST.
This fixes conflicting declarations for __builtin_ms_va_list on amd64:
In file included from shim.h:47,
from test.c:10:
../include/system/stdarg.h:30:27: error: conflicting types for '__builtin_ms_va_list'
typedef __builtin_va_list __builtin_ms_va_list;
^~~~~~~~~~~~~~~~~~~~
cc1: note: previous declaration of '__builtin_ms_va_list' was here
In file included from shim.h:47,
from test-csv.c:9:
../include/system/stdarg.h:30:27: error: conflicting types for '__builtin_ms_va_list'
typedef __builtin_va_list __builtin_ms_va_list;
^~~~~~~~~~~~~~~~~~~~
cc1: note: previous declaration of '__builtin_ms_va_list' was here
In file included from shim.h:47,
from csv.c:6:
../include/system/stdarg.h:30:27: error: conflicting types for '__builtin_ms_va_list'
typedef __builtin_va_list __builtin_ms_va_list;
^~~~~~~~~~~~~~~~~~~~
cc1: note: previous declaration of '__builtin_ms_va_list' was here
Signed-off-by: Steve McIntyre <93sam@debian.org>
|
|
We need to be using our patched version of gnu-efi
Signed-off-by: Steve McIntyre <93sam@debian.org>
|
|
Steve McIntyre reports:
<Sledge> yay, arm64 string test fail
<Sledge> testing gnuefi_signed_strncmp
<Sledge> test_strncmp:713:got 128, expected < 0
<Sledge> test_strncmp:713:Assertion `(gnuefi_signed_strncmp("sbat\314\234\014,", "sbat\314\034\014,", 9)) >= 0' failed.
<Sledge> gnuefi_signed_strncmp failed
<Sledge> looking at that code, this is a test to check how broken the gnuefi strncmp is, yes?
<Sledge> and we're not actually using this implementation in shim AFAICS?
That is a correct understanding, and as such this patch just removes
that test from running on Arm platforms, where it is still too broken to
even do this much.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Commit 018b74d2d69 ("shim: attempt to improve the argument handling") added
added workarounds for a couple of LoadOption problems on some systems, but
introduced a regression since the is_our_path() function can be called with
a NULL start UCS-2 string.
If there's only one string, set start to the start of LoadOptions.
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
scan-build kindly pointed out:
| shim.c:1568:10: warning: Array access (from variable 'start') results in a null pointer dereference [core.NullDereference]
| while (start[loader_len++] != L'\0');
| ^~~~~~~~~~~~~~~~~~~
| 1 warning generated.
It thinks that because of a bad assumption it's making because of the
test immediately before it, which isn't currently necessary /at all/.
In fact, neither is this loop; it appears to be vestigial and the goal
was done in the loop above it.
This patch just solves for how much space is left arithmetically
instead.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Fix a couple of small off-by-one errors in the SBAT variable initial
setup and validation path.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This makes it so we can tell what it's actually doing and why.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This makes it so that if you build with ENABLE_SHIM_DEVEL, the SBAT we
use is named SBAT_DEVEL instead of SBAT, and it's expected to have
EFI_VARIABLE_RUNTIME_ACCESS set.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This fixes ENABLE_SHIM_DEVEL to actually work, and also makes our "goto
die" failure behavior change (to wait considerably longer) based on it.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
For some reason when we try to ever use the builtins, even with the
symbol there as a fallback, something goes horribly wrong somewhere
around here:
| (gdb) bt
| #0 strcmp (s1=0x7d492359 "MD5", s2=0x7d492359 "MD5") at include/system/string.h:57
| #1 0x000000007d460419 in getrn (lh=lh@entry=0x7e081318, data=data@entry=0x7e084398, rhash=rhash@entry=0x7f7c9268) at crypto/lhash/lhash.c:415
| #2 0x000000007d46076e in lh_insert (lh=0x7e081318, data=data@entry=0x7e084398) at crypto/lhash/lhash.c:188
| #3 0x000000007d43e027 in OBJ_NAME_add (name=name@entry=0x7d492359 "MD5", type=type@entry=1, data=data@entry=0x7d4ad3a0 <md5_md> "\004") at crypto/objects/o_names.c:202
As much as I love a Sisyphean challenge, in the interest of not having
bugs or time, this patch changes it to just not use them for anything
other than guaranteeing our implementations have the exact same types as
you would expect.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Be much more explicit about exactly which va_* stuff comes from which
ABI in both shim and gnu-efi. This fixes the problem where we see:
| (null):0:(null)() v->name:"(null)" v->rtname:"(null)"
| (null):0:(null)() v->data_size:0 v->data:0x0
and similar messages where everything is NULL.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
scan-build helpfully notes:
| In file included from shim.c:14:
| In file included from /home/pjones/devel/github.com/shim/sbat-aarch64/shim.h:183:
| /home/pjones/devel/github.com/shim/sbat-aarch64/include/hexdump.h:123:2: error: 'va_start' used in Win64 ABI function
| va_start(ap, at);
| ^
| /usr/lib/gcc/x86_64-redhat-linux/10/include/stdarg.h:47:23: note: expanded from macro 'va_start'
| #define va_start(v,l) __builtin_va_start(v,l)
| ^
This is because one of the patches for the builtin swizzling is missing
a correction for the include order. This patch fixes that order.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
scan-build believes we can hit a situation where get_variable_attr() is
called with NULL data, in which case we're not correctly returning an
error.
This adds the error return.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Our section headers on arm binaries need to include .sbat on fallback
and MokManger, and currently they do not.
The reason for this is that gnu-efi provides static, (mostly) hand-coded
section headers on arm and aarch64, due to having no efi-app-arm and
efi-app-aa64 target support in binutils. Additionally, the assembler
also generates (IMO pointless) relocations for _esbat/_sbat_size when
those are actually inside the section, and relocated symbols can't be
used in our section headers.
This patch moves the .sbat section to be after _edata, so the sections
don't overlap, and moves _esbat and _sbat_size to be after the section,
to avoid the relocation.
I'm not 100% sure we can't have overlapping sections, but now doesn't
seem like the time to find out.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
There's no reason to do the work to set an initial SBAT variable twice,
or to do it /after/ the self check.
This changes it to do it once, before the self check, and then only
raise an error if we're in secure mode.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
A few cleanups:
- Ensure that the data we get from get_variable() is at least big enough
to actually be an SBAT variable
- Only try to delete if the variable is actually set
- Don't set the variable again if deleting it failed
- We don't actually need to get the size of the variable, allocate,
and then get the variable; get_variable() does the allocation for us.
- Don't compare the variable data when get_variable() failed
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
|
|
"So, load options are a giant pain in the ass." - Peter Jones
This patch attempts to workaround two LoadOption problems seen on
my test system: arguments separated with only whitespace (L" ") and
strings that aren't properly NULL terminated.
In addition to my test system, it appears at least one other person
has run into a similar problem and I can reproduce the whitespace
delimiter issue with QEMU+OVMF (OEMU v5.2.0, OVMF v202011).
* https://github.com/rhboot/shim/issues/181
For reference, using QEMU+OVMF and a simple test application,
"test.efi", which does a hexdump of LoadOptions I see the following
when run via the UEFI shell:
FS0:\> test.efi one two three
LoadOptions:
0000: 74 00 65 00 73 00 74 00 2E 00 65 00 66 00 69 00 t.e.s.t...e.f.i.
0016: 20 00 6F 00 6E 00 65 00 20 00 74 00 77 00 6F 00 ..o.n.e...t.w.o.
0032: 20 00 74 00 68 00 72 00 65 00 65 00 00 00 ..t.h.r.e.e...
Signed-off-by: Paul Moore <pmoore2@cisco.com>
|
|
Signed-off-by: Alex Burmashev <alexander.burmashev@oracle.com>
|
|
If FALLBACK_VERBOSE is defined at build time the resulting fallback will
always be verbose despite having the EFI variable defined or not, which
facilitates testing in some scenarios.
Signed-off-by: João Paulo Rechi Vita <jprvita@endlessos.org>
|
|
Make it possible to configure at build time for how long fallback will
wait before moving to the next step when in verbose mode.
Also remind the user they can press the Pause key to pause the boot
process at that point.
Signed-off-by: João Paulo Rechi Vita <jprvita@endlessos.org>
|
|
Currently we wait half a second before resetting the system when running
fallback in verbose mode. Lets wait the same amount of time before
trying to chain-load the first boot entry as well, so we have a chance
to see what is on screen.
Signed-off-by: João Paulo Rechi Vita <jprvita@endlessos.org>
|
|
This helps to identify when the firmware messes up the boot entries
created by fallback.
Signed-off-by: João Paulo Rechi Vita <jprvita@endlessos.org>
|
|
Only print what fallback is doing when running in verbose mode. This way
we can have a silent boot even when fallback is doing its thing.
This commit is based on a previous patch by Carlo Caione.
Signed-off-by: João Paulo Rechi Vita <jprvita@endlessos.org>
|
|
This removes the last use of '#ifdef DEBUG_FALLBACK".
Signed-off-by: João Paulo Rechi Vita <jprvita@endlessos.org>
|
|
Some firmware implementations like the one on the Acer TravelMate
P449-G2-MG completely ignore the value of BootOrder set by fallback, and
overwrite it with a value of its own. On this particular machine, the
boot entry created by fallback on the previous boot is not included by
the firmware on this new BootOrder, so it is not considered when
checking for duplicates.
This problem is agravated by the fact that such firmware does not give
the user the possibility to manually boot from any entry created outside
of the firmware setup program -- the only way to boot a distro that
deploys "the fallback ESP layout" and no \EFI\BOOT\grubx64.efi with this
firmware is through \EFI\BOOT\BOOTX64.EFI. The side effect here is
having a new boot entry created by fallback on every boot.
This commit makes fallback try every Boot* variable when checking for
duplicates, not only the ones listed in BootOrder, so it can find the
duplicate Boot entry and re-use it instead of creating a new one.
https://phabricator.endlessm.com/T15481
Signed-off-by: João Paulo Rechi Vita <jprvita@endlessos.org>
|
|
Signed-off-by: João Paulo Rechi Vita <jprvita@endlessos.org>
|
|
|
|
|
|
This adds test cases for the rest of our ASCII string functions. While
doing so, it fixes two minor bugs:
- strcasecmp() now handles utf8 correctly
- strncpy() no longer does the stpncpy() behavior of clearing leftover
buffer
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
gcc -Wextra, has a lot of good, useful checks, a few obnoxious checks,
and a few absolutely insane checks.
This enables -Wextra, but disables -Wmissing-field-initializers, because
it is irrational nonsense that just leads to worse code. It also
disables some specific things in the Cryptlib and Cryptlib/OpenSSL
trees:
Both:
-Wno-unused-parameter
- there are a fair number of functions that have to conform to some API
or another but have arguments that are unused, but haven't been
marked with UNUSED; we don't need to see warnings about them.
Cryptlib/OpenSSL:
-Wno-empty-body
- functions that exist merely to populate some API
-Wno-implicit-fallthrough
- these probably should get fixed someday, but I bet upstream will do
it and rebasing will solve it
-Wno-old-style-declaration
- this gripes if you write "const static" instead of "static const".
Again I expect rebasing will fix it at some point.
-Wno-unused-but-set-variable
- self explanatory, and again, I expect a rebase to solve it someday.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
There were a couple cases of "uninitialized variable" warnings in the
imported OpenSSL code; I used the current OpenSSL code as a guide for
picking the default values used here.
On my dev system there is one remaining build warning in OpenSSL's
crypto/asn1/x_pkey.c:X509_PKEY_new() function. Unfortunately it
involves some preprocessor crimes and the fix would be a bit ugly.
Fortunately it appears the warning here is harmless and can be
ignored.
As a point of reference, my build system is a current Arch install
with GCC v10.2.0 and GNU-EFI v 3.0.12.
Signed-off-by: Paul Moore <pmoore2@cisco.com>
|
|
Note the one test case I'm not 100% sure about. Someone let me know.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This gets us the same working definition for VA_* va_* etc everywhere,
and it's the same definition edk2 is using.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Some time ago, commit e571428e212 ("Update to openssl to 1.0.2e")
changed the way we define the va_* (and VA_*) functions and macros.
Unfortunately, it only changed for some parts of the tree, and the
different parts of the tree need to both call each other and use the
same types in all cases. Additionally, they need to all be able to call
gnu-efi functions such as VPrint, which means they need the same va_list
type definitions everywhere.
This partially reverts that patch, adding EFIAPI back and unsetting
NO_BUILTIN_VA_FUNCS everywhere.
|
|
This re-structures our includes so we can be sure everything is always
including all the system headers in a uniform, predictable way.
Temporarily it also adds a bunch of junk at all the places we use
variadic functions to specifically pick either the MS (cdecl) or ELF
ABIs.
I'm not 100% sure that's all correct (see later patch) but it's enough
to allow this to build.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
|
|
Shim is rather more friendly with EFI internals than most code, and as a
result can end up making assumptions that are out of step with those made
by gnu-efi. Since both projects are developed independently, and since
distributions are often trying to build versions of shim against whatever
version of gnu-efi they are shipping, this can result in awkward build
failures. The easiest way to handle this is to use a git submodule and
import a known-good version of shim directly into the build tree. Given
static linking, this will also make reproducible builds easier.
Plus some changes from pjones:
- Fix up some more include paths
- more fine grained clean rules
- use our make ARCH
- use an rhboot/ repo for the gnu-efi remote
Signed-off-by: Matthew Garrett <mjg59@google.com>
|
