Since commit 7ce3259f67 (video/fb/fbfill: Fix potential integer
overflow), clang builds of grub-emu have failed with messages like:
/usr/bin/ld: libgrubmods.a(libgrubmods_a-fbfill.o): in function `grub_video_fbfill_direct24':
fbfill.c:(.text+0x28e): undefined reference to `__muloti4'
This appears to be due to a weird quirk in how clang compiles
grub_mul(dst->mode_info->bytes_per_pixel, width, &rowskip)
which is grub_mul(unsigned int, int, &grub_size_t).
It looks like clang somewhere promotes everything to 128-bit maths
before ultimately reducing down to 64 bit for grub_size_t. I think
this is because width is signed, and indeed converting width to an
unsigned int makes the problem go away.
This conversion also makes more sense generally:
- the caller of all the fbfill_directN functions is
grub_video_fb_fill_dispatch() and it takes width and height as
unsigned ints already,
- it doesn't make sense to fill a negative width or height.
Convert the width and height arguments and associated loop counters
to unsigned ints.
Fixes: 7ce3259f67 (video/fb/fbfill: Fix potential integer overflow)
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
When a start of stream marker is encountered, we call grub_jpeg_decode_sos()
which allocates space for a bitmap.
When a restart marker is encountered, we call grub_jpeg_decode_data() which
then fills in that bitmap.
If we get a restart marker before the start of stream marker, we will
attempt to write to a bitmap_ptr that hasn't been allocated. Catch this
and bail out. This fixes an attempt to write to NULL.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The key line is:
du[jpeg_zigzag_order[pos]] = val * (int) data->quan_table[qt][pos];
jpeg_zigzag_order is grub_uint8_t[64].
I don't understand JPEG decoders quite well enough to explain what's
going on here. However, I observe sometimes pos=64, which leads to an
OOB read of the jpeg_zigzag_order global then an OOB write to du.
That leads to various unpleasant memory corruption conditions.
Catch where pos >= ARRAY_SIZE(jpeg_zigzag_order) and bail.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Our decoder only supports 2 quantization tables. If a file asks for
a quantization table with index > 1, reject it.
Similarly, our decoder only supports 4 Huffman tables. If a file asks
for a Huffman table with index > 3, reject it.
This fixes some out of bounds reads. It's not clear what degree of control
over subsequent execution could be gained by someone who can carefully
set up the contents of memory before loading an invalid JPEG file.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
While it may never happen, and potentially could be caught at the end of
the function, it is worth checking up front for a bad reference to the
next marker just in case of a maliciously crafted file being provided.
Fixes: CID 73694
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
It is minimal possibility that the values being used here will overflow.
So, change the code to use the safemath function grub_mul() to ensure
that doesn't happen.
Fixes: CID 73761
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The calculation of the unsigned 64-bit value is being generated by
multiplying 2, signed or unsigned, 32-bit integers which may overflow
before promotion to unsigned 64-bit. Fix all of them.
Fixes: CID 73703, CID 73767, CID 73833
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The multiplication of 2 unsigned 32-bit integers may overflow before
promotion to unsigned 64-bit. We should ensure that the multiplication
is done with overflow detection. Additionally, use grub_sub() for
subtraction.
Fixes: CID 73640, CID 73697, CID 73702, CID 73823
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Signed-off-by: Marco A Benatto <mbenatto@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The return value of grub_video_gop_fill_mode_info() is never able to be
anything other than GRUB_ERR_NONE. So, rather than continue to return
a value and checking it each time, it is more correct to redefine the
function to not return anything and remove checks of its return value
altogether.
Fixes: CID 96701
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This is needed to properly detect and report UEFI Secure Boot status
to the x86 Linux kernel. The functionality will be added by subsequent
patches.
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Marco A Benatto <mbenatto@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
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>
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>
All other video drivers use "video" as the debug condition instead of "fb"
so change this in the efi/uga driver to make it consistent with the others.
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>
We get 64 bit from PCI BAR but then truncate by assigning to 32 bit.
Make sure to check that pointer does not overflow on 32 bit platform.
Closes: 50931
Signed-off-by: Andrei Borzenkov <arvidjaar@gmail.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
EFI GOP has support for multiple different bitness types of frame buffers
and for a special "BLT only" type which is always defined to be RGBx.
Because grub2 doesn't ever directly access the frame buffer but instead
only renders graphics via the BLT interface anyway, we can easily support
these adapters.
The reason this has come up now is the emerging support for virtio-gpu
in OVMF. That adapter does not have the notion of a memory mapped frame
buffer and thus is BLT only.
Signed-off-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
On UEFI, 'text' gfxpayload is not supported, but we still reach parse_modespec()
with it, which will obviously fail. Fortunately, whatever gfxpayload is set,
we still still have the 'auto' default to fall back to. Allow getting to this
fallback by not trying to parse 'text' as a modespec.
This is because 'text' correctly doesn't parse as a modespec, and ought to have
been ignored before we got to that point, just like it is immediately picked if
we're running on a system where 'text' is a supported video mode.
Bug: https://savannah.gnu.org/bugs/index.php?56217
Signed-off-by: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Let's provide file type info to the I/O layer. This way verifiers
framework and its users will be able to differentiate files and verify
only required ones.
This is preparatory patch.
Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Ross Philipson <ross.philipson@oracle.com>
A GPU inserted into a PCIe I/O slot disappears during system startup.
The problem centers around GRUB and a specific VGA init function in
efi_uga.c. This causes an LER (Link Error Recorvery) because the MMIO
memory has not been enabled before attempting access.
The fix is to add the same coding used in other VGA drivers, specifically
to add a check to insure that it is indeed a VGA controller. And then
enable the MMIO address space with the specific bits.
Signed-off-by: Mike Travis <mike.travis@hpe.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In util/getroot and efidisk slightly modify exitsing comment to mostly
retain it but still make GCC7 compliant with respect to fall through
annotation.
In grub-core/lib/xzembed/xz_dec_lzma2.c it adds same comments as
upstream.
In grub-core/tests/setjmp_tets.c declare functions as "noreturn" to
suppress GCC7 warning.
In grub-core/gnulib/regexec.c use new __attribute__, because existing
annotation is not recognized by GCC7 parser (which requires that comment
immediately precedes case statement).
Otherwise add FALLTHROUGH comment.
Closes: 50598
In file included from ./include/grub/dl.h:23:0,
from grub-core/lib/libgcrypt-grub/cipher/rfc2268.c:3:
./include/grub/list.h:34:18: warning: conflicting types for 'grub_list_push' [en
abled by default]
void EXPORT_FUNC(grub_list_push) (grub_list_t *head, grub_list_t item);
^
./include/grub/symbol.h:68:25: note: in definition of macro 'EXPORT_FUNC'
# define EXPORT_FUNC(x) x
^
In file included from ./include/grub/fs.h:30:0,
from ./include/grub/file.h:25,
from ./grub-core/lib/posix_wrap/stdio.h:23,
from c:\mingw\include\libintl.h:314,
from ./include/grub/i18n.h:33,
from ./include/grub/misc.h:27,
from ./include/grub/list.h:25,
from ./include/grub/dl.h:28,
from grub-core/lib/libgcrypt-grub/cipher/rfc2268.c:3:
./include/grub/partition.h:106:3: note: previous implicit declaration of 'grub_l
ist_push' was here
grub_list_push (GRUB_AS_LIST_P (&grub_partition_map_list),
^
list.h needs just ATTRIBUTE_ERROR from misc.h; split compiler features
into separate file grub/compiler.h and include it instead.
Elyes Haouas