| Age | Commit message (Collapse) | Author |
|---|
|
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>
|
|
Note that there are still some occurrences of WCHAR in Cryptlib/OpenSSL/,
but they're only built on win32 platforms we don't support.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
There's no actual reason we're using -std=gnu89, but it means we get the
"gnu89-inline" semantics, which we would prefer to have to specify
manually when we want it, if ever, which so far we don't.
This also allows us to use some saner syntax without having to nerf
various -W options and similar later, and enables some language features
that are pretty useful, but that's just icing.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This patch does some makefile cleanups, to fix the parts that are
actually just bad that the previous patch left in for clarity:
- removes -fno-builtin . This flag is implied by -ffreestanding , which
we use everywhere.
- gets rid of the two places where ARM has their own -O flags for no
real reason. Note that this will make those use -Os instead of -O2.
- export VERBOSE and DEBUG if they're set.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This improves our static analysis targets by making them work better
with our make variables, and inhibits the use of ccache while building
those.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
These are all the NULL pointer dereferences (which all appear to be, at
worst, very difficult to hit) that gcc -fanalyzer finds in our OpenSSL
code.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Some of our makefile bits are a mess, as you may have noticed, making
changes to them difficult to review.
This patch attempts to make some parts of them vaguely less of a mess,
in order to facilitate review of follow-up changes. To so it:
- coalesces feature flags, optimizations, -W{no-,}, -W{no-}error,
include directives, and define/undefine directives into (mostly)
separate groups.
- exports them as appropriate so the sub-makes can use them
- Makes sure we have -Wextra -Werror everywhere, but adds -Wno-foo and
-Wno-error=foo directives at the appropriate places to keep the net
warnings the same.
- makes the arch defines in Cryptlib and Cryptlib/OpenSSL use the
appropriate ones, with no attempt to make them less stupid, without
changing the overall order.
- coalesces the various includes, with no attempt to make them less
stupid, without changing the overall order.
- One giant glaring whitespace fix in Cryptlib/OpenSSL/Makefile
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
We've been using "make -s", silent mode, which doesn't show us what
commands were invoked. Silent make means we can't see what goes wrong
when the wrong compiler or linker is getting invoked, or when it's being
invoked wrong.
This changes the CI to run it again without -s when it fails.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
When using "objcopy -O binary", it silently drops the sections without
"alloc" or "load" or the sections with "unload". Since we didn't set any
section flags for .sbat, it just contains the "readonly" flag and
objcopy ignored the section totally when generating EFI images for
ARM32 and AArch64.
This commit sets the common read-only data section flags to .sbat so
that objcopy would always copy the section to the final EFI image.
Signed-off-by: Gary Lin <glin@suse.com>
|
|
This get_variable_size() implementation success in either of two cases:
- EFI_SUCCESS with *lenp == 0 if the variable isn't found
- EFI_SUCCESS with *lenp > 0 on success
In the event of other errors, it returns them to you.
There's nothing particularly interesting about the set_variable()
or del_variable() implementation here.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
When grub2 invoked Exit() in AArch64 AAVMF, the VM crashed with the
following messsages:
Unloading driver at 0x000B7D7B000
Synchronous Exception at 0x00000000BF5D5E68
AllocatePool: failed to allocate 800 bytes
Synchronous Exception at 0x00000000BF5D5E68
The similar error also showed when I modified MokManager to call
gBS->Exit() at the end of efi_main(). However, if MokManager just
returned, the error never showed. One significant difference is
whether the loaded image was restored or not, and the firmware seems
to need the original ImageBase pointer to do clean-up.
To avoid the potential crash, this commit adds restore_loaded_image() so
that we can restore the loaded image both in start_image() and
do_exit().
Signed-off-by: Gary Lin <glin@suse.com>
|
|
Fix a typo that inverted the check that validates that a CSV
column is not empty.
Signed-off-by: Thomas Frauendorfer | Miray Software <tf@miray.de>
|
|
The current code rejects the .sbat section if the VirtualSize and
SizeOfRawData fields of the section header aren't the same, but this
is too strict. The VirtualSize corresponds to the size of the SBAT
metadata and isn't necessarily a multiple of the file alignment.
The on-disk size (SizeOfRawData) is a multiple of the file alignment
and is zero padded. Make sure that the on-disk size is at least as
large as virtual size, and ignore the .sbat section if it isn't.
This should fix https://github.com/rhboot/shim/issues/281.
Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
|
|
The raw text of the SBAT.md file is a bit of a mess, this patch
attempts to introduce some consistentcy to the text without affecting
the rendered Markdown.
The only content change was the addition of a missing period ('.') at
the end of sentence/paragraph; all of the other changes were purely
formatting changes.
Signed-off-by: Paul Moore <pmoore2@cisco.com>
|
|
Signed-off-by: Paul Moore <pmoore2@cisco.com>
|
|
Signed-off-by: Gary Lin <glin@suse.com>
|
|
Two places we missed still have 0 for an sbat version - one doc and one
in our data csv.
This fixes those.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This commit fixes the following gcc warnings for casting.
sbat.c: In function 'verify_single_entry':
sbat.c:157:13: error: pointer targets in passing argument 1 of 'strcmp' differ in signedness [-Werror=pointer-sign]
if (strcmp(entry->component_name, sbat_var_entry->component_name) == 0) {
^~~~~
In file included from Cryptlib/Include/string.h:15:0,
from sbat.c:7:
Cryptlib/Include/OpenSslSupport.h:299:16: note: expected 'const char *' but argument is of type 'const CHAR8 * {aka const unsigned char *}'
int strcmp (const char *, const char *);
^~~~~~
sbat.c:157:36: error: pointer targets in passing argument 2 of 'strcmp' differ in signedness [-Werror=pointer-sign]
if (strcmp(entry->component_name, sbat_var_entry->component_name) == 0) {
^~~~~~~~~~~~~~
In file included from Cryptlib/Include/string.h:15:0,
from sbat.c:7:
Cryptlib/Include/OpenSslSupport.h:299:16: note: expected 'const char *' but argument is of type 'const CHAR8 * {aka const unsigned char *}'
int strcmp (const char *, const char *);
^~~~~~
sbat.c:165:19: error: pointer targets in passing argument 1 of 'AsciiStrDecimalToUintn' differ in signedness [-Werror=pointer-sign]
sbat_gen = atoi(entry->component_generation);
^
Cryptlib/Include/OpenSslSupport.h:380:66: note: in definition of macro 'atoi'
#define atoi(nptr) AsciiStrDecimalToUintn(nptr)
^~~~
In file included from Cryptlib/Include/OpenSslSupport.h:21:0,
from Cryptlib/Include/string.h:15,
from sbat.c:7:
Cryptlib/Library/BaseLib.h:9:7: note: expected 'const char *' but argument is of type 'const CHAR8 * {aka const unsigned char *}'
UINTN AsciiStrDecimalToUintn(const char *String);
^~~~~~~~~~~~~~~~~~~~~~
In file included from Cryptlib/Include/string.h:15:0,
from sbat.c:7:
sbat.c:166:23: error: pointer targets in passing argument 1 of 'AsciiStrDecimalToUintn' differ in signedness [-Werror=pointer-sign]
sbat_var_gen = atoi(sbat_var_entry->component_generation);
^
Cryptlib/Include/OpenSslSupport.h:380:66: note: in definition of macro 'atoi'
#define atoi(nptr) AsciiStrDecimalToUintn(nptr)
^~~~
In file included from Cryptlib/Include/OpenSslSupport.h:21:0,
from Cryptlib/Include/string.h:15,
from sbat.c:7:
Cryptlib/Library/BaseLib.h:9:7: note: expected 'const char *' but argument is of type 'const CHAR8 * {aka const unsigned char *}'
UINTN AsciiStrDecimalToUintn(const char *String);
^~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [<builtin>: sbat.o] Error 1
Signed-off-by: Gary Lin <glin@suse.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This brings all the tests Chris Co wrote about parsing the .sbat section
back. Some of the actual test functions became redundant, and some new
ones were needed, but all of the actual test cases should be represented
here.
Note that building and running this test does not quite work yet /on
this branch/. In order to do that, we need some cleanups and
reorganizing that I don't want to push just yet, which can be found on
https://github.com/rhboot/shim/tree/test-reorg
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
[currently #if 0'd to oblivion by pjones]
Signed-off-by: Chris Co <chrco@microsoft.com>
|
|
This does a couple of straightforward tests on our CSV parser, and then
for good measure it does two with random data - one that's just random
data, one that's had all the zeros changed to nonzero values.
Note that building and running this test does not quite work yet /on
this branch/. In order to do that, we need some cleanups and
reorganizing that I don't want to push just yet, which can be found on
https://github.com/rhboot/shim/tree/test-reorg
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Note that building and running this test does not quite work yet /on
this branch/. In order to do that, we need some cleanups and
reorganizing that I don't want to push just yet, which can be found on
https://github.com/rhboot/shim/tree/test-reorg
Signed-off-by: Peter Jones <pjones@redhat.com>
|
