Commit Graph

14719 Commits

Author SHA1 Message Date
Mathieu Trudel-Lapierre
41fd55afdc tpm: Pass unknown error as non-fatal, but debug print the error we got
Signed-off-by: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>

Bug-Debian: https://bugs.debian.org/940911
Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/1848892

Patch-Name: tpm-unknown-error-non-fatal.patch
2021-07-10 22:32:00 +01:00
Colin Watson
164dc5e3a7 releasing package grub2 version 2.04-19 2021-06-19 13:04:53 +01:00
Colin Watson
ee935bb07b Resync grub-install backup and restore patches from upstream
This fixes problems that left the system unbootable after certain kinds
of failure.

Closes: #983435
2021-06-14 00:47:16 +01:00
Javier Martinez Canillas
639560699c util/mkimage: Some fixes to PE binaries section size calculation
Commit f60ba9e594 (util/mkimage: Refactor section setup to use a helper)
added a helper function to setup PE sections, but it caused regressions
in some arches where the natural alignment lead to wrong section sizes.

This patch fixes a few things that were caused the section sizes to be
calculated wrongly. These fixes are:

 * Only align the virtual memory addresses but not the raw data offsets.
 * Use aligned sizes for virtual memory sizes but not for raw data sizes.
 * Always align the sizes to set the virtual memory sizes.

These seems to not cause problems for x64 and aa64 EFI platforms but was
a problem for ia64. Because the size of the ".data" and "mods" sections
were wrong and didn't have the correct content. Which lead to GRUB not
being able to load any built-in module.

Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

Bug-Debian: https://bugs.debian.org/987103

Patch-Name: mkimage-fix-section-sizes.patch
2021-06-14 00:40:46 +01:00
Steve McIntyre
a299d76e06 Add debug to display what's going on with verifiers
Patch-Name: debug_verifiers.patch
2021-06-14 00:40:46 +01:00
Steve McIntyre
dd50925e50 Enable shim_lock and tpm modules for all efi platforms, not just x86_64_efi
Patch-Name: enable_shim_lock_i386_efi.patch
2021-06-14 00:40:46 +01:00
Michael Chang
9ab1fc38fb i386-pc: build verifiers API as module
Given no core functions on i386-pc would require verifiers to work and
the only consumer of the verifier API is the pgp module, it looks good
to me that we can move the verifiers out of the kernel image and let
moddep.lst to auto-load it when pgp is loaded on i386-pc platform.

This helps to reduce the size of core image and thus can relax the
tension of exploding on some i386-pc system with very short MBR gap
size. See also a very comprehensive summary from Colin [1] about the
details.

[1] https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00240.html

V2:
Drop COND_NOT_i386_pc and use !COND_i386_pc.
Add comment in kern/verifiers.c to help understanding what's going on
without digging into the commit history.

Reported-by: Colin Watson <cjwatson@debian.org>
Reviewed-by: Colin Watson <cjwatson@debian.org>
Signed-off-by: Michael Chang <mchang@suse.com>

Origin: other, https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00251.html
Bug-Debian: https://bugs.debian.org/984488
Bug-Debian: https://bugs.debian.org/985374
Last-Update: 2021-03-18

Patch-Name: pc-verifiers-module.patch
2021-06-14 00:40:46 +01:00
Marco A Benatto
33e1379aee kern/mm: Fix grub_debug_calloc() compilation error
Fix compilation error due to missing parameter to
grub_printf() when MM_DEBUG is defined.

Fixes: 64e26162e (calloc: Make sure we always have an overflow-checking calloc() available)

Signed-off-by: Marco A Benatto <mbenatto@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/113-kern-mm-Fix-grub_debug_calloc-compilation-error.patch
2021-06-14 00:40:46 +01:00
Thomas Frauendorfer | Miray Software
057832d370 gfxmenu/gui: Check printf() format in the gui_progress_bar and gui_label
The gui_progress_bar and gui_label components can display the timeout
value. The format string can be set through a theme file. This patch
adds a validation step to the format string.

If a user loads a theme file into the GRUB without this patch then
a GUI label with the following settings

  + label {
  ...
  id = "__timeout__"
  text = "%s"
  }

will interpret the current timeout value as string pointer and print the
memory at that position on the screen. It is not desired behavior.

Signed-off-by: Thomas Frauendorfer | Miray Software <tf@miray.de>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/112-gfxmenu-gui-Check-printf-format-in-the-gui_progress_bar-and-gui_label.patch
2021-06-14 00:40:46 +01:00
Thomas Frauendorfer | Miray Software
918f6b2ef2 kern/misc: Add function to check printf() format against expected format
The grub_printf_fmt_check() function parses the arguments of an untrusted
printf() format and an expected printf() format and then compares the
arguments counts and arguments types. The arguments count in the untrusted
format string must be less or equal to the arguments count in the expected
format string and both arguments types must match.

To do this the parse_printf_arg_fmt() helper function is extended in the
following way:

  1. Add a return value to report errors to the grub_printf_fmt_check().

  2. Add the fmt_check argument to enable stricter format verification:
     - the function expects that arguments definitions are always
       terminated by a supported conversion specifier.
     - positional parameters, "$", are not allowed, as they cannot be
       validated correctly with the current implementation. For example
       "%s%1$d" would assign the first args entry twice while leaving the
       second one unchanged.
     - Return an error if preallocated space in args is too small and
       allocation fails for the needed size. The grub_printf_fmt_check()
       should verify all arguments. So, if validation is not possible for
       any reason it should return an error.
     This also adds a case entry to handle "%%", which is the escape
     sequence to print "%" character.

  3. Add the max_args argument to check for the maximum allowed arguments
     count in a printf() string. This should be set to the arguments count
     of the expected format. Then the parse_printf_arg_fmt() function will
     return an error if the arguments count is exceeded.

The two additional arguments allow us to use parse_printf_arg_fmt() in
printf() and grub_printf_fmt_check() calls.

When parse_printf_arg_fmt() is used by grub_printf_fmt_check() the
function parse user provided untrusted format string too. So, in
that case it is better to be too strict than too lenient.

Signed-off-by: Thomas Frauendorfer | Miray Software <tf@miray.de>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/111-kern-misc-Add-function-to-check-printf-format-against-expected-format.patch
2021-06-14 00:40:46 +01:00
Thomas Frauendorfer | Miray Software
9b8d2f2fe3 kern/misc: Add STRING type for internal printf() format handling
Set printf() argument type for "%s" to new type STRING. This is in
preparation for a follow up patch to compare a printf() format string
against an expected printf() format string.

For "%s" the corresponding printf() argument is dereferenced as pointer
while all other argument types are defined as integer value. However,
when validating a printf() format it is necessary to differentiate "%s"
from "%p" and other integers. So, let's do that.

Signed-off-by: Thomas Frauendorfer | Miray Software <tf@miray.de>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/110-kern-misc-Add-STRING-type-for-internal-printf-format-handling.patch
2021-06-14 00:40:46 +01:00
Thomas Frauendorfer | Miray Software
563dd41fc3 kern/misc: Split parse_printf_args() into format parsing and va_list handling
This patch is preparing for a follow up patch which will use
the format parsing part to compare the arguments in a printf()
format from an external source against a printf() format with
expected arguments.

Signed-off-by: Thomas Frauendorfer | Miray Software <tf@miray.de>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/109-kern-misc-Split-parse_printf_args-into-format-parsing-and-va_list-handling.patch
2021-06-14 00:40:46 +01:00
Dimitri John Ledkov
3585284d69 grub-install-common: Add --sbat option
Signed-off-by: Dimitri John Ledkov <xnox@ubuntu.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/108-grub-install-common-Add-sbat-option.patch
2021-06-14 00:40:46 +01:00
Peter Jones
6502497671 util/mkimage: Add an option to import SBAT metadata into a .sbat section
Add a --sbat option to the grub-mkimage tool which allows us to import
an SBAT metadata formatted as a CSV file into a .sbat section of the
EFI binary.

Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/107-util-mkimage-Add-an-option-to-import-SBAT-metadata-into-a-.sbat-section.patch
2021-06-14 00:40:46 +01:00
Peter Jones
ba21811ddd util/mkimage: Refactor section setup to use a helper
Add a init_pe_section() helper function to setup PE sections. This makes
the code simpler and easier to read.

Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/106-util-mkimage-Refactor-section-setup-to-use-a-helper.patch
2021-06-14 00:40:46 +01:00
Peter Jones
73a3a2df1a util/mkimage: Improve data_size value calculation
According to "Microsoft Portable Executable and Common Object File Format
Specification", the Optional Header SizeOfInitializedData field contains:

  Size of the initialized data section, or the sum of all such sections if
  there are multiple data sections.

Make this explicit by adding the GRUB kernel data size to the sum of all
the modules sizes. The ALIGN_UP() is not required by the PE spec but do
it to avoid alignment issues.

Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/105-util-mkimage-Improve-data_size-value-calculation.patch
2021-06-14 00:40:46 +01:00
Peter Jones
aa9b25913c util/mkimage: Reorder PE optional header fields set-up
This makes the PE32 and PE32+ header fields set-up easier to follow by
setting them closer to the initialization of their related sections.

Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/104-util-mkimage-Reorder-PE-optional-header-fields-set-up.patch
2021-06-14 00:40:46 +01:00
Peter Jones
ae1f7bfbae util/mkimage: Unify more of the PE32 and PE32+ header set-up
There's quite a bit of code duplication in the code that sets the optional
header for PE32 and PE32+. The two are very similar with the exception of
a few fields that have type grub_uint64_t instead of grub_uint32_t.

Factor out the common code and add a PE_OHDR() macro that simplifies the
set-up and make the code more readable.

Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/103-util-mkimage-Unify-more-of-the-PE32-and-PE32-header-set-up.patch
2021-06-14 00:40:46 +01:00
Peter Jones
8b7ccebbf3 util/mkimage: Always use grub_host_to_target32() to initialize PE stack and heap stuff
This change does not impact final result of initialization itself.
However, it eases PE code unification in subsequent patches.

Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/102-util-mkimage-Always-use-grub_host_to_target32-to-initialize-PE-stack-and-heap-stuff.patch
2021-06-14 00:40:46 +01:00
Peter Jones
e6f6175cdb util/mkimage: Use grub_host_to_target32() instead of grub_cpu_to_le32()
The latter doesn't take into account the target image endianness. There is
a grub_cpu_to_le32_compile_time() but no compile time variant for function
grub_host_to_target32(). So, let's keep using the other one for this case.

Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/101-util-mkimage-Use-grub_host_to_target32-instead-of-grub_cpu_to_le32.patch
2021-06-14 00:40:46 +01:00
Javier Martinez Canillas
d0d8d98f77 util/mkimage: Remove unused code to add BSS section
The code is compiled out so there is no reason to keep it.

Additionally, don't set bss_size field since we do not add a BSS section.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/100-util-mkimage-Remove-unused-code-to-add-BSS-section.patch
2021-06-14 00:40:46 +01:00
Chris Coulson
3a2c3a451b kern/efi: Add initial stack protector implementation
It works only on UEFI platforms but can be quite easily extended to
others architectures and platforms if needed.

Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Marco A Benatto <mbenatto@redhat.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Patch-Name: 2021-02-security/099-kern-efi-Add-initial-stack-protector-implementation.patch
2021-06-14 00:40:46 +01:00
Chris Coulson
e44c6dbbb5 kern/parser: Fix a stack buffer overflow
grub_parser_split_cmdline() expands variable names present in the supplied
command line in to their corresponding variable contents and uses a 1 kiB
stack buffer for temporary storage without sufficient bounds checking. If
the function is called with a command line that references a variable with
a sufficiently large payload, it is possible to overflow the stack
buffer via tab completion, corrupt the stack frame and potentially
control execution.

Fixes: CVE-2020-27749

Reported-by: Chris Coulson <chris.coulson@canonical.com>
Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/098-kern-parser-Fix-a-stack-buffer-overflow.patch
2021-06-14 00:40:46 +01:00
Chris Coulson
4f38583d80 kern/buffer: Add variable sized heap buffer
Add a new variable sized heap buffer type (grub_buffer_t) with simple
operations for appending data, accessing the data and maintaining
a read cursor.

Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/097-kern-buffer-Add-variable-sized-heap-buffer.patch
2021-06-14 00:40:46 +01:00
Chris Coulson
d5d6b0d72e kern/parser: Refactor grub_parser_split_cmdline() cleanup
Introduce a common function epilogue used for cleaning up on all
return paths, which will simplify additional error handling to be
introduced in a subsequent commit.

Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/096-kern-parser-Refactor-grub_parser_split_cmdline-cleanup.patch
2021-06-14 00:40:46 +01:00
Chris Coulson
a93ec88ea6 kern/parser: Introduce terminate_arg() helper
process_char() and grub_parser_split_cmdline() use similar code for
terminating the most recent argument. Add a helper function for this.

Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/095-kern-parser-Introduce-terminate_arg-helper.patch
2021-06-14 00:40:46 +01:00
Chris Coulson
45812770ce kern/parser: Introduce process_char() helper
grub_parser_split_cmdline() iterates over each command line character.
In order to add error checking and to simplify the subsequent error
handling, split the character processing in to a separate function.

Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/094-kern-parser-Introduce-process_char-helper.patch
2021-06-14 00:40:46 +01:00
Chris Coulson
8d06222bb8 kern/parser: Fix a memory leak
The getline() function supplied to grub_parser_split_cmdline() returns
a newly allocated buffer and can be called multiple times, but the
returned buffer is never freed.

Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/093-kern-parser-Fix-a-memory-leak.patch
2021-06-14 00:40:46 +01:00
Daniel Axtens
288dc4d1b2 fs/btrfs: Squash some uninitialized reads
We need to check errors before calling into a function that uses the result.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/092-fs-btrfs-Squash-some-uninitialized-reads.patch
2021-06-14 00:40:46 +01:00
Daniel Axtens
53b296ab82 fs/btrfs: Validate the number of stripes/parities in RAID5/6
This prevents a divide by zero if nstripes == nparities, and
also prevents propagation of invalid values if nstripes ends up
less than nparities.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/091-fs-btrfs-Validate-the-number-of-stripes-parities-in-RAID5-6.patch
2021-06-14 00:40:46 +01:00
Daniel Axtens
a375ddc310 disk/lvm: Do not allow a LV to be it's own segment's node's LV
This prevents infinite recursion in the diskfilter verification code.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/090-disk-lvm-Do-not-allow-a-LV-to-be-it-s-own-segment-s-node-s-LV.patch
2021-06-14 00:40:46 +01:00
Daniel Axtens
0155cb57f0 disk/lvm: Sanitize rlocn->offset to prevent wild read
rlocn->offset is read directly from disk and added to the metadatabuf
pointer to create a pointer to a block of metadata. It's a 64-bit
quantity so as long as you don't overflow you can set subsequent
pointers to point anywhere in memory.

Require that rlocn->offset fits within the metadata buffer size.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/089-disk-lvm-Sanitize-rlocn-offset-to-prevent-wild-read.patch
2021-06-14 00:40:46 +01:00
Daniel Axtens
77393c17ac disk/lvm: Do not overread metadata
We could reach the end of valid metadata and not realize, leading to
some buffer overreads. Check if we have reached the end and bail.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/088-disk-lvm-Do-not-overread-metadata.patch
2021-06-14 00:40:46 +01:00
Daniel Axtens
b281405c38 disk/lvm: Do not crash if an expected string is not found
Clean up a bunch of cases where we could have strstr() fail and lead to
us dereferencing NULL.

We'll still leak memory in some cases (loops don't clean up allocations
from earlier iterations if a later iteration fails) but at least we're
not crashing.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/087-disk-lvm-Do-not-crash-if-an-expected-string-is-not-found.patch
2021-06-14 00:40:46 +01:00
Daniel Axtens
74db816856 disk/lvm: Bail on missing PV list
There's an if block for the presence of "physical_volumes {", but if
that block is absent, then p remains NULL and a NULL-deref will result
when looking for logical volumes.

It doesn't seem like LVM makes sense without physical volumes, so error
out rather than crashing.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/086-disk-lvm-Bail-on-missing-PV-list.patch
2021-06-14 00:40:46 +01:00
Daniel Axtens
74265556a6 disk/lvm: Don't blast past the end of the circular metadata buffer
This catches at least some OOB reads, and it's possible I suppose that
if 2 * mda_size is less than GRUB_LVM_MDA_HEADER_SIZE it might catch some
OOB writes too (although that hasn't showed up as a crash in fuzzing yet).

It's a bit ugly and I'd appreciate better suggestions.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/085-disk-lvm-Don-t-blast-past-the-end-of-the-circular-metadata-buffer.patch
2021-06-14 00:40:46 +01:00
Daniel Axtens
948755171e disk/lvm: Don't go beyond the end of the data we read from disk
We unconditionally trusted offset_xl from the LVM label header, even if
it told us that the PV header/disk locations were way off past the end
of the data we read from disk.

Require that the offset be sane, fixing an OOB read and crash.

Fixes: CID 314367, CID 314371

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/084-disk-lvm-Don-t-go-beyond-the-end-of-the-data-we-read-from-disk.patch
2021-06-14 00:40:46 +01:00
Daniel Axtens
949888bfa4 io/gzio: Zero gzio->tl/td in init_dynamic_block() if huft_build() fails
If huft_build() fails, gzio->tl or gzio->td could contain pointers that
are no longer valid. Zero them out.

This prevents a double free when grub_gzio_close() comes through and
attempts to free them again.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/083-io-gzio-Zero-gzio-tl-td-in-init_dynamic_block-if-huft_build-fails.patch
2021-06-14 00:40:46 +01:00
Daniel Axtens
4bea24b4a0 io/gzio: Catch missing values in huft_build() and bail
In huft_build(), "v" is a table of values in order of bit length.
The code later (when setting up table entries in "r") assumes that all
elements of this array corresponding to a code are initialized and less
than N_MAX. However, it doesn't enforce this.

With sufficiently manipulated inputs (e.g. from fuzzing), there can be
elements of "v" that are not filled. Therefore a lookup into "e" or "d"
will use an uninitialized value. This can lead to an invalid/OOB read on
those values, often leading to a crash.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/082-io-gzio-Catch-missing-values-in-huft_build-and-bail.patch
2021-06-14 00:40:46 +01:00
Daniel Axtens
5d3a0f0c42 io/gzio: Add init_dynamic_block() clean up if unpacking codes fails
init_dynamic_block() didn't clean up gzio->tl and td in some error
paths. This left td pointing to part of tl. Then in grub_gzio_close(),
when tl was freed the storage for td would also be freed. The code then
attempts to free td explicitly, performing a UAF and then a double free.

Explicitly clean up tl and td in the error paths.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/081-io-gzio-Add-init_dynamic_block-clean-up-if-unpacking-codes-fails.patch
2021-06-14 00:40:46 +01:00
Daniel Axtens
7ad63f14e6 io/gzio: Bail if gzio->tl/td is NULL
This is an ugly fix that doesn't address why gzio->tl comes to be NULL.
However, it seems to be sufficient to patch up a bunch of NULL derefs.

It would be good to revisit this in future and see if we can have
a cleaner solution that addresses some of the causes of the unexpected
NULL pointers.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/080-io-gzio-Bail-if-gzio-tl-td-is-NULL.patch
2021-06-14 00:40:46 +01:00
Daniel Axtens
6bd6187cd0 fs/nilfs2: Properly bail on errors in grub_nilfs2_btree_node_lookup()
We just introduced an error return in grub_nilfs2_btree_node_lookup().
Make sure the callers catch it.

At the same time, make sure that grub_nilfs2_btree_node_lookup() always
inits the index pointer passed to it.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/079-fs-nilfs2-Properly-bail-on-errors-in-grub_nilfs2_btree_node_lookup.patch
2021-06-14 00:40:46 +01:00
Daniel Axtens
85c7e7be6c fs/nilfs2: Don't search children if provided number is too large
NILFS2 reads the number of children a node has from the node. Unfortunately,
that's not trustworthy. Check if it's beyond what the filesystem permits and
reject it if so.

This blocks some OOB reads. I'm not sure how controllable the read is and what
could be done with invalidly read data later on.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/078-fs-nilfs2-Don-t-search-children-if-provided-number-is-too-large.patch
2021-06-14 00:40:45 +01:00
Daniel Axtens
d8441c7237 fs/nilfs2: Reject too-large keys
NILFS2 has up to 7 keys, per the data structure. Do not permit array
indices in excess of that.

This catches some OOB reads. I don't know how controllable the invalidly
read data is or if that could be used later in the program.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/077-fs-nilfs2-Reject-too-large-keys.patch
2021-06-14 00:40:45 +01:00
Daniel Axtens
a7d4ea3e53 fs/jfs: Catch infinite recursion
It's possible with a fuzzed filesystem for JFS to keep getblk()-ing
the same data over and over again, leading to stack exhaustion.

Check if we'd be calling the function with exactly the same data as
was passed in, and if so abort.

I'm not sure what the performance impact of this is and am open to
better ideas.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/076-fs-jfs-Catch-infinite-recursion.patch
2021-06-14 00:40:45 +01:00
Daniel Axtens
46456abd3f fs/jfs: Limit the extents that getblk() can consider
getblk() implicitly trusts that treehead->count is an accurate count of
the number of extents. However, that value is read from disk and is not
trustworthy, leading to OOB reads and crashes. I am not sure to what
extent the data read from OOB can influence subsequent program execution.

Require callers to pass in the maximum number of extents for which
they have storage.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/075-fs-jfs-Limit-the-extents-that-getblk-can-consider.patch
2021-06-14 00:40:45 +01:00
Daniel Axtens
f89b184a61 fs/jfs: Do not move to leaf level if name length is negative
Fuzzing JFS revealed crashes where a negative number would be passed
to le_to_cpu16_copy(). There it would be cast to a large positive number
and the copy would read and write off the end of the respective buffers.

Catch this at the top as well as the bottom of the loop.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/074-fs-jfs-Do-not-move-to-leaf-level-if-name-length-is-negative.patch
2021-06-14 00:40:45 +01:00
Daniel Axtens
45a1b74ac3 fs/sfs: Fix over-read of root object name
There's a read of the name of the root object that assumes that the name
is nul-terminated within the root block. This isn't guaranteed - it seems
SFS would require you to read multiple blocks to get a full name in general,
but maybe that doesn't apply to the root object.

Either way, figure out how much space is left in the root block and don't
over-read it. This fixes some OOB reads.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/073-fs-sfs-Fix-over-read-of-root-object-name.patch
2021-06-14 00:40:45 +01:00
Daniel Axtens
c80196584b fs/hfs: Disable under lockdown
HFS has issues such as infinite mutual recursion that are simply too
complex to fix for such a legacy format. So simply do not permit
it to be loaded under lockdown.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/072-fs-hfs-Disable-under-lockdown.patch
2021-06-14 00:40:45 +01:00
Daniel Axtens
c85530c2ca fs/hfsplus: Don't use uninitialized data on corrupt filesystems
Valgrind identified the following use of uninitialized data:

  ==2782220== Conditional jump or move depends on uninitialised value(s)
  ==2782220==    at 0x42B364: grub_hfsplus_btree_search (hfsplus.c:566)
  ==2782220==    by 0x42B21D: grub_hfsplus_read_block (hfsplus.c:185)
  ==2782220==    by 0x42A693: grub_fshelp_read_file (fshelp.c:386)
  ==2782220==    by 0x42C598: grub_hfsplus_read_file (hfsplus.c:219)
  ==2782220==    by 0x42C598: grub_hfsplus_mount (hfsplus.c:330)
  ==2782220==    by 0x42B8C5: grub_hfsplus_dir (hfsplus.c:958)
  ==2782220==    by 0x4C1AE6: grub_fs_probe (fs.c:73)
  ==2782220==    by 0x407C94: grub_ls_list_files (ls.c:186)
  ==2782220==    by 0x407C94: grub_cmd_ls (ls.c:284)
  ==2782220==    by 0x4D7130: grub_extcmd_dispatcher (extcmd.c:55)
  ==2782220==    by 0x4045A6: execute_command (grub-fstest.c:59)
  ==2782220==    by 0x4045A6: fstest (grub-fstest.c:433)
  ==2782220==    by 0x4045A6: main (grub-fstest.c:772)
  ==2782220==  Uninitialised value was created by a heap allocation
  ==2782220==    at 0x483C7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
  ==2782220==    by 0x4C0305: grub_malloc (mm.c:42)
  ==2782220==    by 0x42C21D: grub_hfsplus_mount (hfsplus.c:239)
  ==2782220==    by 0x42B8C5: grub_hfsplus_dir (hfsplus.c:958)
  ==2782220==    by 0x4C1AE6: grub_fs_probe (fs.c:73)
  ==2782220==    by 0x407C94: grub_ls_list_files (ls.c:186)
  ==2782220==    by 0x407C94: grub_cmd_ls (ls.c:284)
  ==2782220==    by 0x4D7130: grub_extcmd_dispatcher (extcmd.c:55)
  ==2782220==    by 0x4045A6: execute_command (grub-fstest.c:59)
  ==2782220==    by 0x4045A6: fstest (grub-fstest.c:433)
  ==2782220==    by 0x4045A6: main (grub-fstest.c:772)

This happens when the process of reading the catalog file goes sufficiently
wrong that there's an attempt to read the extent overflow file, which has
not yet been loaded. Keep track of when the extent overflow file is
fully loaded and refuse to use it before then.

The load valgrind doesn't like is btree->nodesize, and that's then used
to allocate a data structure. It looks like there are subsequently a lot
of reads based on that pointer so OOB reads are likely, and indeed crashes
(albeit difficult-to-replicate ones) have been observed in fuzzing.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Patch-Name: 2021-02-security/071-fs-hfsplus-Don-t-use-uninitialized-data-on-corrupt-filesystems.patch
2021-06-14 00:40:45 +01:00