After doing some validation with clang from versions 3.8 and up, the
builds prior to version 8.0.0 fail due to the use of safemath functions
at link time.
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Building the current code with clang and the latest gnulib fails due to
the use of a variable-length-array (vla) warning, which turns in to an
error due to the presence of the -Werror during the build.
The gnulib team stated that their code should not be built with -Werror.
At present, the only way to do this is for the complete code-base, by
using the --disable-werror option to configure.
Rather than doing this, and failing to gain any benefit that it provides,
instead, if building with clang, this patch makes it possible to specifically
not error on vlas, while retaining the -Werror functionality otherwise.
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The recent gnulib updates require an implementation of abort(), but the
current macro provided by changeset:
cd37d3d3916c gnulib: Drop no-abort.patch
to config.h.in does not work with the clang compiler since it doesn't
provide a __builtin_trap() implementation, so this element of the
changeset needs to be reverted, and replaced.
After some discussion with Vladimir 'phcoder' Serbinenko and Daniel Kiper
it was suggested to bring back in the change from the changeset:
db7337a3d353 * grub-core/gnulib/regcomp.c (regerror): ...
Which implements abort() as an inline call to grub_abort(), but since
that was made static by changeset:
a8f15bceeafe * grub-core/kern/misc.c (grub_abort): Make static
it is also necessary to revert the specific part that makes it a static
function too.
Another implementation of abort() was found in grub-core/kern/compiler-rt.c
which needs to also be removed to be consistent.
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In the function grub_cryptodisk_endecrypt(), a for loop is incrementing the
variable i by (1U << log_sector_size). The variable i is of type grub_size_t
which is a 64-bit unsigned integer on x86_64 architecture. On the other hand, 1U
is a 32-bit unsigned integer. By performing a left shift on a 32-bit value and
assigning it to a 64-bit variable, the 64-bit variable may have incorrect values
in the high 32-bits if the shift has an overflow. To avoid this, we replace 1U
with (grub_size_t)1.
Fixes: CID 307788
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Every heap grow will cause all disk caches invalidated which decreases
performance severely. This patch moves disk cache invalidation code to
the last of memory squeezing measures. So, disk caches are released only
when there are no other ways to get free memory.
Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
For printf()/fprintf() functions, unsigned integers should use %u as the
valid conversion specifier instead of %d.
Signed-off-by: Qiumiao Zhang <zhangqiumiao1@huawei.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The stack check logs a console message on failure, and the EFI API expects
a NULL terminated UCS-2 string. In order to define a UCS-2 string literal,
kernel.img on amd64 and i386 EFI targets is built with -fshort-wchar.
Also compile kernel.img on other EFI targets with -fshort-wchar.
Fixes: 37ddd94 (kern/efi/init: Log a console error during a stack check failure)
Reported-by: Glenn Washburn <development@efficientek.com>
Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This is useful on cloud instances with remote serial ports as it can be
difficult to connect "fast enough" to get the initial menu display
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The point of no return is used to define a point where no change should
be reverted in a wake of fatal error that consequently aborts the
process. The powerpc-ieee1275 install apparently missed this point of no
return definition that newly installed modules could be inadvertently
reverted after successful image embedding so that boot failure is
incurred due to inconsistent state.
Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
With wildly corrupt inputs, we can end up trying to calloc a very
large amount of memory, which will fail and give us a NULL pointer.
We need to check that to avoid a crash. (And, even if we blocked
such inputs, it is good practice to check the results of allocations
anyway.)
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
A user can now specify UUID strings with dashes, instead of having to remove
dashes. This is backwards-compatibility preserving and also fixes a source
of user confusion over the inconsistency with how UUIDs are specified
between file system UUIDs and cryptomount UUIDs. Since cryptsetup, the
reference implementation for LUKS, displays and generates UUIDs with dashes
there has been additional confusion when using the UUID strings from
cryptsetup as exact input into GRUB does not find the expected cryptodisk.
A new function grub_uuidcasecmp() is added that is general enough to be used
other places where UUIDs are being compared.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Variable values may contain spaces at the end or newlines. However, when
displayed without quotes this is not obvious and can lead to confusion as
to the actual contents of variables. Also for some variables grub_env_get()
returns a NULL pointer instead of a pointer to an empty string and
previously would be printed as "var=(null)". Now such variables will be
displayed as "var=''".
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This adds acpi as bootstrap module whenever it is available. This opens the
path for proper IRQ routing for fully-userland disk drivers.
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Allow treating util/grub-module-verifierXX.c as a file you can build
directly so syntax checkers like vim's "syntastic" plugin, which uses
"gcc -x c -fsyntax-only" to build it, will work.
One still has to do whatever setup is required to make it pick the
right include dirs, which -I options we use, etc., but this makes
it so you can do the checking on the file you're editing, rather
than on a different file.
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Fix the incorrect return value of __clzsi2() function.
Fixes: e795b90 (RISC-V: Add libgcc helpers for clz)
Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
We have multiple reports of things being slower with a 1 MiB initial static
allocation, and a report (more difficult to nail down) of a boot failure
as a result of the smaller initial allocation.
Make the initial memory allocation 32 MiB.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In addition to C locale there is also C.UTF-8 locale now. Filter that as
well, by using ${grub_lang}, which contains a stripped value.
This fixes the following message and resulting boot failure:
error: file `/boot/grub/locale/C.gmo' not found.
Signed-off-by: Christian Hesse <mail@eworm.de>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In some filesystem utils like mksquashfs, they will silently change
behaviour and cause timestamps to unexpectedly change. Build
environments like Debian's set SOURCE_DATE_EPOCH in the environment,
so remove it. Reproducible builds are good and useful for shipped
artifacts, but this causes build-time tests to fail.
Signed-off-by: Steve McIntyre <steve@einval.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The EFI_CONFORMANCE_PROFILES_TABLE_GUID is used for a table of GUIDs for conformance
profiles (cf. UEFI specification 2.10, 4.6.5 EFI_CONFORMANCE_PROFILE_TABLE).
The lsefisystab command is used to display installed EFI configuration tables.
Currently it only shows the GUID but not a short text for the table.
Provide a short text for the EFI_CONFORMANCE_PROFILES_TABLE_GUID.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Recently, ext4 added the large_dir feature, which adds support for
a 3 level htree directory support.
The GRUB supports existing file systems with htree directories by
ignoring their existence, and since the index nodes for the hash tree
look like deleted directory entries (by design), the GRUB can simply do
a brute force O(n) linear search of directories. The same is true for
3 level deep htrees indicated by large_dir feature flag.
Hence, it is safe for the GRUB to ignore the large_dir incompat feature.
Fixes: https://savannah.gnu.org/bugs/?61606
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
A new option is added to the loopback command, -D or --decompress, which
when specified transparently decompresses the backing file. This allows
compressed images to be used as if they were uncompressed.
Add documentation to support this change.
Suggested-by: Li Gen <ligenlive@gmail.com>
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The list of targets that support PCI is in gentpl.py. However, there is no
support for generating makefile script from a .def file that will apply
globally to the makefile, but on a per target basis. So instead, use
gentpl.py in configure to get the list of targets and check if the current
build target is one of them. If it is, set the automake conditional
COND_HAVE_PCI. Then in conf/Makefile.common add -DGRUB_HAS_PCI for the
platform if COND_HAVE_PCI is true.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Store returned value from grub_getkey() in int instead of char to
prevent throwing away the extended bits. This was a problem because,
for instance, the left arrow key press would return
(GRUB_TERM_EXTENDED | 0x4b), which would have the GRUB_TERM_EXTENDED
thrown away leaving 0x4b or 'K'. These extended keys should either
work as intended or do nothing. This change has them do nothing,
instead of inserting a key not pressed by the user.
Signed-off-by: Li Gen <ligenlive@gmail.com>
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The register_key_notify() method should have an output parameter which is
a pointer to the unique handle assigned to the registered notification.
Signed-off-by: Li Gen <ligenlive@gmail.com>
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The netmask configured in firmware is not respected on ppc64 (big endian).
When 255.255.252.0 is set as netmask in firmware, the following is the
value of bootpath string in grub_ieee1275_parse_bootpath():
/vdevice/l-lan@30000002:speed=auto,duplex=auto,192.168.88.10,,192.168.89.113,192.168.88.1,5,5,255.255.252.0,512
The netmask in this bootpath is not a problem, since it's a value specified
in firmware. But the value of subnet_mask.ipv4 was set with 0xfffffc00, and
__builtin_ctz(~grub_le_to_cpu32(subnet_mask.ipv4)) returned 16 (not 22).
As a result, 16 was used for netmask wrongly:
1111 1111 1111 1111 1111 1100 0000 0000 # subnet_mask.ipv4(=0xfffffc00)
0000 0000 1111 1100 1111 1111 1111 1111 # grub_le_to_cpu32(subnet_mask.ipv4)
1111 1111 0000 0011 0000 0000 0000 0000 # ~grub_le_to_cpu32(subnet_mask.ipv4)
and the count of zero with __builtin_ctz() can be 16. This patch changes
it as below:
1111 1111 1111 1111 1111 1100 0000 0000 # subnet_mask.ipv4(=0xfffffc00)
0000 0000 1111 1100 1111 1111 1111 1111 # grub_le_to_cpu32(subnet_mask.ipv4)
1111 1111 1111 1111 1111 1100 0000 0000 # grub_be_to_cpu32(subnet_mask.ipv4)
0000 0000 0000 0000 0000 0011 1111 1111 # ~grub_be_to_cpu32(subnet_mask.ipv4)
The count of zero with __builtin_clz() can be 22 (clz counts the number
of one bits preceding the most significant zero bit).
Signed-off-by: Masahiro Matsuya <mmatsuya@redhat.com>
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Numerous register fields in the relocator state are simply not
used depending on the relocator. This causes Coverity to flag
these fields but there is no real bug here. Simply initializing
the variable to {0} solves the issue. Fixed in the else case too
for consistency.
Fixes: CID 396932
Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This is trivial, but it might save some time to beginners.
Signed-off-by: Andrea G. Monaco <andrea.monaco@autistici.org>
Reviewed-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Add a new --is-supported option to commands/efi/efifwsetup and
conditionalize display on it.
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The "fwsetup" command is only registered if the firmware supports booting
to the firmware setup UI. But it could be possible that the GRUB config
already contains a "fwsetup" entry, because it was generated in a machine
that has support for this feature.
To prevent users getting an error like:
error: ../../grub-core/script/function.c:109:can't find command `fwsetup'.
if it is not supported by the firmware, let's just always register the
command but print a more accurate message if the firmware doesn't
support this option.
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The 30_uefi-firmware template checks if an OsIndicationsSupported UEFI var
exists and EFI_OS_INDICATIONS_BOOT_TO_FW_UI bit is set, to decide whether
a "fwsetup" menu entry would be added or not to the GRUB menu.
But this has the problem that it will only work if the configuration file
was created on an UEFI machine that supports booting to a firmware UI.
This for example doesn't support creating GRUB config files when executing
on systems that support both UEFI and legacy BIOS booting. Since creating
the config file from legacy BIOS wouldn't allow to access the firmware UI.
To prevent this, make the template to unconditionally create the grub.cfg
snippet but check at runtime if was booted through UEFI to decide if this
entry should be added. That way it won't be added when booting with BIOS.
There's no need to check if EFI_OS_INDICATIONS_BOOT_TO_FW_UI bit is set,
since that's already done by the "fwsetup" command when is executed.
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Each call of grub_efi_get_variable() needs a grub_free().
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Coverity reports that dnode_end_t argument of fill_fs_info() is too
large to pass-by-value. Therefore, replace the argument with a pointer.
Fixes: CID 73631
Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
It was reported in the #grub IRC channel on Libera that decryption of
LUKS2 partitions fails with errors about invalid digests and/or salts.
In all of these cases, what failed was decoding the Base64
representation of these, where the encoded data contained invalid
characters.
As it turns out, the root cause is that json-c, which is used by
cryptsetup to read and write the JSON header, will escape some
characters by prepending a backslash when writing JSON strings by
default. Most importantly, json-c also escapes the forward slash, which
is part of the Base64 alphabet. Because GRUB doesn't know to unescape
such characters, decoding this string will rightfully fail.
Interestingly, this issue has until now only been reported by users of
Ubuntu 18.04. And a bit of digging in fact reveals that cryptsetup has
changed the logic in a054206d (Suppress useless slash escaping in json
lib, 2018-04-20), which has been released with cryptsetup v2.0.3. Ubuntu
18.04 is still shipping with cryptsetup v2.0.2 though, which explains
why this is not a more frequent issue.
Fix the issue by using our new grub_json_unescape() helper function
that handles unescaping for us.
Reported-by: Afdal
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <dkiper@net-space.pl>
JSON strings require certain characters to be encoded, either by using
a single reverse solidus character "\" for a set of popular characters,
or by using a Unicode representation of "\uXXXXX". The jsmn library
doesn't handle unescaping for us, so we must implement this functionality
for ourselves.
Add a new function grub_json_unescape() that takes a potentially
escaped JSON string as input and returns a new unescaped string.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <dkiper@net-space.pl>
In the case of an error grub_initrd_load() uses argv[] to print the
filename that caused the error. It is also possible to obtain the
filename from the file handles and there is no need to duplicate that
information in argv[], so let's drop it.
Signed-off-by: Nikita Ermakov <arei@altlinux.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In grub-core/loader/i386/bsdXX.c and grub-core/loader/multiboot_elfxx.c, error
conditionals are simplified to statements such as "if (err)". Even though the
assumption that non-zero values give errors is correct, it would be clearer and
more consistent to compare these conditionals to GRUB_ERR_NONE.
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In util/grub-module-verifierXX.c, the function get_shnum() returns the variable
shnum, which is of the type Elf_Word. In the function, shnum can be obtained by
the e_shnum member of an Elf_Ehdr or the sh_size member of an Elf_Shdr. The
sh_size member can either be grub_uint32_t or grub_uint64_t, depending on the
architecture, but Elf_Word is only grub_uint32_t. To account for when sh_size is
grub_uint64_t, we can set shnum to have type Elf_Shnum and have get_shnum()
return an Elf_Shnum.
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In bsdXX.c and multiboot_elfxx.c, e_phnum is used to obtain the number of
program header table entries, but it wasn't being checked if the value was
there.
According to the elf(5) manual page,
"If the number of entries in the program header table is larger than or equal to
PN_XNUM (0xffff), this member holds PN_XNUM (0xffff) and the real number of
entries in the program header table is held in the sh_info member of the
initial entry in section header table. Otherwise, the sh_info member of the
initial entry contains the value zero."
Since this check wasn't being made, grub_elfXX_get_phnum() is being added to
elfXX.c to make this check and use e_phnum if it doesn't have PN_XNUM as a
value, else use sh_info. We also need to make sure e_phnum isn't greater than
PN_XNUM and sh_info isn't less than PN_XNUM.
Note that even though elf.c and elfXX.c are located in grub-core/kern, they are
compiled as modules and don't need the EXPORT_FUNC() macro to define the functions
in elf.h.
Also, changed casts of phnum to match variables being set as well as dropped
casts when unnecessary.
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In multiboot_elfxx.c, e_shstrndx is used to obtain the section header table
index of the section name string table, but it wasn't being checked if the value
was there.
According to the elf(5) manual page,
"If the index of section name string table section is larger than or equal to
SHN_LORESERVE (0xff00), this member holds SHN_XINDEX (0xffff) and the real
index of the section name string table section is held in the sh_link member of
the initial entry in section header table. Otherwise, the sh_link member of the
initial entry in section header table contains the value zero."
Since this check wasn't being made, grub_elfXX_get_shstrndx() is being added to
elfXX.c to make this check and use e_shstrndx if it doesn't have SHN_XINDEX as a
value, else use sh_link. We also need to make sure e_shstrndx isn't greater than
or equal to SHN_LORESERVE and sh_link isn't less than SHN_LORESERVE.
Note that even though elf.c and elfXX.c are located in grub-core/kern, they are
compiled as modules and don't need the EXPORT_FUNC() macro to define the functions
in elf.h.
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In bsdXX.c and multiboot_elfxx.c, e_shnum is used to obtain the number of
section header table entries, but it wasn't being checked if the value was
there.
According to the elf(5) manual page,
"If the number of entries in the section header table is larger than or equal to
SHN_LORESERVE (0xff00), e_shnum holds the value zero and the real number of
entries in the section header table is held in the sh_size member of the initial
entry in section header table. Otherwise, the sh_size member of the initial
entry in the section header table holds the value zero."
Since this check wasn't being made, grub_elfXX_get_shnum() is being added to
elfXX.c to make this check and use whichever member doesn't have a value of
zero. If both are zero, then we must return an error. We also need to make sure
that e_shnum doesn't have a value greater than or equal to SHN_LORESERVE and
sh_size isn't less than SHN_LORESERVE.
In order to get this function to work, the type ElfXX_Shnum is being added where
Elf32_Shnum defines Elf32_Word and Elf64_Shnum defines Elf64_Xword. This new
type is needed because if shnum obtains a value from sh_size, sh_size could be
of type El32_Word for Elf32_Shdr structures or Elf64_Xword for Elf64_Shdr
structures.
Note that even though elf.c and elfXX.c are located in grub-core/kern, they are
compiled as modules and don't need the EXPORT_FUNC() macro to define the functions
in elf.h.
For a few smaller changes, changed casts of shnum to match variables being set
as well as dropped casts when unnecessary and fixed spacing errors in bsdXX.c.
Also, shnum is an unsigned integer and is compared to int i in multiboot_elfxx.c,
it should be unsigned to match shnum.
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The linux_xen template orders the "early" initrd file(s) _first_
(i.e., before the "real" initrd files) and that seems reasonable,
as microcode updates usually come first.
However, this usually breaks Linux boot with initrd under Xen
because Xen assumes the real initrd is the first multiboot[2]
module after the kernel, passing its address over to Linux in
Xen's start_info struct.
So, if a microcode-only initrd (i.e., without init/userspace)
is found by grub-mkconfig, it ends up considered as a normal
initrd by the Linux kernel, which cannot do anything with it
(as it has no other files) and panic()s unable to mount root
if it depends on a initrd to do that (e.g., root=UUID=...).
...
Well, since Xen doesn't actually use the provided microcode
by default / unless the 'ucode=<module number|scan>' option
is enabled, this isn't used in the general case (and breaks).
Additionally, if an user enables the 'ucode=' option, that
either specifies which module is to be used for microcode,
or scans all modules (regardless of being first) for that.
Thus, for Xen:
- it is *not required* to have microcode first,
- but it is *required* to have real initrd first
So, fix it by ordering the real initrd before early initrd(s).
After:
# touch /boot/xen /boot/microcode.cpio
# grub-mkconfig 2>/dev/null | grep -P '^\t(multiboot|module)'
multiboot /boot/xen ...
module /boot/vmlinuz-5.4.0-122-generic ...
module --nounzip /boot/initrd.img-5.4.0-122-generic
module --nounzip /boot/microcode.cpio
...
Corner case specific to Xen implementation details:
It is actually _possible_ to have a microcode initrd first,
but that requires a non-default option (so can't rely on it),
and it turns out to be inconsistent with its counterpart
(really shouldn't rely on it, as it may get confusing; below).
'ucode=1' does manually specify the first module is microcode
_AND_ clears its bit in the module bitmap. The next module is
now the 'new first', and gets passed to Linux as initrd. Good.
'ucode=scan' checks all modules for microcode, but does _NOT_
clear a bit if it finds one (reasonable, as it can find that
prepended in a "real" initrd anyway, which needs to be used).
The first module still gets passed to Linux as initrd. Bad.
Fixes: e86f6aafb8de (grub-mkconfig/20_linux_xen: Support multiple early initrd images)
Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
Acked-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The linux_xen template can put multiple initrd files in the
same multiboot[2] module[2] command, which is against specs.
This causes ONLY the _first_ initrd file to be loaded; other
files just have filenames in a "cmdline" string of the first
initrd file and are NOT loaded.
Fix this by inserting a module[2] command per initrd file.
Before:
# touch /boot/xen /boot/microcode.cpio
# grub-mkconfig 2>/dev/null | grep -P '^\t(multiboot|module)'
multiboot /boot/xen ...
module /boot/vmlinuz-5.4.0-122-generic ...
module --nounzip /boot/microcode.cpio /boot/initrd.img-5.4.0-122-generic
After:
# touch /boot/xen /boot/microcode.cpio
# grub-mkconfig 2>/dev/null | grep -P '^\t(multiboot|module)'
multiboot /boot/xen ...
module /boot/vmlinuz-5.4.0-122-generic ...
module --nounzip /boot/microcode.cpio
module --nounzip /boot/initrd.img-5.4.0-122-generic
Cause:
The code was copied from the linux template, which is *apparently*
equivalent.. but its initrd command grub_cmd_initrd() *supports*
multiple files (see grub_initrd_init()), while module/module2 in
grub_cmd_module() *does not* (see grub_multiboot[2]_add_module()).
See commit e86f6aafb8de (grub-mkconfig/20_linux_xen: Support multiple early initrd images):
'This is basically a copy of a698240d "grub-mkconfig/10_linux:
Support multiple early initrd images" ...'
Specs:
Both multiboot and multiboot2 specifications mention support for
'multiple boot modules' (struct/tag used for kernel/initrd files):
"Boot loaders don’t have to support multiple boot modules,
but they are strongly encouraged to" [1,2]
However, there is a 1:1 relationship between boot modules and files,
more or less clearly; note the usage of singular/plural "module(s)".
(Multiboot2, clearly: "One tag appears per module".)
Multiboot [1]:
"the ‘mods’ fields indicate ... what boot modules
were loaded ..., and where they can be found.
‘mods_count’ contains the number of modules loaded"
"The first two fields contain the start and end addresses
of the boot module itself."
Multiboot2 [2]:
"This tag indicates ... what boot module was loaded ...,
and where it can be found."
"The ‘mod_start’ and ‘mod_end’ contain the start and end
physical addresses of the boot module itself."
"One tag appears per module.
This tag type may appear multiple times."
And both clearly mention the 'string' field of a boot module,
which is to be used by the operating system, not boot loader:
"The ‘string’ field provides an arbitrary string to be
associated with that particular boot module ...
its exact use is specific to the operating system."
Links:
[1] https://www.gnu.org/software/grub/manual/multiboot/multiboot.html
3.3 Boot information format
[2] https://www.gnu.org/software/grub/manual/multiboot2/multiboot.html
3.6.6 Modules
Fixes: e86f6aafb8de (grub-mkconfig/20_linux_xen: Support multiple early initrd images)
Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
Acked-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Note this cast was fixed in grub_strcasecmp() in commit ce41ab7aab
(* grub-core/kern/misc.c (grub_strcmp): Use unsigned comparison as per
common usage and preffered in several parts of code.), but this commit
omitted fixing it in grub_strncasecmp().
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The previous behavior ignored an error and the output from grub-mkrescue.
This made it difficult to discover that grub-mkrescue was the reason that
tests which rely on grub-shell were failing. Even after discovering
grub-mkrescue was the culprit, there was no output to indicate why it was
failing. It turns out that grub-mkrescue is a thin wrapper around xorriso.
So if you do not have xorriso installed it will fail with an error message
about not being able to find xorriso.
This change will allow grub-mkrescue output to be written to stderr, only
if grub-mkrescue fails. If grub-mkrescue succeeds, there will be no output
from grub-mkrescue so as not to interfere with the functioning of tests.
This change should have no effect on the running of tests or other uses of
grub-shell as it only modifies the error path.
Also, if grub-mkrescue fails, the script exits early. Since grub-shell
needs the ISO image created by grub-mkresue to boot the QEMU instance,
a failure here should be considered fatal.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The "ARM\x64" magic number in the file header identifies an image as one
that implements the bare metal boot protocol, allowing the loader to
simply move the file to a suitably aligned address in memory, with
sufficient headroom for the trailing .bss segment (the required memory
size is described in the header as well).
Note of this matters for GRUB, as it only supports EFI boot. EFI does
not care about this magic number, and nor should GRUB: this prevents us
from booting other PE linux images, such as the generic EFI zboot
decompressor, which is a pure PE/COFF image, and does not implement the
bare metal boot protocol.
So drop the magic number check.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Because grub_util_mkdir() is implemented to not return a value on any
platform, grub_instal_mkdir_p() can test for success by confirming that
the directory requested exists after attempting to create it, otherwise
it should fail with an error and exit.
While fixing this, a flaw in the logic was shown, where the first match
of the path separator, which almost always was the first character in
the path (e.g. /boot/grub2) would result in creating a directory with an
empty name (i.e. ""). To avoid that, it should skip the handling of the
path separator where p is pointing to the first character.
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Coverity signaled 2 issues where the return value of grub_util_mkdir()
was not being tested.
The Windows variant of this code defines the function as having no
return value (void), but the UNIX variants all are mapped using a macro
to the libc mkdir() function, which returns an int value.
To be consistent, the mapping should cast to void to for these too.
Fixes: CID 73583
Fixes: CID 73617
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
