4308 Commits

Author SHA1 Message Date
Sudhakar Kuppusamy
deae293f39 fs/f2fs: Do not read past the end of nat bitmap
A corrupt f2fs filesystem could have a block offset or a bitmap
offset that would cause us to read beyond the bounds of the nat
bitmap.

Introduce the nat_bitmap_size member in grub_f2fs_data which holds
the size of nat bitmap.

Set the size when loading the nat bitmap in nat_bitmap_ptr(), and
catch when an invalid offset would create a pointer past the end of
the allocated space.

Check against the bitmap size in grub_f2fs_test_bit() test bit to avoid
reading past the end of the nat bitmap.

Signed-off-by: Sudhakar Kuppusamy <sudhakar@linux.ibm.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:33 +02:00
Sudhakar Kuppusamy
4bd9877f62 fs/f2fs: Do not read past the end of nat journal entries
A corrupt f2fs file system could specify a nat journal entry count
that is beyond the maximum NAT_JOURNAL_ENTRIES.

Check if the specified nat journal entry count before accessing the
array, and throw an error if it is too large.

Signed-off-by: Sudhakar Kuppusamy <sudhakar@linux.ibm.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:33 +02:00
Daniel Axtens
b26b4c08e7 net/http: Error out on headers with LF without CR
In a similar vein to the previous patch, parse_line() would write
a NUL byte past the end of the buffer if there was an HTTP header
with a LF rather than a CRLF.

RFC-2616 says:

  Many HTTP/1.1 header field values consist of words separated by LWS
  or special characters. These special characters MUST be in a quoted
  string to be used within a parameter value (as defined in section 3.6).

We don't support quoted sections or continuation lines, etc.

If we see an LF that's not part of a CRLF, bail out.

Fixes: CVE-2022-28734

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:33 +02:00
Daniel Axtens
ec6bfd3237 net/http: Fix OOB write for split http headers
GRUB has special code for handling an http header that is split
across two packets.

The code tracks the end of line by looking for a "\n" byte. The
code for split headers has always advanced the pointer just past the
end of the line, whereas the code that handles unsplit headers does
not advance the pointer. This extra advance causes the length to be
one greater, which breaks an assumption in parse_line(), leading to
it writing a NUL byte one byte past the end of the buffer where we
reconstruct the line from the two packets.

It's conceivable that an attacker controlled set of packets could
cause this to zero out the first byte of the "next" pointer of the
grub_mm_region structure following the current_line buffer.

Do not advance the pointer in the split header case.

Fixes: CVE-2022-28734

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:33 +02:00
Daniel Axtens
dad94fffe1 net/http: Do not tear down socket if it's already been torn down
It's possible for data->sock to get torn down in tcp error handling.
If we unconditionally tear it down again we will end up doing writes
to an offset of the NULL pointer when we go to tear it down again.

Detect if it has been torn down and don't do it again.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:33 +02:00
Daniel Axtens
8f287c3e13 net/tftp: Avoid a trivial UAF
Under tftp errors, we print a tftp error message from the tftp header.
However, the tftph pointer is a pointer inside nb, the netbuff. Previously,
we were freeing the nb and then dereferencing it. Don't do that, use it
and then free it later.

This isn't really _bad_ per se, especially as we're single-threaded, but
it trips up fuzzers.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:33 +02:00
Daniel Axtens
ee96520314 net/tftp: Prevent a UAF and double-free from a failed seek
A malicious tftp server can cause UAFs and a double free.

An attempt to read from a network file is handled by grub_net_fs_read(). If
the read is at an offset other than the current offset, grub_net_seek_real()
is invoked.

In grub_net_seek_real(), if a backwards seek cannot be satisfied from the
currently received packets, and the underlying transport does not provide
a seek method, then grub_net_seek_real() will close and reopen the network
protocol layer.

For tftp, the ->close() call goes to tftp_close() and frees the tftp_data_t
file->data. The file->data pointer is not nulled out after the free.

If the ->open() call fails, the file->data will not be reallocated and will
continue point to a freed memory block. This could happen from a server
refusing to send the requisite ack to the new tftp request, for example.

The seek and the read will then fail, but the grub_file continues to exist:
the failed seek does not necessarily cause the entire file to be thrown
away (e.g. where the file is checked to see if it is gzipped/lzio/xz/etc.,
a read failure is interpreted as a decompressor passing on the file, not as
an invalidation of the entire grub_file_t structure).

This means subsequent attempts to read or seek the file will use the old
file->data after free. Eventually, the file will be close()d again and
file->data will be freed again.

Mark a net_fs file that doesn't reopen as broken. Do not permit read() or
close() on a broken file (seek is not exposed directly to the file API -
it is only called as part of read, so this blocks seeks as well).

As an additional defence, null out the ->data pointer if tftp_open() fails.
That would have lead to a simple null pointer dereference rather than
a mess of UAFs.

This may affect other protocols, I haven't checked.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:33 +02:00
Daniel Axtens
96abf4fb9d net/dns: Don't read past the end of the string we're checking against
I don't really understand what's going on here but fuzzing found
a bug where we read past the end of check_with. That's a C string,
so use grub_strlen() to make sure we don't overread it.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:33 +02:00
Daniel Axtens
c1b7eef9fa net/dns: Fix double-free addresses on corrupt DNS response
grub_net_dns_lookup() takes as inputs a pointer to an array of addresses
("addresses") for the given name, and pointer to a number of addresses
("naddresses"). grub_net_dns_lookup() is responsible for allocating
"addresses", and the caller is responsible for freeing it if
"naddresses" > 0.

The DNS recv_hook will sometimes set and free the addresses array,
for example if the packet is too short:

      if (ptr + 10 >= nb->tail)
	{
	  if (!*data->naddresses)
	    grub_free (*data->addresses);
	  grub_netbuff_free (nb);
	  return GRUB_ERR_NONE;
	}

Later on the nslookup command code unconditionally frees the "addresses"
array. Normally this is fine: the array is either populated with valid
data or is NULL. But in these sorts of error cases it is neither NULL
nor valid and we get a double-free.

Only free "addresses" if "naddresses" > 0.

It looks like the other use of grub_net_dns_lookup() is not affected.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:33 +02:00
Daniel Axtens
f407e34f38 net/netbuff: Block overly large netbuff allocs
A netbuff shouldn't be too huge. It's bounded by MTU and TCP segment
reassembly. If we are asked to create one that is unreasonably big, refuse.

This is a hardening measure: if we hit this code, there's a bug somewhere
else that we should catch and fix.

This commit:
  - stops the bug propagating any further.
  - provides a spot to instrument in e.g. fuzzing to try to catch these bugs.

I have put instrumentation (e.g. __builtin_trap() to force a crash) here and
have not been able to find any more crashes.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:33 +02:00
Daniel Axtens
3e4817538d net/ip: Do IP fragment maths safely
We can receive packets with invalid IP fragmentation information. This
can lead to rsm->total_len underflowing and becoming very large.

Then, in grub_netbuff_alloc(), we add to this very large number, which can
cause it to overflow and wrap back around to a small positive number.
The allocation then succeeds, but the resulting buffer is too small and
subsequent operations can write past the end of the buffer.

Catch the underflow here.

Fixes: CVE-2022-28733

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:33 +02:00
Daniel Axtens
830a9628b2 normal/charset: Fix array out-of-bounds formatting unicode for display
In some cases attempting to display arbitrary binary strings leads
to ASAN splats reading the widthspec array out of bounds.

Check the index. If it would be out of bounds, return a width of 1.
I don't know if that's strictly correct, but we're not really expecting
great display of arbitrary binary data, and it's certainly not worse than
an OOB read.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:32 +02:00
Daniel Axtens
22a3f97d39 video/readers/jpeg: Block int underflow -> wild pointer write
Certain 1 px wide images caused a wild pointer write in
grub_jpeg_ycrcb_to_rgb(). This was caused because in grub_jpeg_decode_data(),
we have the following loop:

for (; data->r1 < nr1 && (!data->dri || rst);
     data->r1++, data->bitmap_ptr += (vb * data->image_width - hb * nc1) * 3)

We did not check if vb * width >= hb * nc1.

On a 64-bit platform, if that turns out to be negative, it will underflow,
be interpreted as unsigned 64-bit, then be added to the 64-bit pointer, so
we see data->bitmap_ptr jump, e.g.:

0x6180_0000_0480 to
0x6181_0000_0498
     ^
     ~--- carry has occurred and this pointer is now far away from
          any object.

On a 32-bit platform, it will decrement the pointer, creating a pointer
that won't crash but will overwrite random data.

Catch the underflow and error out.

Fixes: CVE-2021-3697

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:32 +02:00
Daniel Axtens
166a4d6144 video/readers/jpeg: Refuse to handle multiple start of streams
An invalid file could contain multiple start of stream blocks, which
would cause us to reallocate and leak our bitmap. Refuse to handle
multiple start of streams.

Additionally, fix a grub_error() call formatting.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:32 +02:00
Daniel Axtens
768ef2199e video/readers/jpeg: Do not reallocate a given huff table
Fix a memory leak where an invalid file could cause us to reallocate
memory for a huffman table we had already allocated memory for.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:32 +02:00
Daniel Axtens
d5caac8ab7 video/readers/jpeg: Abort sooner if a read operation fails
Fuzzing revealed some inputs that were taking a long time, potentially
forever, because they did not bail quickly upon encountering an I/O error.

Try to catch I/O errors sooner and bail out.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:32 +02:00
Daniel Axtens
690bee69fa video/readers/png: Sanity check some huffman codes
ASAN picked up two OOB global reads: we weren't checking if some code
values fit within the cplens or cpdext arrays. Check and throw an error
if not.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:32 +02:00
Daniel Axtens
210245129c video/readers/png: Avoid heap OOB R/W inserting huff table items
In fuzzing we observed crashes where a code would attempt to be inserted
into a huffman table before the start, leading to a set of heap OOB reads
and writes as table entries with negative indices were shifted around and
the new code written in.

Catch the case where we would underflow the array and bail.

Fixes: CVE-2021-3696

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:32 +02:00
Daniel Axtens
e623866d92 video/readers/png: Drop greyscale support to fix heap out-of-bounds write
A 16-bit greyscale PNG without alpha is processed in the following loop:

      for (i = 0; i < (data->image_width * data->image_height);
	   i++, d1 += 4, d2 += 2)
	{
	  d1[R3] = d2[1];
	  d1[G3] = d2[1];
	  d1[B3] = d2[1];
	}

The increment of d1 is wrong. d1 is incremented by 4 bytes per iteration,
but there are only 3 bytes allocated for storage. This means that image
data will overwrite somewhat-attacker-controlled parts of memory - 3 bytes
out of every 4 following the end of the image.

This has existed since greyscale support was added in 2013 in commit
3ccf16dff98f (grub-core/video/readers/png.c: Support grayscale).

Saving starfield.png as a 16-bit greyscale image without alpha in the gimp
and attempting to load it causes grub-emu to crash - I don't think this code
has ever worked.

Delete all PNG greyscale support.

Fixes: CVE-2021-3695

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:32 +02:00
Daniel Axtens
347880a13c video/readers/png: Refuse to handle multiple image headers
This causes the bitmap to be leaked. Do not permit multiple image headers.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:32 +02:00
Daniel Axtens
5bff31cdb6 video/readers/png: Abort sooner if a read operation fails
Fuzzing revealed some inputs that were taking a long time, potentially
forever, because they did not bail quickly upon encountering an I/O error.

Try to catch I/O errors sooner and bail out.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:32 +02:00
Daniel Axtens
f1ce0e15e7 kern/file: Do not leak device_name on error in grub_file_open()
If we have an error in grub_file_open() before we free device_name, we
will leak it.

Free device_name in the error path and null out the pointer in the good
path once we free it there.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:32 +02:00
Julian Andres Klode
6fe755c5c0 kern/efi/sb: Reject non-kernel files in the shim_lock verifier
We must not allow other verifiers to pass things like the GRUB modules.
Instead of maintaining a blocklist, maintain an allowlist of things
that we do not care about.

This allowlist really should be made reusable, and shared by the
lockdown verifier, but this is the minimal patch addressing
security concerns where the TPM verifier was able to mark modules
as verified (or the OpenPGP verifier for that matter), when it
should not do so on shim-powered secure boot systems.

Fixes: CVE-2022-28735

Signed-off-by: Julian Andres Klode <julian.klode@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:31 +02:00
Chris Coulson
04c86e0bb7 loader/efi/chainloader: Use grub_loader_set_ex()
This ports the EFI chainloader to use grub_loader_set_ex() in order to fix
a use-after-free bug that occurs when grub_cmd_chainloader() is executed
more than once before a boot attempt is performed.

Fixes: CVE-2022-28736

Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:31 +02:00
Chris Coulson
14ceb3b3ff commands/boot: Add API to pass context to loader
Loaders rely on global variables for saving context which is consumed
in the boot hook and freed in the unload hook. In the case where a loader
command is executed twice, calling grub_loader_set() a second time executes
the unload hook, but in some cases this runs when the loader's global
context has already been updated, resulting in the updated context being
freed and potential use-after-free bugs when the boot hook is subsequently
called.

This adds a new API, grub_loader_set_ex(), which allows a loader to specify
context that is passed to its boot and unload hooks. This is an alternative
to requiring that loaders call grub_loader_unset() before mutating their
global context.

Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:31 +02:00
Chris Coulson
1469983ebb loader/efi/chainloader: Simplify the loader state
The chainloader command retains the source buffer and device path passed
to LoadImage(), requiring the unload hook passed to grub_loader_set() to
free them. It isn't required to retain this state though - they aren't
required by StartImage() or anything else in the boot hook, so clean them
up before grub_cmd_chainloader() finishes.

Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 16:39:31 +02:00
Jagannathan Raman
91b38780ac fs/zfs/zfs: zfs_mount() - avoid pointer downcasting
Coverity reports that while loopis in the following functions uses
tainted data as boundary:
  zfs_mount() -> check_mos_features() -> dnode_get() -> zfs_log2()
  zfs_mount() -> grub_memmove()

The defect type is "Untrusted loop bound" caused as a result of
"tainted_data_downcast". Coverity does not like the pointer downcast
here and we need to address it.

We believe Coverity flags pointer downcast for the following two
reasons:
1. External data: The pointer downcast could indicate that the source is
  external data, which we need to further sanitize - such as verifying its
  limits. In this case, the data is read from an external source, which is
  a disk. But, zio_read(), which reads the data from the disk, sanitizes it
  using a checksum. checksum is the best facility that ZFS offers to verify
  external data, and we don't believe a better way exists. Therefore, no
  further action is possible for this.

2. Corruption due to alignment: downcasting a pointer from a strict type
  to less strict type could result in data corruption. For example, the
  following cast would corrupt because uint32_t is 4-byte aligned, and
  won't be able to point to 0x1003 which is not 4-byte aligned.
    uint8_t *ptr = 0x1003;
    uint32_t *word = ptr; (incorrect, alignment issues)

  This patch converts the "osp" pointer in zfs_mount() from a "void" type
  to "objset_phys_t" type to address this issue.

We are not sure if there are any other reasons why Coverity flags the
downcast. However, the fix for alignment issue masks/suppresses any
other issues from showing up.

Fixes: CID 314023

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>
2022-06-07 16:39:31 +02:00
Jagannathan Raman
a5cc223e38 fs/zfs/zfs: make_mdn() - avoid pointer downcasting
Coverity reports that the while loop in the following function uses
tainted data as boundary:
  fill_fs_info() -> dnode_get() -> zfs_log2()

The tainted originated from:
  fill_fs_info() -> make_mdn()

The defect type is "Untrusted loop bound" caused as a result of
"tainted_data_downcast". Coverity does not like the pointer downcast
here and we need to address it.

We believe Coverity flags pointer downcast for the following two
reasons:
1. External data: The pointer downcast could indicate that the source is
  external data, which we need to further sanitize - such as verifying its
  limits. In this case, the data is read from an external source, which is
  a disk. But, zio_read(), which reads the data from the disk, sanitizes it
  using a checksum. checksum is the best facility that ZFS offers to verify
  external data, and we don't believe a better way exists. Therefore, no
  further action is possible for this.

2. Corruption due to alignment: downcasting a pointer from a strict type
  to less strict type could result in data corruption. For example, the
  following cast would corrupt because uint32_t is 4-byte aligned, and
  won't be able to point to 0x1003 which is not 4-byte aligned.
    uint8_t *ptr = 0x1003;
    uint32_t *word = ptr; (incorrect, alignment issues)

This patch converts the "osp" pointer in make_mdn() from a "void" type
to "objset_phys_t" type to address the issue.

We are not sure if there are any other reasons why Coverity flags the
downcast. However, the fix for alignment issue masks/suppresses any
other issues from showing up.

Fixes: CID 314020

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>
2022-06-07 16:39:31 +02:00
Alec Brown
253da39c15 grub-core/loader/i386/bsdXX: Avoid downcasting (char *) to (Elf_Shdr *)
In bsdXX.c, a couple of untrusted loop bound and untrusted allocation size bugs
were flagged by Coverity in the functions grub_openbsd_find_ramdisk() and
grub_freebsd_load_elfmodule(). These bugs were flagged by coverity because the
variable shdr was downcasting from a char pointer to an Elf_Shdr pointer
whenever it was used to set the base value in for loops. To avoid this, we need
to set shdr as an Elf_Shdr pointer where it is initialized.

In the function read_headers(), the function is reading elf section header data
from a file and passing it to the variable shdr as data for a char pointer. If
we switch the type of shdr to an Elf_Shdr pointer in read_headers() as well as
other functions, then we won't need to downcast to an Elf_Shdr pointer. By doing
this, the issue becomes masked from Coverity's view. In the following patches,
we check limits to ensure the data isn't tainted.

Also, switched use of (char *) to (grub_uint8_t *) to give a better indication
of pointer arithmetic and not suggest use of a C string.

Fixes: CID 314018
Fixes: CID 314030
Fixes: CID 314031
Fixes: CID 314039

Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
2022-06-07 13:48:23 +02:00
Stefan Agner
858a0745c8 disk/efi/efidisk: Pass buffers with higher alignment
Some devices report IoAlign values but seem to require buffers with
higher alignment.

The UEFI specification is saying: "IoAlign values of 0 and 1 mean that
the buffer can be placed anywhere in memory. Otherwise, IoAlign must
be a power of 2, and the requirement is that the start address of
a buffer must be evenly divisible by IoAlign with no remainder."

Some devices report IoAlign of 2, however seem to require 4 bytes
aligned buffers. It seems that this got misinterpreted by some vendors
assuming IoAlign is 2^IoAlign. There is also such a hint in an example
in earlier versions of the Driver Writer's Guide:

  ScsiPassThruMode.IoAlign = 2; // Data must be alligned on 4-byte boundary

Some devices report no alignment requirements at all but seem to read
corrupted data or report read errors when passing unaligned buffers.

Work around by using an alignment of at least BlockSize (typically 512
bytes) in any case. If IoAlign (interpreted as per UEFI specification)
requests a higher alignment than BlockSize, follow IoAlign still.

Note: The problem has only noticed with compressed squashfs. It seems
that ext4 (and presumably other file system drivers) pass buffers with
a higher alignment already.

Signed-off-by: Stefan Agner <stefan@agner.ch>
Acked-by: Heinrich Schuchardt <heinrich.schuchardt@canaonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 13:40:48 +02:00
Samuel Thibault
e981b0a248 osdep/hurd/getroot: Use "part:" qualifier
When using userland drivers such as rumpdisk, we'd rather make ext2fs use
parted-based libstore partitioning support. That can be used for kernelland
drivers as well, so we can just make GRUB always use the "part:" qualifier
to switch ext2fs to it.

grub_util_find_hurd_root_device() then has to understand this syntax and
translate it into the /dev/ entry name.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 13:31:33 +02:00
Glenn Washburn
af81ecede1 disk/cryptodisk: Use enum constants as indexes into cryptomount option array
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 13:24:54 +02:00
John Lane
81b2f625f5 disk/cryptodisk: Add options to cryptomount to support keyfiles
Add the options --key-file, --keyfile-offset, and --keyfile-size to
cryptomount and code to put read the requested key file data and pass
via the cargs struct. Note, key file data is for all intents and purposes
equivalent to a password given to cryptomount. So there is no need to
enable support for key files in the various crypto backends (e.g. LUKS1)
because the key data is passed just as if it were a password.

Signed-off-by: John Lane <john@lane.uk.net>
Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 13:22:14 +02:00
Denis 'GNUtoo' Carikli
100e410e9c disk/geli: Unify grub_cryptodisk_dev function names
Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 13:06:38 +02:00
Denis 'GNUtoo' Carikli
6fe6229638 disk/luks: Unify grub_cryptodisk_dev function names
Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 13:04:17 +02:00
Glenn Washburn
56ae06906d commands/macbless: Remove whitespace between N_ macro and open parenthesis
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-06-07 12:51:50 +02:00
Glenn Washburn
ad019a107a net/net: Fix incorrect condition for calling grub_net_tcp_retransmit()
The commit 848724273e4 (net/net: Avoid unnecessary calls to
grub_net_tcp_retransmit()) needs to have its condition inverted to avoid
unnecessary calls to grub_net_tcp_retransmit(). As it is, it creates many
unnecessary calls and does not call grub_net_tcp_retransmit() when needed.
The call to grub_net_tcp_retransmit() should only be made when
grub_net_cards does _not_ equal NULL, meaning that there are potentially
network cards that need TCP retransmission.

Fixes: 848724273e4 (net/net: Avoid unnecessary calls to grub_net_tcp_retransmit())

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-05-24 14:37:24 +02:00
Samuel Thibault
ea1b565e8c osdep/hurd: Support device entries with @/dev/disk: qualifier
Those are used with non-bootstrap disk drivers, for which libstore has to
open /dev/disk before calling device_open on it instead of on the device
master port. Normally in that case all /dev/ entries also have the @/dev/disk:
qualifier, so we can just drop it.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-05-24 14:18:45 +02:00
Qiumiao Zhang
63d3211403 net: Fix NULL pointer dereference when parsing ICMP6_ROUTER_ADVERTISE messages
During UEFI PXE boot in IPv6 network, if the DHCP server adopts stateful
automatic configuration, then the client receives a ICMP6_ROUTER_ADVERTISE
multicast message from the server. This may be received without the interface
having a configured network address, so orig_inf will be NULL, which can lead
to a NULL dereference when creating the default route. Actually, in this case,
the client obtains the default route through DHCPv6 instead of RA messages.
So if orig_inf == NULL and route_inf == NULL, we should not set the
default route.

Fixes: https://savannah.gnu.org/bugs/?62072

Signed-off-by: Qiumiao Zhang <zhangqiumiao1@huawei.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-04-26 15:53:11 +02:00
Darren Kenny
c244e331b9 video/readers/jpeg: Fix possible invalid loop boundary condition
The value of next_marker is adjusted based on the word sized value
read from data->file.

The updated next_marker value should reference a location in the file
just beyond the huffman table, and as such should not have a value
larger than the size of the file.

Fixes: CID 73657

Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-04-20 18:29:00 +02:00
Michael Chang
3ce13d974b lib/reed_solomon: Fix array subscript 0 is outside array bounds
The grub_absolute_pointer() is a compound expression that can only work
within a function. We are out of luck here when the pointer variables
require global definition due to ATTRIBUTE_TEXT that have to use fully
initialized global definition because of the way linkers work.

  static gf_single_t * const gf_powx ATTRIBUTE_TEXT = (void *) 0x100000;

For the reason given above, use GCC diagnostic pragmas to suppress the
array-bounds warning.

Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-04-20 18:29:00 +02:00
Michael Chang
acffb81485 build: Fix -Werror=array-bounds array subscript 0 is outside array bounds
The GRUB is failing to build with GCC-12 in many places like this:

  In function 'init_cbfsdisk',
      inlined from 'grub_mod_init' at ../../grub-core/fs/cbfs.c:391:3:
  ../../grub-core/fs/cbfs.c:345:7: error: array subscript 0 is outside array bounds of 'grub_uint32_t[0]' {aka 'unsigned int[]'} [-Werror=array-bounds]
    345 |   ptr = *(grub_uint32_t *) 0xfffffffc;
        |   ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This is caused by GCC regression in 11/12 [1]. In a nut shell, the
warning is about detected invalid accesses at non-zero offsets to NULL
pointers. Since hardwired constant address is treated as NULL plus an
offset in the same underlying code, the warning is therefore triggered.

Instead of inserting #pragma all over the places where literal pointers
are accessed to avoid diagnosing array-bounds, we can try to borrow the
idea from Linux kernel that the absolute_pointer() macro [2][3] is used
to disconnect a pointer using literal address from it's original object,
hence GCC won't be able to make assumptions on the boundary while doing
pointer arithmetic. With that we can greatly reduce the code we have to
cover up by making initial literal pointer assignment to use the new
wrapper but not having to track everywhere literal pointers are
accessed. This also makes code looks cleaner.

Please note the grub_absolute_pointer() macro requires to be invoked in
a function as long as it is compound expression. Some global variables
with literal pointers has been changed to local ones in order to use
grub_absolute_pointer() to initialize it. The shuffling is basically done
in a selective and careful way that the variable's scope doesn't matter
being local or global, for example, the global variable must not get
modified at run time throughout. For the record, here's the list of
global variables got shuffled in this patch:

  grub-core/commands/i386/pc/drivemap.c:int13slot
  grub-core/term/i386/pc/console.c:bios_data_area
  grub-core/term/ns8250.c:serial_hw_io_addr

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
[2] https://elixir.bootlin.com/linux/v5.16.14/source/include/linux/compiler.h#L180
[3] https://elixir.bootlin.com/linux/v5.16.14/source/include/linux/compiler-gcc.h#L31

Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-04-20 18:27:52 +02:00
Chad Kimes
9322a7740f net/drivers/efi/efinet: Configure VLAN from UEFI device used for PXE
This patch handles automatic configuration of VLAN when booting from PXE
on UEFI hardware.

Signed-off-by: Chad Kimes <chkimes@github.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-04-20 14:20:36 +02:00
Chad Kimes
c143056a34 kern/efi/efi: Print VLAN info in EFI device path
Signed-off-by: Chad Kimes <chkimes@github.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-04-20 13:58:13 +02:00
Chad Kimes
954c48b9c8 net/net: Add net_set_vlan command
Previously there was no way to set the 802.1Q VLAN identifier, despite
support for vlantag in the net module. The only location vlantag was
being populated was from PXE boot and only for Open Firmware hardware.
This commit allows users to manually configure VLAN information for any
interface.

Example usage:
  grub> net_ls_addr
  efinet1 00:11:22:33:44:55 192.0.2.100
  grub> net_set_vlan efinet1 100
  grub> net_ls_addr
  efinet1 00:11:22:33:44:55 192.0.2.100 vlan100
  grub> net_set_vlan efinet1 0
  efinet1 00:11:22:33:44:55 192.0.2.100

Signed-off-by: Chad Kimes <chkimes@github.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-04-20 13:52:16 +02:00
Chad Kimes
98c299e540 net/net: Add vlan information to net_ls_addr output
Example output:
  grub> net_ls_addr
  efinet1 00:11:22:33:44:55 192.0.2.100 vlan100

Signed-off-by: Chad Kimes <chkimes@github.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-04-20 13:42:30 +02:00
Chris Coulson
37ddd9457f kern/efi/init: Log a console error during a stack check failure
The initial implementation of the stack protector just busy looped
in __stack_chk_fail in order to reduce the amount of code being
executed after the stack has been compromised because of a lack of
firmware memory protections. With future firmware implementations
incorporating memory protections such as W^X, call in to boot services
when an error occurs in order to log a message to the console before
automatically rebooting the machine.

Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2022-04-04 22:26:31 +02:00
Alec Brown
a97d1ebb8e loader/i386/xnu: Fix uninitialized scalar variable
In the function grub_xnu_boot(), struct grub_relocator32_state state is called
but isn't being initialized. This results in the members grub_uint32_t ebx,
grub_uint32_t ecx, grub_uint32_t edx, grub_uint32_t edi, and grub_uint32_t esi
being filled with junk data from the stack since none of them are being set to
any values. We can prevent this by setting state to {0}.

Fixes: CID 375035

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>
2022-04-04 20:28:55 +02:00
Alec Brown
4fa5dd08f5 loader/i386/xnu: Fix uninitialized scalar variable
In the function grub_xnu_boot_resume(), struct grub_relocator32_state state is
called but isn't being initialized. This results in the members grub_uint32_t
ebx, grub_uint32_t ecx, grub_uint32_t edx, grub_uint32_t esi, and grub_uint32_t
edi being filled with junk data from the stack since none of them are being set
to any values. We can prevent this by setting state to {0}.

Fixes: CID 375031

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>
2022-04-04 20:28:55 +02:00
Alec Brown
261e4511ce loader/i386/pc/linux: Fix uninitialized scalar variable
In the function grub_linux16_boot(), struct grub_relocator16_state state is
called but isn't being initialized. This results in the members grub_uint32_t
ebx, grub_uint32_t edx, grub_uint32_t esi, and grub_uint32_t ebp being filled
with junk data from the stack since none of them are being set to any values.
We can prevent this by setting state to {0}.

Fixes: CID 375028

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>
2022-04-04 20:28:54 +02:00