Commit Graph

896 Commits

Author SHA1 Message Date
Chris Coulson
3d157bbd06 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>
2021-03-02 15:54:19 +01:00
Chris Coulson
b1c9e9e889 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>
2021-03-02 15:54:19 +01:00
Chris Coulson
c6c426e5ab 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>
2021-03-02 15:54:19 +01:00
Daniel Axtens
f41f0af48a kern/misc: Always set *end in grub_strtoull()
Currently, if there is an error in grub_strtoull(), *end is not set.
This differs from the usual behavior of strtoull(), and also means that
some callers may use an uninitialized value for *end.

Set *end unconditionally.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2021-03-02 15:54:17 +01:00
Darren Kenny
bc9c468a2c kern/partition: Check for NULL before dereferencing input string
There is the possibility that the value of str comes from an external
source and continuing to use it before ever checking its validity is
wrong. So, needs fixing.

Additionally, drop unneeded part initialization.

Fixes: CID 292444

Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2021-03-02 15:54:16 +01:00
Darren Kenny
6aee4bfd69 kern/efi/mm: Fix possible NULL pointer dereference
The model of grub_efi_get_memory_map() is that if memory_map is NULL,
then the purpose is to discover how much memory should be allocated to
it for the subsequent call.

The problem here is that with grub_efi_is_finished set to 1, there is no
check at all that the function is being called with a non-NULL memory_map.

While this MAY be true, we shouldn't assume it.

The solution to this is to behave as expected, and if memory_map is NULL,
then don't try to use it and allow memory_map_size to be filled in, and
return 0 as is done later in the code if the buffer is too small (or NULL).

Additionally, drop unneeded ret = 1.

Fixes: CID 96632

Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2021-03-02 15:54:16 +01:00
Darren Kenny
ed286ceba6 kern/efi: Fix memory leak on failure
Free the memory allocated to name before returning on failure.

Fixes: CID 296222

Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2021-03-02 15:54:16 +01:00
Darren Kenny
d06161b035 kern/parser: Fix resource leak if argc == 0
After processing the command-line yet arriving at the point where we are
setting argv, we are allocating memory, even if argc == 0, which makes
no sense since we never put anything into the allocated argv.

The solution is to simply return that we've successfully processed the
arguments but that argc == 0, and also ensure that argv is NULL when
we're not allocating anything in it.

There are only 2 callers of this function, and both are handling a zero
value in argc assuming nothing is allocated in argv.

Fixes: CID 96680

Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2021-03-02 15:54:16 +01:00
Javier Martinez Canillas
7630ec5397 dl: Only allow unloading modules that are not dependencies
When a module is attempted to be removed its reference counter is always
decremented. This means that repeated rmmod invocations will cause the
module to be unloaded even if another module depends on it.

This may lead to a use-after-free scenario allowing an attacker to execute
arbitrary code and by-pass the UEFI Secure Boot protection.

While being there, add the extern keyword to some function declarations in
that header file.

Fixes: CVE-2020-25632

Reported-by: Chris Coulson <chris.coulson@canonical.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2021-03-02 15:54:15 +01:00
Javier Martinez Canillas
8f73052885 efi: Use grub_is_lockdown() instead of hardcoding a disabled modules list
Now the GRUB can check if it has been locked down and this can be used to
prevent executing commands that can be utilized to circumvent the UEFI
Secure Boot mechanisms. So, instead of hardcoding a list of modules that
have to be disabled, prevent the usage of commands that can be dangerous.

This not only allows the commands to be disabled on other platforms, but
also properly separate the concerns. Since the shim_lock verifier logic
should be only about preventing to run untrusted binaries and not about
defining these kind of policies.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2021-03-02 15:54:15 +01:00
Javier Martinez Canillas
98b00a403c efi: Lockdown the GRUB when the UEFI Secure Boot is enabled
If the UEFI Secure Boot is enabled then the GRUB must be locked down
to prevent executing code that can potentially be used to subvert its
verification mechanisms.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2021-03-02 15:54:15 +01:00
Javier Martinez Canillas
d903674717 kern/lockdown: Set a variable if the GRUB is locked down
It may be useful for scripts to determine whether the GRUB is locked
down or not. Add the lockdown variable which is set to "y" when the GRUB
is locked down.

Suggested-by: Dimitri John Ledkov <xnox@ubuntu.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2021-03-02 15:54:15 +01:00
Javier Martinez Canillas
578c95298b kern: Add lockdown support
When the GRUB starts on a secure boot platform, some commands can be
used to subvert the protections provided by the verification mechanism and
could lead to booting untrusted system.

To prevent that situation, allow GRUB to be locked down. That way the code
may check if GRUB has been locked down and further restrict the commands
that are registered or what subset of their functionality could be used.

The lockdown support adds the following components:

* The grub_lockdown() function which can be used to lockdown GRUB if,
  e.g., UEFI Secure Boot is enabled.

* The grub_is_lockdown() function which can be used to check if the GRUB
  was locked down.

* A verifier that flags OS kernels, the GRUB modules, Device Trees and ACPI
  tables as GRUB_VERIFY_FLAGS_DEFER_AUTH to defer verification to other
  verifiers. These files are only successfully verified if another registered
  verifier returns success. Otherwise, the whole verification process fails.

  For example, PE/COFF binaries verification can be done by the shim_lock
  verifier which validates the signatures using the shim_lock protocol.
  However, the verification is not deferred directly to the shim_lock verifier.
  The shim_lock verifier is hooked into the verification process instead.

* A set of grub_{command,extcmd}_lockdown functions that can be used by
  code registering command handlers, to only register unsafe commands if
  the GRUB has not been locked down.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2021-03-02 15:54:15 +01:00
Marco A Benatto
5e280caa65 efi: Move the shim_lock verifier to the GRUB core
Move the shim_lock verifier from its own module into the core image. The
Secure Boot lockdown mechanism has the intent to prevent the load of any
unsigned code or binary when Secure Boot is enabled.

The reason is that GRUB must be able to prevent executing untrusted code
if UEFI Secure Boot is enabled, without depending on external modules.

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>
2021-03-02 15:54:15 +01:00
Marco A Benatto
9e95f45cee verifiers: Move verifiers API to kernel image
Move verifiers API from a module to the kernel image, so it can be
used there as well. There are no functional changes in this patch.

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>
2021-03-02 15:54:15 +01:00
Glenn Washburn
9b4e8f0c4b mips: Enable __clzdi2()
This patch is similar to commit 9dab2f51e (sparc: Enable __clzsi2() and
__clzdi2()) but for MIPS target and __clzdi2() only, __clzsi2() was
already enabled.

Suggested-by: Daniel Kiper <dkiper@net-space.pl>
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-18 23:04:36 +01:00
Glenn Washburn
880dfd8f40 disk: Rename grub_disk_get_size() to grub_disk_native_sectors()
The function grub_disk_get_size() is confusingly named because it actually
returns a sector count where the sectors are sized in the GRUB native sector
size. Rename to something more appropriate.

Suggested-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:03 +01:00
Glenn Washburn
407ddead69 disk: Move hardcoded max disk size literal to a GRUB_DISK_MAX_SECTORS in disk.h
There is a hardcoded maximum disk size that can be read or written from,
currently set at 1 EiB in grub_disk_adjust_range(). Move the literal into a
macro in disk.h, so our assumptions are more visible. This hard coded limit
does not prevent using larger disks, just GRUB won't read/write past the
limit. The comment accompanying this restriction didn't quite make sense to
me, so its been modified too.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:03 +01:00
Glenn Washburn
8919eecd84 fs: Fix block lists not being able to address to end of disk sometimes
When checking if a block list goes past the end of the disk, make sure
the total size of the disk is in GRUB native sector sizes, otherwise there
will be blocks at the end of the disk inaccessible by block lists.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:03 +01:00
Daniel Kiper
d7e54b2e5f efi: Add secure boot detection
Introduce grub_efi_get_secureboot() function which returns whether
UEFI Secure Boot is enabled or not on UEFI systems.

Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
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>
2020-12-11 13:56:22 +01:00
Daniel Kiper
ac5c936754 efi: Add a function to read EFI variables with attributes
It will be used 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: Ignat Korchagin <ignat@cloudflare.com>
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>
2020-12-11 13:55:31 +01:00
Daniel Kiper
04ae030d0e efi: Return grub_efi_status_t from grub_efi_get_variable()
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>
2020-12-11 13:54:54 +01:00
Javier Martinez Canillas
b87781feaf i386: Don't include <grub/cpu/linux.h> in coreboot and ieee1275 startup.S
Nothing defined in the header file is used in the assembly code but it
may lead to build errors if some headers are included through this and
contains definitions that are not recognized by the assembler, e.g.:

../include/grub/types.h: Assembler messages:
../include/grub/types.h:76: Error: no such instruction: `typedef signed char grub_int8_t'
../include/grub/types.h:77: Error: no such instruction: `typedef short grub_int16_t'
../include/grub/types.h:78: Error: no such instruction: `typedef int grub_int32_t'

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-11 13:52:18 +01:00
Cao jin
2b106a2a4e kern/i386/realmode: Update comment
Commit b81d609e4c did not update it.

Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-10-30 21:53:00 +01:00
Daniel Kiper
c386331364 i386/efi/init: Drop bogus include
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
2020-09-18 21:22:32 +02:00
Alexey Makhalov
f7bd9986f6 efi: Fix use-after-free in halt/reboot path
commit 92bfc33db9 ("efi: Free malloc regions on exit")
introduced memory freeing in grub_efi_fini(), which is
used not only by exit path but by halt/reboot one as well.
As result of memory freeing, code and data regions used by
modules, such as halt, reboot, acpi (used by halt) also got
freed. After return to module code, CPU executes, filled
by UEFI firmware (tested with edk2), 0xAFAFAFAF pattern as
a code. Which leads to #UD exception later.

grub> halt
!!!! X64 Exception Type - 06(#UD - Invalid Opcode)  CPU Apic ID - 00000000 !!!!
RIP  - 0000000003F4EC28, CS  - 0000000000000038, RFLAGS - 0000000000200246
RAX  - 0000000000000000, RCX - 00000000061DA188, RDX - 0A74C0854DC35D41
RBX  - 0000000003E10E08, RSP - 0000000007F0F860, RBP - 0000000000000000
RSI  - 00000000064DB768, RDI - 000000000832C5C3
R8   - 0000000000000002, R9  - 0000000000000000, R10 - 00000000061E2E52
R11  - 0000000000000020, R12 - 0000000003EE5C1F, R13 - 00000000061E0FF4
R14  - 0000000003E10D80, R15 - 00000000061E2F60
DS   - 0000000000000030, ES  - 0000000000000030, FS  - 0000000000000030
GS   - 0000000000000030, SS  - 0000000000000030
CR0  - 0000000080010033, CR2 - 0000000000000000, CR3 - 0000000007C01000
CR4  - 0000000000000668, CR8 - 0000000000000000
DR0  - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
DR3  - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400
GDTR - 00000000079EEA98 0000000000000047, LDTR - 0000000000000000
IDTR - 0000000007598018 0000000000000FFF,   TR - 0000000000000000
FXSAVE_STATE - 0000000007F0F4C0

Proposal here is to continue to free allocated memory for
exit boot services path but keep it for halt/reboot path
as it won't be much security concern here.
Introduced GRUB_LOADER_FLAG_EFI_KEEP_ALLOCATED_MEMORY
loader flag to be used by efi halt/reboot path.

Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Peter Jones
d2cf823d0e efi: Fix some malformed device path arithmetic errors
Several places we take the length of a device path and subtract 4 from
it, without ever checking that it's >= 4. There are also cases where
this kind of malformation will result in unpredictable iteration,
including treating the length from one dp node as the type in the next
node. These are all errors, no matter where the data comes from.

This patch adds a checking macro, GRUB_EFI_DEVICE_PATH_VALID(), which
can be used in several places, and makes GRUB_EFI_NEXT_DEVICE_PATH()
return NULL and GRUB_EFI_END_ENTIRE_DEVICE_PATH() evaluate as true when
the length is too small. Additionally, it makes several places in the
code check for and return errors in these cases.

Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Peter Jones
b73cee7f1f emu: Make grub_free(NULL) safe
The grub_free() implementation in grub-core/kern/mm.c safely handles
NULL pointers, and code at many places depends on this. We don't know
that the same is true on all host OSes, so we need to handle the same
behavior in grub-emu's implementation.

Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Peter Jones
f725fa7cb2 calloc: Use calloc() at most places
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>
2020-07-29 16:55:47 +02:00
Peter Jones
64e26162eb calloc: Make sure we always have an overflow-checking calloc() available
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>
2020-07-29 16:55:47 +02:00
Marc Zyngier
6a34fdb76a arm: Fix 32-bit ARM handling of the CTR register
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>
2020-05-25 15:02:51 +02:00
Daniel Kiper
eb46ee98bc mips/cache: Add missing nop's in delay slots
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>
2020-05-15 14:30:07 +02:00
Hans de Goede
12341958d2 kern/term: Accept ESC, F4 and holding SHIFT as user interrupt keys
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>
2020-04-21 22:13:44 +02:00
Hans de Goede
5bcdf67642 kern/term: Make grub_getkeystatus() helper function available everywhere
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>
2020-04-21 22:08:52 +02:00
Peter Jones
df84d6e94c efi: Print error messages to grub_efi_allocate_pages_real()
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>
2020-03-10 21:41:16 +01:00
Peter Jones
8d88ae92b5 kern: Add grub_debug_enabled()
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>
2020-03-10 21:40:06 +01:00
Peter Jones
cc93c5a849 efi: Print more debug info in our module loader
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>
2020-03-10 21:39:44 +01:00
Peter Jones
d5a32255de misc: Make grub_strtol() "end" pointers have safer const qualifiers
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>
2020-02-28 12:41:29 +01:00
Paulo Flabiano Smorigo
8c2c35dcc0 kern: Add %X option to printf functions
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>
2020-02-18 15:12:06 +01:00
Vladimir 'phcoder' Serbinenko
5610734770 hostfs: #undef open and close.
Unlike in case of disks in this case it's just a single place, so it's easier
to just #undef

Signed-off-by: Vladimir Serbinenko <phcoder@google.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2019-06-07 15:36:28 +02:00
Michael Chang
4dd4ceec02 efi: Fix gcc9 error -Waddress-of-packed-member
The address of fp->path_name could be unaligned since seeking into the
device path buffer for a given node could end in byte boundary.

The fix is allocating aligned buffer by grub_malloc for holding the
UTF16 string copied from fp->path_name, and after using that buffer as
argument for grub_utf16_to_utf8 to convert it to UTF8 string.

[  255s] ../../grub-core/kern/efi/efi.c: In function 'grub_efi_get_filename':
[  255s] ../../grub-core/kern/efi/efi.c:410:60: error: taking address of packed member of 'struct grub_efi_file_path_device_path' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[  255s]   410 |    p = (char *) grub_utf16_to_utf8 ((unsigned char *) p, fp->path_name, len);
[  255s]       |                                                          ~~^~~~~~~~~~~
[  255s] ../../grub-core/kern/efi/efi.c: In function 'grub_efi_print_device_path':
[  255s] ../../grub-core/kern/efi/efi.c:900:33: error: taking address of packed member of 'struct grub_efi_file_path_device_path' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[  255s]   900 |     *grub_utf16_to_utf8 (buf, fp->path_name,
[  255s]       |                               ~~^~~~~~~~~~~

Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2019-04-23 11:37:08 +02:00
Heinrich Schuchardt
bc58fded50 efi: Avoid NULL dereference if FilePath is NULL
The UEFI specification allows LoadImage() to be called with a memory
location only and without a device path. In this case FilePath will not be
set in the EFI_LOADED_IMAGE_PROTOCOL.

So in function grub_efi_get_filename() the device path argument may be
NULL. As we cannot determine the device path in this case just return NULL
from the function.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2019-04-23 11:33:02 +02:00
Vladimir Serbinenko
ad4bfeec5c Change fs functions to add fs_ prefix
This avoid conflict with gnulib

Signed-off-by: Vladimir Serbinenko <phcoder@google.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2019-04-09 10:03:29 +10:00
Eric Snowberg
0f1b648b45 ieee1275: NULL pointer dereference in grub_ieee1275_encode_devname()
Function grub_strndup() may return NULL, this is called from
function grub_ieee1275_get_devname() which is then called from
function grub_ieee1275_encode_devname() to set device. The device
variable could then be used with a NULL pointer.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
Reviewed-by: Colin Watson <cjwatson@ubuntu.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2019-04-04 18:34:05 +02:00
Vladimir Serbinenko
384091967d Rename grub_disk members
Otherwise it horribly clashes with gnulib when it's
replacing open/write/read/close

Signed-off-by: Vladimir Serbinenko <phcoder@google.com>
2019-03-25 15:14:52 +01:00
Vladimir Serbinenko
d900dfa985 kern/emu/misc.c: Don't include config-util.h when running as GRUB_BUILD
Signed-off-by: Vladimir Serbinenko <phcoder@google.com>
2019-03-25 15:10:15 +01:00
Vladimir Serbinenko
4ff051880f Support R_PPC_PLTREL24
It's emitted by clang 7. It's the same as R_PPC_REL24.

Signed-off-by: Vladimir Serbinenko <phcoder@google.com>
2019-03-25 15:08:49 +01:00
Daniel Kiper
9dab2f51ea sparc: Enable __clzsi2() and __clzdi2()
This patch is similiar to commit e795b9011 (RISC-V: Add libgcc helpers
for clz) but for SPARC target.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Ross Philipson <ross.philipson@oracle.com>
2019-03-20 11:38:28 +01:00
Daniel Kiper
e42b0d97ec mips: Enable __clzsi2()
This patch is similiar to commit e795b9011 (RISC-V: Add libgcc helpers
for clz) but for MIPS target.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Ross Philipson <ross.philipson@oracle.com>
2019-03-20 11:38:28 +01:00
Eric Snowberg
3434ddec0e ieee1275: obdisk driver
Add a new disk driver called obdisk for IEEE1275 platforms.  Currently
the only platform using this disk driver is SPARC, however other IEEE1275
platforms could start using it if they so choose.  While the functionality
within the current IEEE1275 ofdisk driver may be suitable for PPC and x86, it
presented too many problems on SPARC hardware.

Within the old ofdisk, there is not a way to determine the true canonical
name for the disk.  Within Open Boot, the same disk can have multiple names
but all reference the same disk.  For example the same disk can be referenced
by its SAS WWN, using this form:

/pci@302/pci@2/pci@0/pci@17/LSI,sas@0/disk@w5000cca02f037d6d,0

It can also be referenced by its PHY identifier using this form:

/pci@302/pci@2/pci@0/pci@17/LSI,sas@0/disk@p0

It can also be referenced by its Target identifier using this form:

/pci@302/pci@2/pci@0/pci@17/LSI,sas@0/disk@0

Also, when the LUN=0, it is legal to omit the ,0 from the device name.  So with
the disk above, before taking into account the device aliases, there are 6 ways
to reference the same disk.

Then it is possible to have 0 .. n device aliases all representing the same disk.
Within this new driver the true canonical name is determined using the the
IEEE1275 encode-unit and decode-unit commands when address_cells == 4.  This
will determine the true single canonical name for the device so multiple ihandles
are not opened for the same device.  This is what frequently happens with the old
ofdisk driver.  With some devices when they are opened multiple times it causes
the entire system to hang.

Another problem solved with this driver is devices that do not have a device
alias can be booted and used within GRUB. Within the old ofdisk, this was not
possible, unless it was the original boot device.  All devices behind a SAS
or SCSI parent can be found.   Within the old ofdisk, finding these disks
relied on there being an alias defined.  The alias requirement is not
necessary with this new driver.  It can also find devices behind a parent
after they have been hot-plugged.  This is something that is not possible
with the old ofdisk driver.

The old ofdisk driver also incorrectly assumes that the device pointing to by a
device alias is in its true canonical form. This assumption is never made with
this new driver.

Another issue solved with this driver is that it properly caches the ihandle
for all open devices.  The old ofdisk tries to do this by caching the last
opened ihandle.  However this does not work properly because the layer above
does not use a consistent device name for the same disk when calling into the
driver.  This is because the upper layer uses the bootpath value returned within
/chosen, other times it uses the device alias, and other times it uses the
value within grub.cfg.  It does not have a way to figure out that these devices
are the same disk.  This is not a problem with this new driver.

Due to the way GRUB repeatedly opens and closes the same disk. Caching the
ihandle is important on SPARC.  Without caching, some SAS devices can take
15 - 20 minutes to get to the GRUB menu. This ihandle caching is not possible
without correctly having the canonical disk name.

When available, this driver also tries to use the deblocker #blocks and
a way of determining the disk size.

Finally and probably most importantly, this new driver is also capable of
seeing all partitions on a GPT disk.  With the old driver, the GPT
partition table can not be read and only the first partition on the disk
can be seen.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2019-03-12 20:04:07 +01:00