Commit Graph
7 Commits
Author SHA1 Message Date
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
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
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
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