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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
This is just a preparatory patch to move the functions higher in the file,
since these will be called by the grub_prepare_for_text_output() function
that will be introduced in a later patch.
The logic is unchanged by this patch. Functions definitions are just moved
to avoid a forward declaration in a later patch, keeping the code clean.
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
We bumped into the build error while testing gcc-10 pre-release.
In file included from ../../include/grub/file.h:22,
from ../../grub-core/fs/zfs/zfs.c:34:
../../grub-core/fs/zfs/zfs.c: In function 'zap_leaf_lookup':
../../grub-core/fs/zfs/zfs.c:2263:44: error: array subscript '<unknown>' is outside the bounds of an interior zero-length array 'grub_uint16_t[0]' {aka 'short unsigned int[0]'} [-Werror=zero-length-bounds]
2263 | for (chunk = grub_zfs_to_cpu16 (l->l_hash[LEAF_HASH (blksft, h, l)], endian);
../../include/grub/types.h:241:48: note: in definition of macro 'grub_le_to_cpu16'
241 | # define grub_le_to_cpu16(x) ((grub_uint16_t) (x))
| ^
../../grub-core/fs/zfs/zfs.c:2263:16: note: in expansion of macro 'grub_zfs_to_cpu16'
2263 | for (chunk = grub_zfs_to_cpu16 (l->l_hash[LEAF_HASH (blksft, h, l)], endian);
| ^~~~~~~~~~~~~~~~~
In file included from ../../grub-core/fs/zfs/zfs.c:48:
../../include/grub/zfs/zap_leaf.h:72:16: note: while referencing 'l_hash'
72 | grub_uint16_t l_hash[0];
| ^~~~~~
Here I'd like to quote from the gcc document [1] which seems best to
explain what is going on here.
"Although the size of a zero-length array is zero, an array member of
this kind may increase the size of the enclosing type as a result of
tail padding. The offset of a zero-length array member from the
beginning of the enclosing structure is the same as the offset of an
array with one or more elements of the same type. The alignment of a
zero-length array is the same as the alignment of its elements.
Declaring zero-length arrays in other contexts, including as interior
members of structure objects or as non-member objects, is discouraged.
Accessing elements of zero-length arrays declared in such contexts is
undefined and may be diagnosed."
The l_hash[0] is apparnetly an interior member to the enclosed structure
while l_entries[0] is the trailing member. And the offending code tries
to access members in l_hash[0] array that triggers the diagnose.
Given that the l_entries[0] is used to get proper alignment to access
leaf chunks, we can accomplish the same thing through the ALIGN_UP macro
thus eliminating l_entries[0] from the structure. In this way we can
pacify the warning as l_hash[0] now becomes the last member to the
enclosed structure.
[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
We bumped into the build error while testing gcc-10 pre-release.
../../grub-core/disk/mdraid1x_linux.c: In function 'grub_mdraid_detect':
../../grub-core/disk/mdraid1x_linux.c:181:15: error: array subscript <unknown> is outside array bounds of 'grub_uint16_t[0]' {aka 'short unsigned int[0]'} [-Werror=array-bounds]
181 | (char *) &sb.dev_roles[grub_le_to_cpu32 (sb.dev_number)]
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../grub-core/disk/mdraid1x_linux.c:98:17: note: while referencing 'dev_roles'
98 | grub_uint16_t dev_roles[0]; /* Role in array, or 0xffff for a spare, or 0xfffe for faulty. */
| ^~~~~~~~~
../../grub-core/disk/mdraid1x_linux.c:127:33: note: defined here 'sb'
127 | struct grub_raid_super_1x sb;
| ^~
cc1: all warnings being treated as errors
Apparently gcc issues the warning when trying to access sb.dev_roles
array's member, since it is a zero length array as the last element of
struct grub_raid_super_1x that is allocated sparsely without extra
chunks for the trailing bits, so the warning looks legitimate in this
regard.
As the whole thing here is doing offset computation, it is undue to use
syntax that would imply array member access then take address from it
later. Instead we could accomplish the same thing through basic array
pointer arithmetic to pacify the warning.
Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The memory requested for the event is not released here,
causing memory leaks. This patch fixes this problem.
Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The LVM cache logical volume is the logical volume consisting of the original
and the cache pool logical volume. The original is usually on a larger and
slower storage device while the cache pool is on a smaller and faster one. The
performance of the original volume can be improved by storing the frequently
used data on the cache pool to utilize the greater performance of faster
device.
The default cache mode "writethrough" ensures that any data written will be
stored both in the cache and on the origin LV, therefore grub can be straight
to read the original lv as no data loss is guarenteed.
The second cache mode is "writeback", which delays writing from the cache pool
back to the origin LV to have increased performance. The drawback is potential
data loss if losing the associated cache device.
During the boot time grub reads the LVM offline i.e. LVM volumes are not
activated and mounted, hence it should be fine to read directly from original
lv since all cached data should have been flushed back in the process of taking
it offline.
It is also not much helpful to the situation by adding fsync calls to the
install code. The fsync did not force to write back dirty cache to the original
device and rather it would update associated cache metadata to complete the
write transaction with the cache device. IOW the writes to cached blocks still
go only to the cache device.
To write back dirty cache, as LVM cache did not support dirty cache flush per
block range, there'no way to do it for file. On the other hand the "cleaner"
policy is implemented and can be used to write back "all" dirty blocks in a
cache, which effectively drain all dirty cache gradually to attain and last in
the "clean" state, which can be useful for shrinking or decommissioning a
cache. The result and effect is not what we are looking for here.
In conclusion, as it seems no way to enforce file writes to the original
device, grub may suffer from power failure as it cannot assemble the cache
device and read the dirty data from it. However since the case is only
applicable to writeback mode which is sensitive to data lost in nature, I'd
still like to propose my (relatively simple) patch and treat reading dirty
cache as improvement.
Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
When building GRUB with memory management debugging enabled, then the
build fails because of `grub_debug_malloc()` and `grub_debug_free()`
being undefined in the luks2 module. The cause is that we patch
"base64.h" to unconditionaly include "config-util.h", which shouldn't be
included for modules at all. As a result, `MM_DEBUG` is defined when
building the module, causing it to use the debug memory allocation
functions. As these are not built into modules, we end up with a linker
error.
Fix the issue by removing the <config-util.h> include altogether. The
sole reason it was included was for the `_GL_ATTRIBUTE_CONST` macro,
which we can simply define as empty in case it's not set.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This allows comparing file ages on EFI system partitions.
Signed-off-by: David Michael <fedora.dm0@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This provides the node's attributes outside the iterator function
so the file modification time can be accessed and reported.
Signed-off-by: David Michael <fedora.dm0@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Fixes a build failure:
grub-core/commands/date.c:49: undefined reference to `grub_get_weekday_name'
grub-core/commands/ls.c:155: undefined reference to `grub_unixtime2datetime'
Bug: https://bugs.gentoo.org/711512
Signed-off-by: Mike Gilbert <floppym@gentoo.org>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Add debug information to EFI GOP video driver probing function.
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>
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>
No messages were printed in this function, add some to ease debugging.
Also, the function returns a void * pointer so return NULL instead of
0 to 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>
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>
Coverity Scan reports that the grub_strrchr() function can return NULL if
the character is not found. Check if that's the case for dirfile pointer.
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>
Add a grub_debug_enabled() helper function instead of open coding it.
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>
The function that searches the mods section base address does not have
any debug information. Add some debugging outputs that could be useful.
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>
The Micron PCIe SSDs Linux driver (mtip32xx) exposes block devices
as /dev/rssd[a-z]+[0-9]*. Add support for these rssd device names.
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>
Linux creates modalias strings by filtering out non-ASCII, space,
and colon characters. Provide an option that does the same filtering
so people can create a modalias string in GRUB, and then match their
modalias patterns against it.
Signed-off-by: Julian Andres Klode <julian.klode@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Currently the string functions grub_strtol(), grub_strtoul(), and
grub_strtoull() don't declare the "end" pointer in such a way as to
require the pointer itself or the character array to be immutable to the
implementation, nor does the C standard do so in its similar functions,
though it does require us not to change any of it.
The typical declarations of these functions follow this pattern:
long
strtol(const char * restrict nptr, char ** restrict endptr, int base);
Much of the reason for this is historic, and a discussion of that
follows below, after the explanation of this change. (GRUB currently
does not include the "restrict" qualifiers, and we name the arguments a
bit differently.)
The implementation is semantically required to treat the character array
as immutable, but such accidental modifications aren't stopped by the
compiler, and the semantics for both the callers and the implementation
of these functions are sometimes also helped by adding that requirement.
This patch changes these declarations to follow this pattern instead:
long
strtol(const char * restrict nptr,
const char ** const restrict endptr,
int base);
This means that if any modification to these functions accidentally
introduces either an errant modification to the underlying character
array, or an accidental assignment to endptr rather than *endptr, the
compiler should generate an error. (The two uses of "restrict" in this
case basically mean strtol() isn't allowed to modify the character array
by going through *endptr, and endptr isn't allowed to point inside the
array.)
It also means the typical use case changes to:
char *s = ...;
const char *end;
long l;
l = strtol(s, &end, 10);
Or even:
const char *p = str;
while (p && *p) {
long l = strtol(p, &p, 10);
...
}
This fixes 26 places where we discard our attempts at treating the data
safely by doing:
const char *p = str;
long l;
l = strtol(p, (char **)&ptr, 10);
It also adds 5 places where we do:
char *p = str;
while (p && *p) {
long l = strtol(p, (const char ** const)&p, 10);
...
/* more calls that need p not to be pointer-to-const */
}
While moderately distasteful, this is a better problem to have.
With one minor exception, I have tested that all of this compiles
without relevant warnings or errors, and that /much/ of it behaves
correctly, with gcc 9 using 'gcc -W -Wall -Wextra'. The one exception
is the changes in grub-core/osdep/aros/hostdisk.c , which I have no idea
how to build.
Because the C standard defined type-qualifiers in a way that can be
confusing, in the past there's been a slow but fairly regular stream of
churn within our patches, which add and remove the const qualifier in many
of the users of these functions. This change should help avoid that in
the future, and in order to help ensure this, I've added an explanation
in misc.h so that when someone does get a compiler warning about a type
error, they have the fix at hand.
The reason we don't have "const" in these calls in the standard is
purely anachronistic: C78 (de facto) did not have type qualifiers in the
syntax, and the "const" type qualifier was added for C89 (I think; it
may have been later). strtol() appears to date from 4.3BSD in 1986,
which means it could not be added to those functions in the standard
without breaking compatibility, which is usually avoided.
The syntax chosen for type qualifiers is what has led to the churn
regarding usage of const, and is especially confusing on string
functions due to the lack of a string type. Quoting from C99, the
syntax is:
declarator:
pointer[opt] direct-declarator
direct-declarator:
identifier
( declarator )
direct-declarator [ type-qualifier-list[opt] assignment-expression[opt] ]
...
direct-declarator [ type-qualifier-list[opt] * ]
...
pointer:
* type-qualifier-list[opt]
* type-qualifier-list[opt] pointer
type-qualifier-list:
type-qualifier
type-qualifier-list type-qualifier
...
type-qualifier:
const
restrict
volatile
So the examples go like:
const char foo; // immutable object
const char *foo; // mutable pointer to object
char * const foo; // immutable pointer to mutable object
const char * const foo; // immutable pointer to immutable object
const char const * const foo; // XXX extra const keyword in the middle
const char * const * const foo; // immutable pointer to immutable
// pointer to immutable object
const char ** const foo; // immutable pointer to mutable pointer
// to immutable object
Making const left-associative for * and right-associative for everything
else may not have been the best choice ever, but here we are, and the
inevitable result is people using trying to use const (as they should!),
putting it at the wrong place, fighting with the compiler for a bit, and
then either removing it or typecasting something in a bad way. I won't
go into describing restrict, but its syntax has exactly the same issue
as with const.
Anyway, the last example above actually represents the *behavior* that's
required of strtol()-like functions, so that's our choice for the "end"
pointer.
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The debug message printed when decryption with a keyslot fails is
missing its trailing newline. Add it to avoid mangling it with
subsequent output.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The necessary check for NULL before use of function ver->close is not
taking place in the failure path. This patch simply adds the missing
check and fixes the problem that GRUB hangs indefinitely after booting
rogue image without valid signature if secure boot is turned on.
Now it displays like this for booting rogue UEFI image:
error: bad shim signature
error: you need to load the kernel first
Press any key to continue...
and then you can go back to boot menu by pressing any key or after a few
seconds expired.
Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This patch implements a search for a specific configuration when the config
file is on a remoteserver. It uses the following order:
1) DHCP client UUID option.
2) MAC address (in lower case hexadecimal with dash separators);
3) IP (in upper case hexadecimal) or IPv6;
4) The original grub.cfg file.
This procedure is similar to what is used by pxelinux and yaboot:
http://www.syslinux.org/wiki/index.php/PXELINUX#config
It is enabled by default but can be disabled by setting the environment
variable "feature_net_search_cfg" to "n" in an embedded configuration.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=873406
Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This patch sets a net_<interface>_clientid and net_<interface>_clientuuid
GRUB environment variables, using the DHCP client ID and UUID options if
these are found.
In the same way than net_<interface>_<option> variables are set for other
options such domain name, boot file, next server, etc.
Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The printf(3) function has support for the %X format specifier, to output
an unsigned hexadecimal integer in uppercase.
This can be achived in GRUB using the %x format specifier in grub_printf()
and calling grub_toupper(), but it is more convenient if there is support
for %X in grub_printf().
Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@br.ibm.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The common datetime helper functions are currently included in the normal
module, but this makes any other module that calls these functions to have
a dependency with the normal module only for this reason.
Since the normal module does a lot of stuff, it calls functions from other
modules. But since other modules may depend on it for calling the datetime
helpers, this could lead to circular dependencies between modules.
As an example, when platform == xen the grub_get_datetime() function from
the datetime module calls to the grub_unixtime2datetime() helper function
from the normal module. Which leads to the following module dependency:
datetime -> normal
and send_dhcp_packet() from the net module calls the grub_get_datetime()
function, which leads to the following module dependency:
net -> datetime -> normal
but that means that the normal module is not allowed to depend on net or
any other module that depends on it due the transitive dependency caused
by datetime. A recent patch attempted to add support to fetch the config
file over the network, which leads to the following circular dependency:
normal -> net -> datetime -> normal
So having the datetime helpers in the normal module makes it quite fragile
and easy to add circular dependencies like these, that break the build due
the genmoddep.awk script catching the issues.
Fix this by taking the datetime helper functions out of the normal module
and instead add them to the datetime module itself. Besides fixing these
issues, it makes more sense to have these helper functions there anyways.
Reported-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This patch updates the miniLZO library to a newer version, which among other
things fixes "CVE-2014-4607 - lzo: lzo1x_decompress_safe() integer overflow"
that is present in the current used in GRUB.
It also updates the "GRUB Developers Manual", to mention that the library is
used and describes the process to update it to a newer release when needed.
Resolves: http://savannah.gnu.org/bugs/?42635
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>
gcc says:
grub-core/fs/squash4.c: In function ‘direct_read’:
grub-core/fs/squash4.c:868:10: error: ‘err’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
868 | if (err)
| ^
cc1: all warnings being treated as errors
This patch initializes it to GRUB_ERR_NONE.
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
While testing the 86-DOS lDebug [1] booting from GRUB2, newer versions of the
debugger would fail to load when booted using GRUB's freedos command. The
behaviour observed in a qemu i386 machine was that the ROM-BIOS's boot load
would start anew, instead of loading the selected debugger as kernel.
It came to light that there was a size limit: Kernel files that were 58880
bytes (E600h) long or shorter succeeded to boot, while files that were 64000
bytes or longer failed in the manner described.
Eventually it turned out that the relocator16 stub succeeded whenever it was
placed completely within the first 64 KiB of the Low Memory Area. The chunk
for the relocator is allocated with a minimum address of 0x8010 and a maximum
address just below 0xA0000 [2]. That means if the kernel is, for instance,
E600h bytes long, then the kernel will be allocated memory starting at 00600h
(the fixed FreeDOS kernel load address) up to E600h + 00600h = 0EC00h, which
leaves 1400h (5120) bytes for the relocator to stay in the first 64 KiB.
If the kernel is 64000 bytes (FA00h) long, then the relocator must go to
FA00h + 00600h = 10000h at least which is outside the first 64 KiB.
The problem is that the relocator16 initialises the DS register with a
"pseudo real mode" descriptor, which is defined with a segment limit of
64 KiB and a segment base of zero. After that, the relocator addressed
parts of itself (implicitly) using the DS register, with an offset from
ESI, which holds the linear address of the relocator's base [3]. With the
larger kernel files this would lead to accessing data beyond the 64 KiB
segment limit, presumably leading to a fault and perhaps a subsequent
triple-fault or such.
This patch fixes the relocator to set the segment base of the descriptors
to the base address of the relocator; then, the subsequent accesses to
the relocator's variables are done without the ESI register as an index.
This does not interfere with the relocator's or its target's normal
operation; the segment limits are still loaded with 64 KiB and all the
segment bases are subsequently reset by the relocator anyway.
Current versions of the debugger to test are uploaded to [4]. The file
ldebugnh.com (LZ4-compressed and built with -D_EXTHELP=0) at 58368 bytes
loads successfully, whereas ldebug.com at 64000 bytes fails. Loading one
of these files requires setting root to a FAT FS partition and using the
freedos command to specify the file as kernel:
set root='(hd0,msdos1)'
freedos /ldebug.com
boot
Booting the file using the multiboot command (which uses a WIP entrypoint
of the debugger) works, as it does not use GRUB's relocator16 but instead
includes a loader in the kernel itself, which drops it back to 86 Mode.
[1]: https://hg.ulukai.org/ecm/ldebug
[2]: http://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/lib/i386/relocator.c?id=495781f5ed1b48bf27f16c53940d6700c181c74c#n127
[3]: http://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/lib/i386/relocator16.S?id=495781f5ed1b48bf27f16c53940d6700c181c74c#n97
[4]: https://ulukai.org/ecm/lDebug-5479a7988d21-nohelp.zip
Signed-off-by: C. Masloch <pushbx@ulukai.org>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
With cryptsetup 2.0, a new version of LUKS was introduced that breaks
compatibility with the previous version due to various reasons. GRUB
currently lacks any support for LUKS2, making it impossible to decrypt
disks encrypted with that version. This commit implements support for
this new format.
Note that LUKS1 and LUKS2 are quite different data formats. While they
do share the same disk signature in the first few bytes, representation
of encryption parameters is completely different between both versions.
While the former version one relied on a single binary header, only,
LUKS2 uses the binary header only in order to locate the actual metadata
which is encoded in JSON. Furthermore, the new data format is a lot more
complex to allow for more flexible setups, like e.g. having multiple
encrypted segments and other features that weren't previously possible.
Because of this, it was decided that it doesn't make sense to keep both
LUKS1 and LUKS2 support in the same module and instead to implement it
in two different modules luks and luks2.
The proposed support for LUKS2 is able to make use of the metadata to
decrypt such disks. Note though that in the current version, only the
PBKDF2 key derival function is supported. This can mostly attributed to
the fact that the libgcrypt library currently has no support for either
Argon2i or Argon2id, which are the remaining KDFs supported by LUKS2. It
wouldn't have been much of a problem to bundle those algorithms with
GRUB itself, but it was decided against that in order to keep down the
number of patches required for initial LUKS2 support. Adding it in the
future would be trivial, given that the code structure is already in
place.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The luks module contains quite a lot of logic to parse cipher and
cipher-mode strings like aes-xts-plain64 into constants to apply them
to the grub_cryptodisk_t structure. This code will be required by the
upcoming luks2 module, as well, which is why this commit moves it into
its own function grub_cryptodisk_setcipher in the cryptodisk module.
While the strings are probably rather specific to the LUKS modules, it
certainly does make sense that the cryptodisk module houses code to set
up its own internal ciphers instead of hosting that code in the luks
module.
Except for necessary adjustments around error handling, this commit does
an exact move of the cipher configuration logic from luks.c to
cryptodisk.c. Any behavior changes are unintentional.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
