Commit graph

3976 commits

Author SHA1 Message Date
Daniel Kiper
c386331364 i386/efi/init: Drop bogus include
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
2020-09-18 21:22:32 +02:00
Javier Martinez Canillas
a6838bbc67 tftp: Roll-over block counter to prevent data packets timeouts
Commit 781b3e5efc (tftp: Do not use priority queue) caused a regression
when fetching files over TFTP whose size is bigger than 65535 * block size.

  grub> linux /images/pxeboot/vmlinuz
  grub> echo $?
  0
  grub> initrd /images/pxeboot/initrd.img
  error: timeout reading '/images/pxeboot/initrd.img'.
  grub> echo $?
  28

It is caused by the block number counter being a 16-bit field, which leads
to a maximum file size of ((1 << 16) - 1) * block size. Because GRUB sets
the block size to 1024 octets (by using the TFTP Blocksize Option from RFC
2348 [0]), the maximum file size that can be transferred is 67107840 bytes.

The TFTP PROTOCOL (REVISION 2) RFC 1350 [1] does not mention what a client
should do when a file size is bigger than the maximum, but most TFTP hosts
support the block number counter to be rolled over. That is, acking a data
packet with a block number of 0 is taken as if the 65356th block was acked.

It was working before because the block counter roll-over was happening due
an overflow. But that got fixed by the mentioned commit, which led to the
regression when attempting to fetch files larger than the maximum size.

To allow TFTP file transfers of unlimited size again, re-introduce a block
counter roll-over so the data packets are acked preventing the timeouts.

[0]: https://tools.ietf.org/html/rfc2348
[1]: https://tools.ietf.org/html/rfc1350

Fixes: 781b3e5efc (tftp: Do not use priority queue)

Suggested-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-11 15:52:07 +02:00
Glenn Washburn
e1b0992a8d cryptodisk: Fix incorrect calculation of start sector
Here dev is a grub_cryptodisk_t and dev->offset is offset in sectors of size
native to the cryptodisk device. The sector is correctly transformed into
native grub sector size, but then added to dev->offset which is not
transformed. It would be nice if the type system would help us with this.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-11 15:51:04 +02:00
Glenn Washburn
3b3ac0c982 cryptodisk: Unregister cryptomount command when removing module
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-11 15:48:50 +02:00
Patrick Steinhardt
eb77486141 luks2: Improve error reporting when decrypting/verifying key
While we already set up error messages in both luks2_verify_key() and
luks2_decrypt_key(), we do not ever print them. This makes it really
hard to discover why a given key actually failed to decrypt a disk.

Improve this by including the error message in the user-visible output.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-11 15:48:13 +02:00
Patrick Steinhardt
1066336dc8 luks: Fix out-of-bounds copy of UUID
When configuring a LUKS disk, we copy over the UUID from the LUKS header
into the new grub_cryptodisk_t structure via grub_memcpy(). As size
we mistakenly use the size of the grub_cryptodisk_t UUID field, which
is guaranteed to be strictly bigger than the LUKS UUID field we're
copying. As a result, the copy always goes out-of-bounds and copies some
garbage from other surrounding fields. During runtime, this isn't
noticed due to the fact that we always NUL-terminate the UUID and thus
never hit the trailing garbage.

Fix the issue by using the size of the local stripped UUID field.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-11 15:47:39 +02:00
Patrick Steinhardt
b35792dccb json: Remove invalid typedef redefinition
The C standard does not allow for typedef redefinitions, even if they
map to the same underlying type. In order to avoid including the
jsmn.h in json.h and thus exposing jsmn's internals, we have exactly
such a forward-declaring typedef in json.h. If enforcing the GNU99 C
standard, clang may generate a warning about this non-standard
construct.

Fix the issue by using a simple "struct jsmntok" forward declaration
instead of using a typedef.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Tested-by: Chuck Tuffli <chuck@freebsd.org>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-11 15:46:55 +02:00
Cao jin
74259522d7 i386/relocator_common: Drop empty #ifdef
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-11 15:46:36 +02:00
Ave Milia
a0ba8fae61 video/bochs: Fix typo
Signed-off-by: Ave Milia <avemilia@protonmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-11 15:45:56 +02:00
Colin Watson
e7b8856f8b linux: Fix integer overflows in initrd size handling
These could be triggered by a crafted filesystem with very large files.

Fixes: CVE-2020-15707

Signed-off-by: Colin Watson <cjwatson@debian.org>
Reviewed-by: Jan Setje-Eilers <jan.setjeeilers@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Peter Jones
0dcbf3652b loader/linux: Avoid overflow on initrd size calculation
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Alexey Makhalov
f7bd9986f6 efi: Fix use-after-free in halt/reboot path
commit 92bfc33db9 ("efi: Free malloc regions on exit")
introduced memory freeing in grub_efi_fini(), which is
used not only by exit path but by halt/reboot one as well.
As result of memory freeing, code and data regions used by
modules, such as halt, reboot, acpi (used by halt) also got
freed. After return to module code, CPU executes, filled
by UEFI firmware (tested with edk2), 0xAFAFAFAF pattern as
a code. Which leads to #UD exception later.

grub> halt
!!!! X64 Exception Type - 06(#UD - Invalid Opcode)  CPU Apic ID - 00000000 !!!!
RIP  - 0000000003F4EC28, CS  - 0000000000000038, RFLAGS - 0000000000200246
RAX  - 0000000000000000, RCX - 00000000061DA188, RDX - 0A74C0854DC35D41
RBX  - 0000000003E10E08, RSP - 0000000007F0F860, RBP - 0000000000000000
RSI  - 00000000064DB768, RDI - 000000000832C5C3
R8   - 0000000000000002, R9  - 0000000000000000, R10 - 00000000061E2E52
R11  - 0000000000000020, R12 - 0000000003EE5C1F, R13 - 00000000061E0FF4
R14  - 0000000003E10D80, R15 - 00000000061E2F60
DS   - 0000000000000030, ES  - 0000000000000030, FS  - 0000000000000030
GS   - 0000000000000030, SS  - 0000000000000030
CR0  - 0000000080010033, CR2 - 0000000000000000, CR3 - 0000000007C01000
CR4  - 0000000000000668, CR8 - 0000000000000000
DR0  - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
DR3  - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400
GDTR - 00000000079EEA98 0000000000000047, LDTR - 0000000000000000
IDTR - 0000000007598018 0000000000000FFF,   TR - 0000000000000000
FXSAVE_STATE - 0000000007F0F4C0

Proposal here is to continue to free allocated memory for
exit boot services path but keep it for halt/reboot path
as it won't be much security concern here.
Introduced GRUB_LOADER_FLAG_EFI_KEEP_ALLOCATED_MEMORY
loader flag to be used by efi halt/reboot path.

Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Daniel Kiper
098058752e efi/chainloader: Propagate errors from copy_file_path()
Without any error propagated to the caller, make_file_path()
would then try to advance the invalid device path node with
GRUB_EFI_NEXT_DEVICE_PATH(), which would fail, returning a NULL
pointer that would subsequently be dereferenced. Hence, propagate
errors from copy_file_path().

Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Peter Jones
d2cf823d0e efi: Fix some malformed device path arithmetic errors
Several places we take the length of a device path and subtract 4 from
it, without ever checking that it's >= 4. There are also cases where
this kind of malformation will result in unpredictable iteration,
including treating the length from one dp node as the type in the next
node. These are all errors, no matter where the data comes from.

This patch adds a checking macro, GRUB_EFI_DEVICE_PATH_VALID(), which
can be used in several places, and makes GRUB_EFI_NEXT_DEVICE_PATH()
return NULL and GRUB_EFI_END_ENTIRE_DEVICE_PATH() evaluate as true when
the length is too small. Additionally, it makes several places in the
code check for and return errors in these cases.

Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Peter Jones
b73cee7f1f emu: Make grub_free(NULL) safe
The grub_free() implementation in grub-core/kern/mm.c safely handles
NULL pointers, and code at many places depends on this. We don't know
that the same is true on all host OSes, so we need to handle the same
behavior in grub-emu's implementation.

Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Peter Jones
879c4a8342 lvm: Fix two more potential data-dependent alloc overflows
It appears to be possible to make a (possibly invalid) lvm PV with
a metadata size field that overflows our type when adding it to the
address we've allocated. Even if it doesn't, it may be possible to do so
with the math using the outcome of that as an operand. Check them both.

Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Peter Jones
f5703eb062 hfsplus: Fix two more overflows
Both node->size and node->namelen come from the supplied filesystem,
which may be user-supplied. We can't trust them for the math unless we
know they don't overflow. Making sure they go through grub_add() or
grub_calloc() first will give us that.

Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Alexey Makhalov
07e5b79e22 relocator: Fix grub_relocator_alloc_chunk_align() top memory allocation
Current implementation of grub_relocator_alloc_chunk_align()
does not allow allocation of the top byte.

Assuming input args are:
  max_addr = 0xfffff000;
  size = 0x1000;

And this is valid. But following overflow protection will
unnecessarily move max_addr one byte down (to 0xffffefff):
  if (max_addr > ~size)
    max_addr = ~size;

~size + 1 will fix the situation. In addition, check size
for non zero to do not zero max_addr.

Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Chris Coulson
426f57383d script: Avoid a use-after-free when redefining a function during execution
Defining a new function with the same name as a previously defined
function causes the grub_script and associated resources for the
previous function to be freed. If the previous function is currently
executing when a function with the same name is defined, this results
in use-after-frees when processing subsequent commands in the original
function.

Instead, reject a new function definition if it has the same name as
a previously defined function, and that function is currently being
executed. Although a behavioural change, this should be backwards
compatible with existing configurations because they can't be
dependent on the current behaviour without being broken.

Fixes: CVE-2020-15706

Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Alexey Makhalov
61ff5602fe relocator: Protect grub_relocator_alloc_chunk_align() max_addr against integer underflow
This commit introduces integer underflow mitigation in max_addr calculation
in grub_relocator_alloc_chunk_align() invocation.

It consists of 2 fixes:
  1. Introduced grub_relocator_alloc_chunk_align_safe() wrapper function to perform
     sanity check for min/max and size values, and to make safe invocation of
     grub_relocator_alloc_chunk_align() with validated max_addr value. Replace all
     invocations such as grub_relocator_alloc_chunk_align(..., min_addr, max_addr - size, size, ...)
     by grub_relocator_alloc_chunk_align_safe(..., min_addr, max_addr, size, ...).
  2. Introduced UP_TO_TOP32(s) macro for the cases where max_addr is 32-bit top
     address (0xffffffff - size + 1) or similar.

Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Alexey Makhalov
caea56d1f8 relocator: Protect grub_relocator_alloc_chunk_addr() input args against integer underflow/overflow
Use arithmetic macros from safemath.h to accomplish it. In this commit,
I didn't want to be too paranoid to check every possible math equation
for overflow/underflow. Only obvious places (with non zero chance of
overflow/underflow) were refactored.

Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Alexey Makhalov
781b3e5efc tftp: Do not use priority queue
There is not need to reassemble the order of blocks. Per RFC 1350,
server must wait for the ACK, before sending next block. Data packets
can be served immediately without putting them to priority queue.

Logic to handle incoming packet is this:
  - if packet block id equal to expected block id, then
    process the packet,
  - if packet block id is less than expected - this is retransmit
    of old packet, then ACK it and drop the packet,
  - if packet block id is more than expected - that shouldn't
    happen, just drop the packet.

It makes the tftp receive path code simpler, smaller and faster.
As a benefit, this change fixes CID# 73624 and CID# 96690, caused
by following while loop:

  while (cmp_block (grub_be_to_cpu16 (tftph->u.data.block), data->block + 1) == 0)

where tftph pointer is not moving from one iteration to another, causing
to serve same packet again. Luckily, double serving didn't happen due to
data->block++ during the first iteration.

Fixes: CID 73624, CID 96690

Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Konrad Rzeszutek Wilk
f8ad7a3dd8 multiboot2: Fix memory leak if grub_create_loader_cmdline() fails
Fixes: CID 292468

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Konrad Rzeszutek Wilk
d17770857e udf: Fix memory leak
Fixes: CID 73796

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Jan Setje-Eilers <jan.setjeeilers@oracle.com>
2020-07-29 16:55:48 +02:00
Konrad Rzeszutek Wilk
61b7ca08d1 term: Fix overflow on user inputs
This requires a very weird input from the serial interface but can cause
an overflow in input_buf (keys) overwriting the next variable (npending)
with the user choice:

(pahole output)

struct grub_terminfo_input_state {
        int                        input_buf[6];         /*     0    24 */
        int                        npending;             /*    24     4 */ <- CORRUPT
        ...snip...

The magic string requires causing this is "ESC,O,],0,1,2,q" and we overflow
npending with "q" (aka increase npending to 161). The simplest fix is to
just to disallow overwrites input_buf, which exactly what this patch does.

Fixes: CID 292449

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Konrad Rzeszutek Wilk
16c0dbf4bc lzma: Make sure we don't dereference past array
The two dimensional array p->posSlotEncoder[4][64] is being dereferenced
using the GetLenToPosState() macro which checks if len is less than 5,
and if so subtracts 2 from it. If len = 0, that is 0 - 2 = 4294967294.
Obviously we don't want to dereference that far out so we check if the
position found is greater or equal kNumLenToPosStates (4) and bail out.

N.B.: Upstream LZMA 18.05 and later has this function completely rewritten
without any history.

Fixes: CID 51526

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Chris Coulson
dc052e5ac7 json: Avoid a double-free when parsing fails.
When grub_json_parse() succeeds, it returns the root object which
contains a pointer to the provided JSON string. Callers are
responsible for ensuring that this string outlives the root
object and for freeing its memory when it's no longer needed.

If grub_json_parse() fails to parse the provided JSON string,
it frees the string before returning an error. This results
in a double free in luks2_recover_key(), which also frees the
same string after grub_json_parse() returns an error.

This changes grub_json_parse() to never free the JSON string
passed to it, and updates the documentation for it to make it
clear that callers are responsible for ensuring that the string
outlives the root JSON object.

Fixes: CID 292465

Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Alexey Makhalov
6d7a59a2a1 xnu: Fix double free in grub_xnu_devprop_add_property()
grub_xnu_devprop_add_property() should not free utf8 and utf16 as it get
allocated and freed in the caller.

Minor improvement: do prop fields initialization after memory allocations.

Fixes: CID 292442, CID 292457, CID 292460, CID 292466

Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Alexey Makhalov
26a8c19307 gfxmenu: Fix double free in load_image()
self->bitmap should be zeroed after free. Otherwise, there is a chance
to double free (USE_AFTER_FREE) it later in rescale_image().

Fixes: CID 292472

Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Daniel Kiper
89f3da1a3d font: Do not load more than one NAME section
The GRUB font file can have one NAME section only. Though if somebody
crafts a broken font file with many NAME sections and loads it then the
GRUB leaks memory. So, prevent against that by loading first NAME
section and failing in controlled way on following one.

Reported-by: Chris Coulson <chris.coulson@canonical.com>
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Jan Setje-Eilers <jan.setjeeilers@oracle.com>
2020-07-29 16:55:48 +02:00
Peter Jones
2a1edcf2ed iso9660: Don't leak memory on realloc() failures
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Peter Jones
3f05d693d1 malloc: Use overflow checking primitives where we do complex allocations
This attempts to fix the places where we do the following where
arithmetic_expr may include unvalidated data:

  X = grub_malloc(arithmetic_expr);

It accomplishes this by doing the arithmetic ahead of time using grub_add(),
grub_sub(), grub_mul() and testing for overflow before proceeding.

Among other issues, this fixes:
  - allocation of integer overflow in grub_video_bitmap_create()
    reported by Chris Coulson,
  - allocation of integer overflow in grub_png_decode_image_header()
    reported by Chris Coulson,
  - allocation of integer overflow in grub_squash_read_symlink()
    reported by Chris Coulson,
  - allocation of integer overflow in grub_ext2_read_symlink()
    reported by Chris Coulson,
  - allocation of integer overflow in read_section_as_string()
    reported by Chris Coulson.

Fixes: CVE-2020-14309, CVE-2020-14310, CVE-2020-14311

Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:47 +02:00
Peter Jones
f725fa7cb2 calloc: Use calloc() at most places
This modifies most of the places we do some form of:

  X = malloc(Y * Z);

to use calloc(Y, Z) instead.

Among other issues, this fixes:
  - allocation of integer overflow in grub_png_decode_image_header()
    reported by Chris Coulson,
  - allocation of integer overflow in luks_recover_key()
    reported by Chris Coulson,
  - allocation of integer overflow in grub_lvm_detect()
    reported by Chris Coulson.

Fixes: CVE-2020-14308

Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:47 +02:00
Peter Jones
64e26162eb calloc: Make sure we always have an overflow-checking calloc() available
This tries to make sure that everywhere in this source tree, we always have
an appropriate version of calloc() (i.e. grub_calloc(), xcalloc(), etc.)
available, and that they all safely check for overflow and return NULL when
it would occur.

Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:47 +02:00
Peter Jones
a4d3fbdff1 yylex: Make lexer fatal errors actually be fatal
When presented with a command that can't be tokenized to anything
smaller than YYLMAX characters, the parser calls YY_FATAL_ERROR(errmsg),
expecting that will stop further processing, as such:

  #define YY_DO_BEFORE_ACTION \
        yyg->yytext_ptr = yy_bp; \
        yyleng = (int) (yy_cp - yy_bp); \
        yyg->yy_hold_char = *yy_cp; \
        *yy_cp = '\0'; \
        if ( yyleng >= YYLMAX ) \
                YY_FATAL_ERROR( "token too large, exceeds YYLMAX" ); \
        yy_flex_strncpy( yytext, yyg->yytext_ptr, yyleng + 1 , yyscanner); \
        yyg->yy_c_buf_p = yy_cp;

The code flex generates expects that YY_FATAL_ERROR() will either return
for it or do some form of longjmp(), or handle the error in some way at
least, and so the strncpy() call isn't in an "else" clause, and thus if
YY_FATAL_ERROR() is *not* actually fatal, it does the call with the
questionable limit, and predictable results ensue.

Unfortunately, our implementation of YY_FATAL_ERROR() is:

   #define YY_FATAL_ERROR(msg)                     \
     do {                                          \
       grub_printf (_("fatal error: %s\n"), _(msg));     \
     } while (0)

The same pattern exists in yyless(), and similar problems exist in users
of YY_INPUT(), several places in the main parsing loop,
yy_get_next_buffer(), yy_load_buffer_state(), yyensure_buffer_stack,
yy_scan_buffer(), etc.

All of these callers expect YY_FATAL_ERROR() to actually be fatal, and
the things they do if it returns after calling it are wildly unsafe.

Fixes: CVE-2020-10713

Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:47 +02:00
Marc Zyngier
6a34fdb76a arm: Fix 32-bit ARM handling of the CTR register
When booting on an ARMv8 core that implements either CTR.IDC or CTR.DIC
(indicating that some of the cache maintenance operations can be
removed when dealing with I/D-cache coherency, GRUB dies with a
"Unsupported cache type 0x........" message.

This is pretty likely to happen when running in a virtual machine
hosted on an arm64 machine (I've triggered it on a system built around
a bunch of Cortex-A55 cores, which implements CTR.IDC).

It turns out that the way GRUB deals with the CTR register is a bit
harsh for anything from ARMv7 onwards. The layout of the register is
backward compatible, meaning that nothing that gets added is allowed to
break earlier behaviour. In this case, ignoring IDC is completely fine,
and only results in unnecessary cache maintenance.

We can thus avoid being paranoid, and align the 32bit behaviour with
its 64bit equivalent.

This patch has the added benefit that it gets rid of a (gnu-specific)
case range too.

Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-05-25 15:02:51 +02:00
Javier Martinez Canillas
96be75ecbd net: Break out nested function
Nested functions are not supported in C, but are permitted as an extension
in the GNU C dialect. Commit cb2f15c544 ("normal/main: Search for specific
config files for netboot") added a nested function which caused the build
to break when compiling with clang.

Break that out into a static helper function to make the code portable again.

Reported-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-05-25 14:52:10 +02:00
Javier Martinez Canillas
cc6bd49a52 tpm: Enable module for all EFI platforms
The module is only enabled for x86_64, but there's nothing specific to
x86_64 in the implementation and can be enabled for all EFI platforms.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-05-25 14:50:42 +02:00
Tianjia Zhang
c867185b81 tpm: Rename function grub_tpm_log_event() to grub_tpm_measure()
grub_tpm_log_event() and grub_tpm_measure() are two functions that
have the same effect. So, keep grub_tpm_log_event() and rename it
to grub_tpm_measure(). This way we get also a more clear semantics.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-05-15 15:37:28 +02:00
Tianjia Zhang
0fa9ed41ac verifiers: Add verify string debug message
Like grub_verifiers_open(), the grub_verify_string() should also
display this debug message, which is very helpful for debugging.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-05-15 15:26:41 +02:00
Javier Martinez Canillas
0f3600bf1b envblk: Fix buffer overrun when attempting to shrink a variable value
If an existing variable is set with a value whose length is smaller than
the current value, a memory corruption can happen due copying padding '#'
characters outside of the environment block buffer.

This is caused by a wrong calculation of the previous free space position
after moving backward the characters that followed the old variable value.

That position is calculated to fill the remaining of the buffer with the
padding '#' characters. But since isn't calculated correctly, it can lead
to copies outside of the buffer.

The issue can be reproduced by creating a variable with a large value and
then try to set a new value that is much smaller:

$ grub2-editenv --version
grub2-editenv (GRUB) 2.04

$ grub2-editenv env create

$ grub2-editenv env set a="$(for i in {1..500}; do var="b$var"; done; echo $var)"

$ wc -c env
1024 grubenv

$ grub2-editenv env set a="$(for i in {1..50}; do var="b$var"; done; echo $var)"
malloc(): corrupted top size
Aborted (core dumped)

$ wc -c env
0 grubenv

Reported-by: Renaud Métrich <rmetrich@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-05-15 15:24:59 +02:00
Daniel Kiper
eb46ee98bc mips/cache: Add missing nop's in delay slots
Lack of them causes random instructions to be executed before the
jump really happens.

Signed-off-by: Vladimir Serbinenko <phcoder@google.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-05-15 14:30:07 +02:00
Patrick Steinhardt
c543d67810 luks2: Propagate error when reading area key fails
When decrypting a given keyslot, all error cases except for one set up
an error and return the error code. The only exception is when we try to
read the area key: instead of setting up an error message, we directly
print it via grub_dprintf().

Convert the outlier to use grub_error() to allow more uniform handling
of errors.

Signed-off-by: Patrick Steinhardt <ps@kps.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-04-21 22:18:26 +02:00
Patrick Steinhardt
e933feb578 json: Get rid of casts for "jsmntok_t"
With the upstream change having landed that adds a name to the
previously anonymous "jsmntok" typedef, we can now add a forward
declaration for that struct in our code. As a result, we no longer have
to store the "tokens" member of "struct grub_json" as a void pointer but
can instead use the forward declaration, allowing us to get rid of casts
of that field.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-04-21 22:16:41 +02:00
Patrick Steinhardt
3b81607b55 json: Update jsmn library to upstream commit 053d3cd
Update our embedded version of the jsmn library to upstream commit
053d3cd (Merge pull request #175 from pks-t/pks/struct-type,
2020-04-02).

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-04-21 22:15:14 +02:00
Hans de Goede
12341958d2 kern/term: Accept ESC, F4 and holding SHIFT as user interrupt keys
On some devices the ESC key is the hotkey to enter the BIOS/EFI setup
screen, making it really hard to time pressing it right. Besides that
ESC is also pretty hard to discover for a user who does not know it
will unhide the menu.

This commit makes F4, which was chosen because is not used as a hotkey
to enter the BIOS setup by any vendor, also interrupt sleeps / stop the
menu countdown.

This solves the ESC gets into the BIOS setup and also somewhat solves
the discoverability issue, but leaves the timing issue unresolved.

This commit fixes the timing issue by also adding support for keeping
SHIFT pressed during boot to stop the menu countdown. This matches
what Ubuntu is doing, which should also help with discoverability.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-04-21 22:13:44 +02:00
Hans de Goede
2d7c3abd87 efi/console: Do not set text-mode until we actually need it
If we're running with a hidden menu we may never need text mode, so do not
change the video-mode to text until we actually need it.

This allows to boot a machine without unnecessary graphical transitions and
provide a seamless boot experience to users.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-04-21 22:13:14 +02:00
Hans de Goede
ea138d11be efi/console: Implement getkeystatus() support
Implement getkeystatus() support in the EFI console driver.

This is needed because the logic to determine if a key was pressed to make
the menu countdown stop will be changed by a later patch to also take into
account the SHIFT key being held down.

For this reason the EFI console driver has to support getkeystatus() to
allow detecting that event.

Note that if a non-modifier key gets pressed and repeated calls to
getkeystatus() are made then it will return the modifier status at the
time of the non-modifier key, until that key-press gets consumed by a
getkey() call.

This is a side-effect of how the EFI simple-text-input protocol works
and cannot be avoided.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-04-21 22:12:50 +02:00
Hans de Goede
8bab36af60 efi/console: Add grub_console_read_key_stroke() helper function
This is a preparatory patch for adding getkeystatus() support to the
EFI console driver.

We can get modifier status through the simple_text_input read_key_stroke()
method, but if a non-modifier key is (also) pressed the read_key_stroke()
call will consume that key from the firmware's queue.

The new grub_console_read_key_stroke() helper buffers upto 1 key-stroke.
If it has a non-modifier key buffered, it will return that one, if its
buffer is empty, it will fills its buffer by getting a new key-stroke.

If called with consume=1 it will empty its buffer after copying the
key-data to the callers buffer, this is how getkey() will use it.

If called with consume=0 it will keep the last key-stroke buffered, this
is how getkeystatus() will call it. This means that if a non-modifier
key gets pressed, repeated getkeystatus() calls will return the modifiers
of that key-press until it is consumed by a getkey() call.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-04-21 22:11:17 +02:00
Hans de Goede
5bcdf67642 kern/term: Make grub_getkeystatus() helper function available everywhere
Move grub_getkeystatushelper() function from grub-core/commands/keystatus.c
to grub-core/kern/term.c and export it so that it can be used outside of
the keystatus command code too.

There's no logic change in this patch. The function definition is moved so
it can be called from grub-core/kern/term.c in a subsequent patch. It will
be used to determine if a SHIFT key has was held down and use that also to
interrupt the countdown, without the need to press a key at the right time.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-04-21 22:08:52 +02:00