Glenn Washburn [Mon, 3 Mar 2025 08:12:02 +0000 (02:12 -0600)]
tests: Cleanup generated files on expected failure in grub_cmd_cryptomount
grub-shell-luks-tester only cleans up generated files when the test it
runs returns success. Sometimes tests are run that should fail. Add
a --xfail argument to grub-shell-luks-tester and pass it from
grub_cmd_cryptomount when invoking a test that is expected to fail.
Reported-by: Thomas Schmitt <scdbackup@gmx.net> Signed-off-by: Glenn Washburn <development@efficientek.com> Tested-by: Thomas Schmitt <scdbackup@gmx.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Glenn Washburn [Thu, 6 Mar 2025 06:18:43 +0000 (00:18 -0600)]
tests/util/grub-shell-luks-tester: Find cryptodisk by UUID
GRUB has the capability to search all the disks for a cryptodisk of a
given UUID. Use this instead of hardcoding which disk is the cryptodisk,
which can change when devices are added or removed, or potentially when
QEMU is upgraded. This can not be done for the detached header tests
because the header contains the UUID.
Also, capitalize comment lines for consistency.
Signed-off-by: Glenn Washburn <development@efficientek.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Glenn Washburn [Thu, 6 Mar 2025 06:48:13 +0000 (00:48 -0600)]
tests/util/grub-shell: Default qemuopts to envvar $GRUB_QEMU_OPTS
Fix a regression where qemuopts was mistakenly defaulted to the empty
string. This prevents the sending of arbitrary QEMU options to tests,
which may be desirable for overriding the machine type. There was a
concern that allowing the tester to accept arbitrary options would add
headaches for another developer trying to diagnose why a test failed on
the testers machine because he could not be sure if any additional
options were passed to make the test fail. However, all the options are
recorded in the run.sh generated script, so this concern is unwarranted.
Fixes: 6d729ced70 (tests/util/grub-shell: Add $GRUB_QEMU_OPTS to run.sh to easily see unofficial QEMU arguments) Signed-off-by: Glenn Washburn <development@efficientek.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Mark cachevol LV's as ignored features, which is true only if they are
configured as "writethrough". This patch does not let GRUB boot from
"writeback" cache-enabled LV's.
Signed-off-by: Patrick Plenefisch <simonpatp@gmail.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The LV matching must be done after processing the ignored feature
indirections, as integrity volumes & caches may have several levels
of indirection that the segments must be shifted through.
Signed-off-by: Patrick Plenefisch <simonpatp@gmail.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
disk/lvm: Make cache_lv more generic as ignored_feature_lv
This patch isn't necessary by itself, but when combined with subsequent
patches it enhances readability as ignored_features_lv is then used for
multiple types of extra LV's, not just cache LV's.
Signed-off-by: Patrick Plenefisch <simonpatp@gmail.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Glenn Washburn [Sun, 2 Mar 2025 05:15:37 +0000 (23:15 -0600)]
commands/ls: Add directory header for dir args
Like the GNU ls, first print a line with the directory path before printing
files in the directory, which will not have a directory component, but only
if there is more than one argument.
Signed-off-by: Glenn Washburn <development@efficientek.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Glenn Washburn [Sun, 2 Mar 2025 05:15:34 +0000 (23:15 -0600)]
commands/ls: Show modification time for file paths
The modification time for paths to files was not being printed because
the grub_dirhook_info, which contains the mtime, was initialized to NULL.
Instead of calling print_file() directly, use fs->fs_dir() to call
print_file() with a properly filled in grub_dirhook_info. This has the
added benefit of reducing code complexity.
Signed-off-by: Glenn Washburn <development@efficientek.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Andrew Hamilton [Fri, 28 Feb 2025 21:55:24 +0000 (15:55 -0600)]
docs: Capture additional commands restricted by lockdown
Update documentation to capture that all memrw commands, the minicmd
dump command, and raw memory dumping via hexdump are restricted when
lockdown is enabled. This aligns to recent GRUB code updates.
Signed-off-by: Andrew Hamilton <adhamilt@gmail.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
grub-core/lib/tss2/tss2_structs.h contains a duplicate typedef as follows:
typedef TPMS_SCHEME_HASH_t TPMS_SCHEME_KDF2_t;
This causes a build failure when compiling with clang. Remove the
duplicate typedef which allows successfully building GRUB with clang.
Signed-off-by: Andrew Hamilton <adhamilt@gmail.com> Reviewed-by: Ross Philipson <ross.philipson@oracle.com> Reviewed-by: Gary Lin <glin@suse.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
James Le Cuirot [Mon, 30 Dec 2024 10:52:23 +0000 (10:52 +0000)]
script/execute: Don't let trailing blank lines determine the return code
The grub_script_execute_sourcecode() parses and executes code one line
at a time, updating the return code each time because only the last line
determines the final status. However, trailing new lines were also
executed, masking any failure on the previous line. Fix this by only
trying to execute the command when there is actually one present.
This has presumably never been noticed because this code is not used by
regular functions, only in special cases like eval and menu entries. The
latter generally don't return at all, having booted an OS. When failing
to boot, upstream GRUB triggers the fallback mechanism regardless of the
return code.
We noticed the problem while using Red Hat's patches, which change this
behaviour to take account of the return code. In that case, a failure
takes you back to the menu rather than triggering a fallback.
Signed-off-by: James Le Cuirot <jlecuirot@microsoft.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Glenn Washburn [Fri, 10 Jan 2025 20:19:41 +0000 (14:19 -0600)]
gitignore: Ignore generated files from libtasn
The commit 504058e8 (libtasn1: Compile into asn1 module) generates files
into the grub-core/lib/libtasn1-grub directory and commit 99cda678
(asn1_test: Test module for libtasn1) generates files into the
grub-core/tests/asn1/tests directory. Ignore these directories as they
are not under revision control.
Signed-off-by: Glenn Washburn <development@efficientek.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Pascal Hambourg [Mon, 24 Feb 2025 17:10:45 +0000 (18:10 +0100)]
util/grub.d/30_os-prober.in: Conditionally show or hide chain and efi menu entries
On systems which support multiple boot platforms such as BIOS and
EFI, it makes no sense to show menu entries which are not supported
by the current boot platform. Menu entries generated from os-prober
"chain" boot type use boot sector chainloading which is supported
on PC BIOS platform only.
Show "chain" menu entries only if boot platform is PC BIOS.
Show "efi" menu entries only if boot platform is EFI.
This is aimed to allow os-prober to report both EFI and PC BIOS
boot loaders regardless of the current boot mode on x86 systems
which support both EFI and legacy BIOS boot, in order to generate
a config file which can be used with either BIOS or EFI boot.
Signed-off-by: Pascal Hambourg <pascal@plouf.fr.eu.org> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Pascal Hambourg [Fri, 21 Feb 2025 11:20:28 +0000 (12:20 +0100)]
util/grub.d/30_os-prober.in: Fix GRUB_OS_PROBER_SKIP_LIST for non-EFI
GRUB documentation states:
GRUB_OS_PROBER_SKIP_LIST
List of space-separated FS UUIDs of filesystems to be ignored from
os-prober output. For efi chainloaders it’s <UUID>@<EFI FILE>
But the actual behaviour does not match this description.
GRUB_OS_PROBER_SKIP_LIST="<UUID>"
does nothing. In order to skip non-EFI bootloaders, you must set
GRUB_OS_PROBER_SKIP_LIST="<UUID>@<DEVICE>"
which is both absurd, <UUID> and <DEVICE> are redundant, and wrong,
<DEVICE> such as /dev/sd* may not be persistent across boots.
Also, any non-word character is accepted as a separator, including "-"
and "@" which may be present in UUIDs. This can cause false positives
because of partial UUID match.
This patch fixes these flaws while retaining some backward compatibility
with previous behaviour which may be expected by existing setups:
- also accept <UUID>@/dev/* (with warning) for non-EFI bootloaders,
- also accept comma and semicolon as separator.
Fixes: 55e706c9 (Add GRUB_OS_PROBER_SKIP_LIST to selectively skipping systems) Signed-off-by: Pascal Hambourg <pascal@plouf.fr.eu.org> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Glenn Washburn [Sat, 4 Jan 2025 07:50:32 +0000 (01:50 -0600)]
docs: Do not reference non-existent --dumb option
This appears to be a relic from GRUB legacy that used a --dumb option for
its terminal command. The proper way to do this in GRUB2 is to set the
terminal to "dumb" via the terminfo command.
Fixes: https://savannah.gnu.org/bugs/?66302 Reported-by: Jernej Jakob <jernej.jakob+savgnu@gmail.com> Signed-off-by: Glenn Washburn <development@efficientek.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Glenn Washburn [Sat, 4 Jan 2025 07:41:15 +0000 (01:41 -0600)]
docs: Replace @lbracechar{} and @rbracechar{} with @{ and @}
Support for @lbracechar{} and @rbracechar{} was added in GNU Texinfo 5.0
but many older systems may have versions lower than this. Use @{ and @}
to support a wider range of GNU Texinfo versions.
Signed-off-by: Glenn Washburn <development@efficientek.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Egor Ignatov [Thu, 23 Jan 2025 17:44:14 +0000 (20:44 +0300)]
fs/xfs: Fix grub_xfs_iterate_dir() return value in case of failure
Commit ef7850c757 (fs/xfs: Fix issues found while fuzzing the XFS
filesystem) introduced multiple boundary checks in grub_xfs_iterate_dir()
but handled the error incorrectly returning error code instead of 0.
Fix it. Also change the error message so that it doesn't match the
message in grub_xfs_read_inode().
Fixes: ef7850c757 (fs/xfs: Fix issues found while fuzzing the XFS filesystem) Signed-off-by: Egor Ignatov <egori@altlinux.org> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Darrick J. Wong [Mon, 3 Feb 2025 23:41:22 +0000 (15:41 -0800)]
fs/xfs: Add new superblock features added in Linux 6.12/6.13
The Linux port of XFS added a few new features in 2024. The existing
GRUB driver doesn't attempt to read or write any of the new metadata,
so, all three can be added to the incompat allowlist.
On the occasion align XFS_SB_FEAT_INCOMPAT_NREXT64 value.
Signed-off-by: Darrick J. Wong <djwong@kernel.org> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Michael Chang [Fri, 21 Feb 2025 01:06:12 +0000 (09:06 +0800)]
fs/ext2: Rework out-of-bounds read for inline and external extents
Previously, the number of extent entries was not properly capped based
on the actual available space. This could lead to insufficient reads for
external extents since the computation was based solely on the inline
extent layout.
In this patch, when processing the extent header we determine whether
the header is stored inline, i.e. at inode->blocks.dir_blocks, or in an
external extent block. We then clamp the number of entries accordingly
(using max_inline_ext for inline extents and max_external_ext for
external extent blocks).
This change ensures that only the valid number of extent entries is
processed preventing out-of-bound reads and potential filesystem
corruption.
Fixes: 7e2f750f0a (fs/ext2: Fix out-of-bounds read for inline extents) Signed-off-by: Michael Chang <mchang@suse.com> Tested-by: Christian Hesse <mail@eworm.de> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Leo Sandoval [Wed, 22 Jan 2025 20:10:24 +0000 (14:10 -0600)]
disk/ahci: Remove conditional operator for endtime
The conditional makes no sense when the two possible expressions have
the same value, so, remove it (perhaps the compiler does it for us but
better to remove it). This change makes spinup argument unused. So, drop
it as well.
Signed-off-by: Leo Sandoval <lsandova@redhat.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
term/ns8250-spcr: Return if redirection is disabled
The Microsoft spec for SPCR says "The base address of the Serial Port
register set described using the ACPI Generic Address Structure, or
0 if console redirection is disabled". So, return early if redirection
is disabled (base address = 0). If this check is not done we may get
invalid ports on machines with redirection disabled and boot may hang
when reading the grub.cfg file.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> Reviewed-by: Leo Sandoval <lsandova@redhat.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Lukas Fink [Sun, 5 Jan 2025 08:24:11 +0000 (02:24 -0600)]
commands/file: Fix NULL dereference in the knetbsd tests
The pointer returned by grub_elf_file() is not checked to verify it is
not NULL before use. A NULL pointer may be returned when the given file
does not have a valid ELF header.
Fixes: https://savannah.gnu.org/bugs/?61960 Signed-off-by: Glenn Washburn <development@efficientek.com> Signed-off-by: Lukas Fink <lukas.fink1@gmail.com> Reviewed-by: Ross Philipson <ross.philipson@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> Reviewed-by: Ross Philipson <ross.philipson@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Ideally we would like to avoid all memory allocations after exiting EFI
Boot Services altogether but that requires significant code changes. This
patch adds a simple workaround that resets grub_mm_add_region_fn to NULL
after ExitBootServices() call, so:
- Memory allocations have a better chance of succeeding because grub_memalign()
will try to reclaim the disk cache if it sees a NULL in grub_mm_add_region_fn.
- At worst it will fail to allocate memory but it will explicitly tell users
that it's out of memory, which is still much better than the current
situation where it fails in a fairly random way and triggers a CPU fault.
Signed-off-by: Ruihan Li <lrh2000@pku.edu.cn> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Duan Yayong [Mon, 9 Dec 2024 06:48:32 +0000 (14:48 +0800)]
i386/tsc: The GRUB menu gets stuck due to unserialized rdtsc
This patch is used to fix GRUB menu gets stuck in server AC
poweron/poweroff stress test of x86_64, which is reproduced with
1/200 ratio. The root cause analysis as below:
Q: What's the code logic?
A: The grub_tsc_init() function will init tsc by setting grub_tsc_rate,
which call stack is:
Among, grub_divmod64() function needs tsc_diff as the second parameter.
In grub_pmtimer_wait_count_tsc(), we will call grub_get_tsc() function
to get time stamp counter value to assign to start_tsc variable, and
get into while (1) loop space to get end_tsc variable value with same
function, after 3580 ticks, return "end_tsc - start_tsc". Actually,
rdtsc instruction will be called in grub_get_tsc, but rdtsc instruction
is not reliable (for the reason see the next question), which will cause
tsc_diff to be a very big number larger than (1UL << 32) or a negative
number, so that grub_tsc_rate will be zero. When run_menu() function is
startup, and calls grub_tsc_get_time_ms() function to get current time
to check if timeout time reach, at this time, grub_tsc_get_time_ms()
function will return zero due to zero grub_tsc_rate variable, then GRUB
menu gets stuck...
Q: What's the difference between rdtsc and rdtscp instructions in x86_64
architecture? Here is more explanations from Intel® 64 and IA-32
Architectures Software Developer’s Manual Volume 2B (December 2024):
https://cdrdv2.intel.com/v1/dl/getContent/671241
A: In page 4-558 -> RDTSC—Read Time-Stamp Counter:
The RDTSC instruction is not a serializing instruction. It does not
necessarily wait until all previous instructions have been executed
before reading the counter. Similarly, subsequent instructions may
begin execution before the read operation is performed. The following
items may guide software seeking to order executions of RDTSC:
- If software requires RDTSC to be executed only after all previous
instructions have executed and all previous loads are globally
visible, it can execute LFENCE immediately before RDTSC.
- If software requires RDTSC to be executed only after all previous
instructions have executed and all previous loads and stores are
globally visible, it can execute the sequence MFENCE;LFENCE
immediately before RDTSC.
- If software requires RDTSC to be executed prior to execution of any
subsequent instruction (including any memory accesses), it can execute
the sequence LFENCE immediately after RDTSC.
A: In page 4-560 -> RDTSCP—Read Time-Stamp Counter and Processor ID:
The RDTSCP instruction is not a serializing instruction, but it does wait
until all previous instructions have executed and all previous loads are
globally visible. But it does not wait for previous stores to be globally
visible, and subsequent instructions may begin execution before the read
operation is performed. The following items may guide software seeking to
order executions of RDTSCP:
- If software requires RDTSCP to be executed only after all previous
stores are globally visible, it can execute MFENCE immediately before
RDTSCP.
- If software requires RDTSCP to be executed prior to execution of any
subsequent instruction (including any memory accesses), it can execute
LFENCE immediately after RDTSCP.
Q: Why there is a cpuid serializing instruction before rdtsc instruction,
but "grub_get_tsc" still cannot work as expect?
A: From Intel® 64 and IA-32 Architectures Software Developer's Manual
Volume 2A: Instruction Set Reference, A-L (December 2024):
https://cdrdv2.intel.com/v1/dl/getContent/671199
In page 3-222 -> CPUID—CPU Identification:
CPUID can be executed at any privilege level to serialize instruction execution.
Serializing instruction execution guarantees that any modifications to flags,
registers, and memory for previous instructions are completed before
the next instruction is fetched and executed.
So we only kept the instruction rdtsc and its previous instruction in order
currently. But it is still out-of-order possibility between rdtsc instruction
and its subsequent instruction.
Q: Why do we do this fix?
A: In the one hand, add cpuid instruction after rdtsc instruction to make sure
rdtsc instruction to be executed prior to execution of any subsequent instruction,
about serializing execution that all previous instructions have been executed
before rdtsc, there is a cpuid usage in original code. In the other hand, using
cpuid instruction rather than lfence can make sure a forward compatibility for
previous HW.
Base this fix, we did 1500 cycles power on/off stress test, and did not reproduce
this issue again.
Fixes: https://savannah.gnu.org/bugs/?66257 Signed-off-by: Duan Yayong <duanyayong@bytedance.com> Signed-off-by: Li Yongqiang <liyongqiang@huaqin.com> Signed-off-by: Sun Ming <simon.sun@huaqin.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Duan Yayong [Thu, 28 Nov 2024 03:48:26 +0000 (11:48 +0800)]
kern/i386/tsc_pmtimer: The GRUB menu gets stuck due to failed calibration
The grub_divmod64() may return 0 but grub_tsc_calibrate_from_pmtimer()
still returns 1 saying calibration succeeded. Of course it is not true.
So, return 0 when grub_divmod64() returns 0. This way other calibration
functions can be called subsequently.
Signed-off-by: Duan Yayong <duanyayong@bytedance.com> Signed-off-by: Li Yongqiang <liyongqiang@huaqin.com> Signed-off-by: Sun Ming <simon.sun@huaqin.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Sergii Dmytruk [Fri, 1 Nov 2024 22:54:40 +0000 (00:54 +0200)]
loader/i386/linux: Fix cleanup if kernel doesn't support 64-bit addressing
Simply returning from grub_cmd_linux() doesn't free "file" resource nor
calls grub_dl_ref(my_mod). Jump to "fail" label for proper cleanup like
other error checks do.
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Alec Brown [Wed, 5 Feb 2025 22:04:08 +0000 (22:04 +0000)]
loader/i386/bsd: Use safe math to avoid underflow
The operation kern_end - kern_start may underflow when we input it into
grub_relocator_alloc_chunk_addr() call. To avoid this we can use safe
math for this subtraction.
Fixes: CID 73845 Signed-off-by: Alec Brown <alec.r.brown@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Alec Brown [Fri, 7 Feb 2025 01:47:57 +0000 (01:47 +0000)]
loader/i386/linux: Cast left shift to grub_uint32_t
The Coverity complains that we might overflow into a negative value when
setting linux_params.kernel_alignment to (1 << align). We can remedy
this by casting it to grub_uint32_t.
Fixes: CID 473876 Signed-off-by: Alec Brown <alec.r.brown@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Lidong Chen [Thu, 6 Feb 2025 18:16:57 +0000 (18:16 +0000)]
kern/misc: Add sanity check after grub_strtoul() call
When the format string, fmt0, includes a positional argument
grub_strtoul() or grub_strtoull() is called to extract the argument
position. However, the returned argument position isn't fully validated.
If the format is something like "%0$x" then these functions return
0 which leads to an underflow in the calculation of the args index, curn.
The fix is to add a check to ensure the extracted argument position is
greater than 0 before computing curn. Additionally, replace one
grub_strtoull() with grub_strtoul() and change curn type to make code
more correct.
Lidong Chen [Thu, 6 Feb 2025 18:16:56 +0000 (18:16 +0000)]
kern/partition: Add sanity check after grub_strtoul() call
The current code incorrectly assumes that both the input and the values
returned by grub_strtoul() are always valid which can lead to potential
errors. This fix ensures proper validation to prevent any unintended issues.
Alec Brown [Tue, 4 Feb 2025 15:11:10 +0000 (15:11 +0000)]
bus/usb/ehci: Define GRUB_EHCI_TOGGLE as grub_uint32_t
The Coverity indicates that GRUB_EHCI_TOGGLE is an int that contains
a negative value and we are using it for the variable token which is
grub_uint32_t. To remedy this we can cast the definition to grub_uint32_t.
Fixes: CID 473851 Signed-off-by: Alec Brown <alec.r.brown@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Lidong Chen [Wed, 29 Jan 2025 06:48:38 +0000 (06:48 +0000)]
osdep/unix/getroot: Fix potential underflow
The entry_len is initialized in grub_find_root_devices_from_mountinfo()
to 0 before the while loop iterates through /proc/self/mountinfo. If the
file is empty or contains only invalid entries entry_len remains
0 causing entry_len - 1 in the subsequent for loop initialization
to underflow. To prevent this add a check to ensure entry_len > 0 before
entering the for loop.
Fixes: CID 473877 Signed-off-by: Lidong Chen <lidong.chen@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> Reviewed-by: Ross Philipson <ross.philipson@oracle.com>
Lidong Chen [Wed, 29 Jan 2025 06:48:37 +0000 (06:48 +0000)]
script/execute: Fix potential underflow and NULL dereference
The result is initialized to 0 in grub_script_arglist_to_argv().
If the for loop condition is not met both result.args and result.argc
remain 0 causing result.argc - 1 to underflow and/or result.args NULL
dereference. Fix the issues by adding relevant checks.
Alec Brown [Tue, 28 Jan 2025 05:15:50 +0000 (05:15 +0000)]
fs/sfs: Check if allocated memory is NULL
When using grub_zalloc(), if we are out of memory, this function can fail.
After allocating memory, we should check if grub_zalloc() returns NULL.
If so, we should handle this error.
Fixes: CID 473856 Signed-off-by: Alec Brown <alec.r.brown@oracle.com> Reviewed-by: Ross Philipson <ross.philipson@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Alec Brown [Wed, 22 Jan 2025 18:04:44 +0000 (18:04 +0000)]
net: Check if returned pointer for allocated memory is NULL
When using grub_malloc(), the function can fail if we are out of memory.
After allocating memory we should check if this function returned NULL
and handle this error if it did.
Signed-off-by: Alec Brown <alec.r.brown@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Lidong Chen [Wed, 22 Jan 2025 07:17:01 +0000 (07:17 +0000)]
fs/zfs: Check if returned pointer for allocated memory is NULL
When using grub_malloc() or grub_zalloc(), these functions can fail if
we are out of memory. After allocating memory we should check if these
functions returned NULL and handle this error if they did.
Signed-off-by: Lidong Chen <lidong.chen@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Lidong Chen [Tue, 21 Jan 2025 19:02:38 +0000 (19:02 +0000)]
fs: Prevent overflows when assigning returned values from read_number()
The direct assignment of the unsigned long long value returned by
read_number() can potentially lead to an overflow on a 32-bit systems.
The fix replaces the direct assignments with calls to grub_cast()
which detects the overflows and safely assigns the values if no
overflow is detected.
Signed-off-by: Lidong Chen <lidong.chen@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Alec Brown [Wed, 22 Jan 2025 02:55:11 +0000 (02:55 +0000)]
disk: Check if returned pointer for allocated memory is NULL
When using grub_malloc(), grub_zalloc() or grub_calloc(), these functions can
fail if we are out of memory. After allocating memory we should check if these
functions returned NULL and handle this error if they did.
On the occasion make a NULL check in ATA code more obvious.
Signed-off-by: Alec Brown <alec.r.brown@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
B Horn [Thu, 18 Apr 2024 19:37:10 +0000 (20:37 +0100)]
commands/memrw: Disable memory reading in lockdown mode
With the rest of module being blocked in lockdown mode it does not make
a lot of sense to leave memory reading enabled. This also goes in par
with disabling the dump command.
Reported-by: B Horn <b@horn.uk> Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
B Horn [Thu, 18 Apr 2024 19:29:39 +0000 (20:29 +0100)]
commands/minicmd: Block the dump command in lockdown mode
The dump enables a user to read memory which should not be possible
in lockdown mode.
Fixes: CVE-2025-1118 Reported-by: B Horn <b@horn.uk> Reported-by: Jonathan Bar Or <jonathanbaror@gmail.com> Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Lidong Chen [Mon, 16 Dec 2024 20:22:41 +0000 (20:22 +0000)]
commands/test: Stack overflow due to unlimited recursion depth
The test_parse() evaluates test expression recursively. Due to lack of
recursion depth check a specially crafted expression may cause a stack
overflow. The recursion is only triggered by the parentheses usage and
it can be unlimited. However, sensible expressions are unlikely to
contain more than a few parentheses. So, this patch limits the recursion
depth to 100, which should be sufficient.
Reported-by: Nils Langius <nils@langius.de> Signed-off-by: Lidong Chen <lidong.chen@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Jonathan Bar Or [Thu, 23 Jan 2025 18:17:05 +0000 (19:17 +0100)]
commands/read: Fix an integer overflow when supplying more than 2^31 characters
The grub_getline() function currently has a signed integer variable "i"
that can be overflown when user supplies more than 2^31 characters.
It results in a memory corruption of the allocated line buffer as well
as supplying large negative values to grub_realloc().
Fixes: CVE-2025-0690 Reported-by: Jonathan Bar Or <jonathanbaror@gmail.com> Signed-off-by: Jonathan Bar Or <jonathanbaror@gmail.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Lidong Chen [Fri, 22 Nov 2024 06:27:57 +0000 (06:27 +0000)]
gettext: Integer overflow leads to heap OOB write
The size calculation of the translation buffer in
grub_gettext_getstr_from_position() may overflow
to 0 leading to heap OOB write. This patch fixes
the issue by using grub_add() and checking for
an overflow.
Fixes: CVE-2024-45777 Reported-by: Nils Langius <nils@langius.de> Signed-off-by: Lidong Chen <lidong.chen@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> Reviewed-by: Alec Brown <alec.r.brown@oracle.com>
Lidong Chen [Fri, 22 Nov 2024 06:27:56 +0000 (06:27 +0000)]
gettext: Integer overflow leads to heap OOB write or read
Calculation of ctx->grub_gettext_msg_list size in grub_mofile_open() may
overflow leading to subsequent OOB write or read. This patch fixes the
issue by replacing grub_zalloc() and explicit multiplication with
grub_calloc() which does the same thing in safe manner.
Fixes: CVE-2024-45776 Reported-by: Nils Langius <nils@langius.de> Signed-off-by: Lidong Chen <lidong.chen@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> Reviewed-by: Alec Brown <alec.r.brown@oracle.com>
B Horn [Fri, 1 Nov 2024 23:52:06 +0000 (23:52 +0000)]
gettext: Remove variables hooks on module unload
The gettext module does not entirely cleanup after itself in
its GRUB_MOD_FINI() leaving a few variables hooks in place.
It is not possible to unload gettext module because normal
module depends on it. Though fix the issues for completeness.
Fixes: CVE-2025-0622 Reported-by: B Horn <b@horn.uk> Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
B Horn [Fri, 1 Nov 2024 23:46:55 +0000 (23:46 +0000)]
normal: Remove variables hooks on module unload
The normal module does not entirely cleanup after itself in
its GRUB_MOD_FINI() leaving a few variables hooks in place.
It is not possible to unload normal module now but fix the
issues for completeness.
On the occasion replace 0s with NULLs for "pager" variable
hooks unregister.
Fixes: CVE-2025-0622 Reported-by: B Horn <b@horn.uk> Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Lidong Chen [Fri, 22 Nov 2024 06:27:55 +0000 (06:27 +0000)]
commands/extcmd: Missing check for failed allocation
The grub_extcmd_dispatcher() calls grub_arg_list_alloc() to allocate
a grub_arg_list struct but it does not verify the allocation was successful.
In case of failed allocation the NULL state pointer can be accessed in
parse_option() through grub_arg_parse() which may lead to a security issue.
Fixes: CVE-2024-45775 Reported-by: Nils Langius <nils@langius.de> Signed-off-by: Lidong Chen <lidong.chen@oracle.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> Reviewed-by: Alec Brown <alec.r.brown@oracle.com>
B Horn [Thu, 7 Nov 2024 06:00:36 +0000 (06:00 +0000)]
kern/dl: Check for the SHF_INFO_LINK flag in grub_dl_relocate_symbols()
The grub_dl_relocate_symbols() iterates through the sections in
an ELF looking for relocation sections. According to the spec [1]
the SHF_INFO_LINK flag should be set if the sh_info field is meant
to be a section index.
B Horn [Thu, 18 Apr 2024 14:59:26 +0000 (15:59 +0100)]
kern/dl: Fix for an integer overflow in grub_dl_ref()
It was possible to overflow the value of mod->ref_count, a signed
integer, by repeatedly invoking insmod on an already loaded module.
This led to a use-after-free. As once ref_count was overflowed it became
possible to unload the module while there was still references to it.
This resolves the issue by using grub_add() to check if the ref_count
will overflow and then stops further increments. Further changes were
also made to grub_dl_unref() to check for the underflow condition and
the reference count was changed to an unsigned 64-bit integer.
Reported-by: B Horn <b@horn.uk> Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Daniel Axtens [Fri, 8 Mar 2024 11:47:20 +0000 (22:47 +1100)]
video/readers/jpeg: Do not permit duplicate SOF0 markers in JPEG
Otherwise a subsequent header could change the height and width
allowing future OOB writes.
Fixes: CVE-2024-45774 Reported-by: Nils Langius <nils@langius.de> Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
B Horn [Thu, 18 Apr 2024 16:32:34 +0000 (17:32 +0100)]
net/tftp: Fix stack buffer overflow in tftp_open()
An overly long filename can be passed to tftp_open() which would cause
grub_normalize_filename() to write out of bounds.
Fixed by adding an extra argument to grub_normalize_filename() for the
space available, making it act closer to a strlcpy(). As several fixed
strings are strcpy()'d after into the same buffer, their total length is
checked to see if they exceed the remaining space in the buffer. If so,
return an error.
On the occasion simplify code a bit by removing unneeded rrqlen zeroing.
Reported-by: B Horn <b@horn.uk> Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
B Horn [Fri, 15 Nov 2024 13:12:09 +0000 (13:12 +0000)]
net: Fix OOB write in grub_net_search_config_file()
The function included a call to grub_strcpy() which copied data from an
environment variable to a buffer allocated in grub_cmd_normal(). The
grub_cmd_normal() didn't consider the length of the environment variable.
So, the copy operation could exceed the allocation and lead to an OOB
write. Fix the issue by replacing grub_strcpy() with grub_strlcpy() and
pass the underlying buffers size to the grub_net_search_config_file().
Fixes: CVE-2025-0624 Reported-by: B Horn <b@horn.uk> Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
B Horn [Fri, 1 Nov 2024 23:49:48 +0000 (23:49 +0000)]
net: Remove variables hooks when interface is unregisted
The grub_net_network_level_interface_unregister(), previously
implemented in a header, did not remove the variables hooks that
were registered in grub_net_network_level_interface_register().
Fix this by implementing the same logic used to register the
variables and move the function into the grub-core/net/net.c.
Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
B Horn [Thu, 28 Nov 2024 04:05:04 +0000 (04:05 +0000)]
net: Unregister net_default_ip and net_default_mac variables hooks on unload
The net module is a dependency of normal. So, it shouldn't be possible
to unload the net. Though unregister variables hooks as a precaution.
It also gets in line with unregistering the other net module hooks.
Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
B Horn [Thu, 18 Apr 2024 18:04:13 +0000 (19:04 +0100)]
script/execute: Limit the recursion depth
If unbounded recursion is allowed it becomes possible to collide the
stack with the heap. As UEFI firmware often lacks guard pages this
becomes an exploitable issue as it is possible in some cases to do
a controlled overwrite of a section of this heap region with
arbitrary data.
Reported-by: B Horn <b@horn.uk> Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
B Horn [Sat, 16 Nov 2024 21:24:19 +0000 (21:24 +0000)]
kern/partition: Limit recursion in part_iterate()
The part_iterate() is used by grub_partition_iterate() as a callback in
the partition iterate functions. However, part_iterate() may also call
the partition iterate functions which may lead to recursion. Fix potential
issue by limiting the recursion depth.
Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
B Horn [Sun, 12 May 2024 03:09:24 +0000 (04:09 +0100)]
kern/disk: Limit recursion depth
The grub_disk_read() may trigger other disk reads, e.g. via loopbacks.
This may lead to very deep recursion which can corrupt the heap. So, fix
the issue by limiting reads depth.
Reported-by: B Horn <b@horn.uk> Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Michael Chang [Thu, 29 Aug 2024 05:27:30 +0000 (13:27 +0800)]
disk/cryptodisk: Require authentication after TPM unlock for CLI access
The GRUB may use TPM to verify the integrity of boot components and the
result can determine whether a previously sealed key can be released. If
everything checks out, showing nothing has been tampered with, the key
is released and GRUB unlocks the encrypted root partition for the next
stage of booting.
However, the liberal Command Line Interface (CLI) can be misused by
anyone in this case to access files in the encrypted partition one way
or another. Despite efforts to keep the CLI secure by preventing utility
command output from leaking file content, many techniques in the wild
could still be used to exploit the CLI, enabling attacks or learning
methods to attack. It's nearly impossible to account for all scenarios
where a hack could be applied.
Therefore, to mitigate potential misuse of the CLI after the root device
has been successfully unlocked via TPM, the user should be required to
authenticate using the LUKS password. This added layer of security
ensures that only authorized users can access the CLI reducing the risk
of exploitation or unauthorized access to the encrypted partition.
Fixes: CVE-2024-49504 Signed-off-by: Michael Chang <mchang@suse.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The grub_file_open() and grub_file_close() should be the only places
that allow a reference to a filesystem to stay open. So, add grub_dl_t
to grub_fs_t and set this in the GRUB_MOD_INIT() for each filesystem to
avoid issues when filesystems forget to do it themselves or do not track
their own references, e.g. squash4.
The fs_label(), fs_uuid(), fs_mtime() and fs_read() should all ref and
unref in the same function but it is essentially redundant in GRUB
single threaded model.
Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
B Horn [Sun, 12 May 2024 02:01:40 +0000 (03:01 +0100)]
kern/file: Ensure file->data is set
This is to avoid a generic issue were some filesystems would not set
data and also not set a grub_errno. This meant it was possible for many
filesystems to grub_dl_unref() themselves multiple times resulting in
it being possible to unload the filesystems while there were still
references to them, e.g., via a loopback.
Reported-by: B Horn <b@horn.uk> Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
B Horn [Sun, 12 May 2024 05:03:58 +0000 (06:03 +0100)]
fs/xfs: Ensuring failing to mount sets a grub_errno
It was previously possible for grub_xfs_mount() to return NULL without
setting grub_errno if the XFS version was invalid. This resulted in it
being possible for grub_dl_unref() to be called twice allowing the XFS
module to be unloaded while there were still references to it.
Fixing this problem in general by ensuring a grub_errno is set if the
fail label is reached.
Reported-by: B Horn <b@horn.uk> Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Michael Chang [Fri, 31 May 2024 07:14:57 +0000 (15:14 +0800)]
fs/xfs: Fix out-of-bounds read
The number of records in the root key array read from disk was not being
validated against the size of the root node. This could lead to an
out-of-bounds read.
This patch adds a check to ensure that the number of records in the root
key array does not exceed the expected size of a root node read from
disk. If this check detects an out-of-bounds condition the operation is
aborted to prevent random errors due to metadata corruption.
Reported-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Michael Chang <mchang@suse.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
B Horn [Tue, 14 May 2024 11:39:56 +0000 (12:39 +0100)]
fs/ntfs: Implement attribute verification
It was possible to read OOB when an attribute had a size that exceeded
the allocated buffer. This resolves that by making sure all attributes
that get read are fully in the allocated space by implementing
a function to validate them.
Defining the offsets in include/grub/ntfs.h but they are only used in
the validation function and not across the rest of the NTFS code.
Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
B Horn [Tue, 14 May 2024 11:39:56 +0000 (12:39 +0100)]
fs/ntfs: Use a helper function to access attributes
Right now to access the next attribute the code reads the length of the
current attribute and adds that to the current pointer. This is error
prone as bounds checking needs to be performed all over the place. So,
implement a helper and ensure its used across find_attr() and read_attr().
This commit does *not* implement full bounds checking. It is just the
preparation work for this to be added into the helper.
Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
B Horn [Tue, 7 Jan 2025 11:38:34 +0000 (11:38 +0000)]
fs/ntfs: Track the end of the MFT attribute buffer
The end of the attribute buffer should be stored alongside the rest of
the attribute struct as right now it is not possible to implement bounds
checking when accessing attributes sequentially.
This is done via:
- updating init_attr() to set at->end and check is is not initially out of bounds,
- implementing checks as init_attr() had its type change in its callers,
- updating the value of at->end when needed.
Signed-off-by: B Horn <b@horn.uk> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Michael Chang [Mon, 3 Jun 2024 04:12:06 +0000 (12:12 +0800)]
fs/ntfs: Fix out-of-bounds read
When parsing NTFS file records the presence of the 0xFF marker indicates
the end of the attribute list. This value signifies that there are no
more attributes to process.
However, when the end marker is missing due to corrupted metadata the
loop continues to read beyond the attribute list resulting in out-of-bounds
reads and potentially entering an infinite loop.
This patch adds a check to provide a stop condition for the loop ensuring
it stops at the end of the attribute list or at the end of the Master File
Table. This guards against out-of-bounds reads and prevents infinite loops.
Reported-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Michael Chang <mchang@suse.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Michael Chang [Fri, 31 May 2024 07:14:23 +0000 (15:14 +0800)]
fs/ext2: Fix out-of-bounds read for inline extents
When inline extents are used, i.e. the extent tree depth equals zero,
a maximum of four entries can fit into the inode's data block. If the
extent header states a number of entries greater than four the current
ext2 implementation causes an out-of-bounds read. Fix this issue by
capping the number of extents to four when reading inline extents.
Reported-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Michael Chang <mchang@suse.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
projects / thirdparty / grub.git / log
