In commit 178ac5107389 (affs: Fix memory leaks), fixes were made to
grub_affs_iterate_dir() to prevent memory leaks from occurring after it
returns without freeing node. However, there were still some instances
where node was causing a memory leak when the function returns after
calling grub_affs_create_node(). In this function, new memory is
allocated to node but doesn't get freed until the hook() function is
called near the end. Before hook() is called, node should be freed in
grub_affs_create_node() before returning out of it.
Fixes: 178ac5107389 (affs: Fix memory leaks)
Fixes: CID 73759
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Allow the use of HTTP servers listening on ports other 80. This is done
with an extension to the http notation:
(http[,server[,port]])
- or -
(http[,server[:port]])
Signed-off-by: Stephen Balousek <sbalousek@wickedloop.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Sometimes you only know which debug logging facility names you want to
turn off, not necessarily all the ones you want enabled. This patch allows
the debug string to contain facility names in the $debug variable which are
prefixed with a "-" to disable debug log messages for that conditional. Say
you want all debug logging on except for btrfs and scripting, then do:
"set debug=all,-btrfs,-scripting"
Note, that only the last occurrence of the facility name with or without a
leading "-" is considered. So simply appending ",-facilityname" to the
$debug variable will disable that conditional. To illustrate, the command
"set debug=all,-btrfs,-scripting,btrfs" will enable btrfs.
Also, add documentation explaining this new behavior.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The Coverity output is:
*** CID 366905: Memory - illegal accesses (USE_AFTER_FREE)
/grub-core/disk/cryptodisk.c: 1064 in grub_cryptodisk_scan_device_real()
1058 cleanup:
1059 if (askpass)
1060 {
1061 cargs->key_len = 0;
1062 grub_free (cargs->key_data);
1063 }
>>> CID 366905: Memory - illegal accesses (USE_AFTER_FREE)
>>> Using freed pointer "dev".
1064 return dev;
1065 }
1066
1067 #ifdef GRUB_UTIL
1068 #include <grub/util/misc.h>
1069 grub_err_t
Here the "dev" variable can point to a freed cryptodisk device if the
function grub_cryptodisk_insert() fails. This can happen only on a OOM
condition, but when this happens grub_cryptodisk_insert() calls grub_free on
the passed device. Since grub_cryptodisk_scan_device_real() assumes that
grub_cryptodisk_insert() is always successful, it will return the device,
though the device was freed.
Change grub_cryptodisk_insert() to not free the passed device on failure.
Then on grub_cryptodisk_insert() failure, free the device pointer. This is
done by going to the label "error", which will call cryptodisk_close() to
free the device and set the device pointer to NULL, so that a pointer to
freed memory is not returned.
Fixes: CID 366905
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The grub_mm_init_region() does some things that seem magical, especially
around region merging. Make it a bit clearer.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The grub_free() possesses a surprising number of quirks, and also
uses single-letter variable names confusingly to iterate through
the free list.
Document what's going on.
Use prev and cur to iterate over the free list.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Small allocations move the region's *first pointer. The comment
says that this happens for allocations under 64K. The code says
it's for allocations under 32K. Commit 45bf8b3a7549 changed the
code intentionally: make the comment match.
Fixes: 45bf8b3a7549 (* grub-core/kern/mm.c (grub_real_malloc): Decrease cut-off of moving the)
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
When iterating through the singly linked list of free blocks,
grub_real_malloc() uses p and q for the current and previous blocks
respectively. This isn't super clear, so swap to using prev and cur.
This makes another quirk more obvious. The comment at the top of
grub_real_malloc() might lead you to believe that the function will
allocate from *first if there is space in that block.
It actually doesn't do that, and it can't do that with the current
data structures. If we used up all of *first, we would need to change
the ->next of the previous block to point to *first->next, but we
can't do that because it's a singly linked list and we don't have
access to *first's previous block.
What grub_real_malloc() actually does is set *first to the initial
previous block, and *first->next is the block we try to allocate
from. That allows us to keep all the data structures consistent.
Document that.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Up to now GRUB can only embed to the first 64 KiB before primary
superblock of btrfs, effectively limiting the GRUB core size. That
could consequently pose restrictions to feature enablement like
advanced zstd compression.
This patch attempts to utilize full unused area reserved by btrfs for
the bootloader outlined in the document [1]:
The first 1MiB on each device is unused with the exception of primary
superblock that is on the offset 64KiB and spans 4KiB.
Apart from that, adjacent sectors to superblock and first block group
are not used for embedding in case of overflow and logged access to
adjacent sectors could be useful for tracing it up.
This patch has been tested to provide out of the box support for btrfs
zstd compression with which GRUB has been installed to the partition.
[1] https://btrfs.wiki.kernel.org/index.php/Manpage/btrfs(5)#BOOTLOADER_SUPPORT
Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Call grub_partition_get_name() unconditionally to initialize the part
variable. Then part will only be NULL when grub_partition_get_name() errors.
Note that when source->partition is NULL, then grub_partition_get_name()
returns an allocated empty string. So no comma or partition will be printed,
as desired.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Note that cargs.search_uuid does not need to be initialized in various parts
of the cryptomount argument parsing, just once when cargs is declared with
a struct initializer. The previous code used a global variable which would
retain the value across cryptomount invocations.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The crypto device modules should only be setting up the crypto devices and
not getting user input. This has the added benefit of simplifying the code
such that three essentially duplicate pieces of code are merged into one.
Add documentation of passphrase option for cryptomount as it is now usable.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Previously, the cryptomount arguments were passed by global variable and
function call argument, neither of which are ideal. This change passes data
via a grub_cryptomount_args struct, which can be added to over time as
opposed to continually adding arguments to the cryptodisk scan and
recover_key.
As an example, passing a password as a cryptomount argument is implemented.
However, the backends are not implemented, so testing this will return a not
implemented error.
Also, add comments to cryptomount argument parsing to make it more obvious
which argument states are being handled.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
When a cryptmount is specified with a UUID, but no cryptodisk backends find
a disk with that UUID, return a more detailed message giving telling the
user that they might not have a needed cryptobackend module loaded.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Update such that "cryptomount -u UUID" will not print two error messages
when an invalid passphrase is given and the most relevant error message
will be displayed.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This displays an error notifying the user that they'll want to load
a backend module to make cryptomount useful.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The global "have_it" was never used by the crypto-backends, but was used to
determine if a crypto-backend successfully mounted a cryptodisk with a given
UUID. This is not needed however, because grub_device_iterate() will return
1 if and only if grub_cryptodisk_scan_device() returns 1. And
grub_cryptodisk_scan_device() will now only return 1 if a search_uuid has
been specified and a cryptodisk was successfully setup by a crypto-backend or
a cryptodisk of the requested UUID is already open.
To implement this grub_cryptodisk_scan_device_real() is modified to return
a cryptodisk or NULL on failure and having the appropriate grub_errno set to
indicated failure. Note that grub_cryptodisk_scan_device_real() will fail now
with a new errno GRUB_ERR_BAD_MODULE when none of the cryptodisk backend
modules succeed in identifying the source disk.
With this change grub_device_iterate() will return 1 when a crypto device is
successfully decrypted or when the source device has already been successfully
opened. Prior to this change, trying to mount an already successfully opened
device would trigger an error with the message "no such cryptodisk found",
which is at best misleading. The mount should silently succeed in this case,
which is what happens with this patch.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Create a library function for CloseProtocol() and use it for the SNP driver.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In the context of the implementation of the EFI_LOAD_FILE2_PROTOCOL for
the initial ramdisk it was observed that opening the SNP protocol failed.
https://lists.gnu.org/archive/html/grub-devel/2021-10/msg00020.html
This is due to an incorrect call to CloseProtocol().
The first parameter of CloseProtocol() is the handle, not the interface.
We call OpenProtocol() with ControllerHandle == NULL. Hence we must also
call CloseProtcol() with ControllerHandel == NULL.
Each call of OpenProtocol() for the same network card handle is expected to
return the same interface pointer. If we want to close the protocol which
we opened non-exclusively when searching for a card, we have to do this
before opening the protocol exclusively.
As there is no guarantee that we successfully open the protocol add checks
in the transmit and receive functions.
Reported-by: Andreas Schwab <schwab@linux-m68k.org>
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
minilzo fails to build on a number of Debian release architectures
(armel, mips64el, mipsel, ppc64el) with errors such as:
../../grub-core/lib/minilzo/minilzo.c: In function 'lzo_memops_get_le16':
../../grub-core/lib/minilzo/minilzo.c:3479:11: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
3479 | * (lzo_memops_TU2p) (lzo_memops_TU0p) (dd) = * (const lzo_memops_TU2p) (const lzo_memops_TU0p) (ss); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../grub-core/lib/minilzo/minilzo.c:3530:5: note: in expansion of macro 'LZO_MEMOPS_COPY2'
3530 | LZO_MEMOPS_COPY2(&v, ss);
| ^~~~~~~~~~~~~~~~
The latest upstream version is 2.10, so updating to it seems like a good
idea on general principles, and it fixes builds on all the above
architectures.
The update procedure documented in the GRUB Developers Manual worked; I
just updated the version numbers to make it clear that it's been
executed recently.
Signed-off-by: Colin Watson <cjwatson@debian.org>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The GRUB btrfs implementation can't handle two very basic btrfs
file layouts:
1. Mixed inline/regualr extents
# mkfs.btrfs -f test.img
# mount test.img /mnt/btrfs
# xfs_io -f -c "pwrite 0 1k" -c "sync" -c "falloc 0 4k" \
-c "pwrite 4k 4k" /mnt/btrfs/file
# umount /mnt/btrfs
# ./grub-fstest ./grub-fstest --debug=btrfs ~/test.img hex "/file"
Such mixed inline/regular extents case is not recommended layout,
but all existing tools and kernel can handle it without problem.
2. NO_HOLES feature
# mkfs.btrfs -f test.img -O no_holes
# mount test.img /mnt/btrfs
# xfs_io -f -c "pwrite 0 4k" -c "pwrite 8k 4k" /mnt/btrfs/file
# umount /mnt/btrfs
# ./grub-fstest ./grub-fstest --debug=btrfs ~/test.img hex "/file"
NO_HOLES feature is going to be the default mkfs feature in the incoming
v5.15 release, and kernel has support for it since v4.0.
The way GRUB btrfs code iterates through file extents relies on no gap
between extents.
If any gap is hit, then GRUB btrfs will error out, without any proper
reason to help debug the bug.
This is a bad assumption, since a long long time ago btrfs has a new
feature called NO_HOLES to allow btrfs to skip the padding hole extent
to reduce metadata usage.
The NO_HOLES feature is already stable since kernel v4.0 and is going to
be the default mkfs feature in the incoming v5.15 btrfs-progs release.
When there is a extent gap, instead of error out, just try next item.
This is still not ideal, as kernel/progs/U-boot all do the iteration
item by item, not relying on the file offset continuity.
But it will be way more time consuming to correct the whole behavior than
starting from scratch to build a proper designed btrfs module for GRUB.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Commit 23e39f50ca7a (disk/ldm: Make sure comp data is freed before exiting from
make_vg()) fixed several spots in make_vg() where comp data was leaking memory
when an error was being handled but missed one. To avoid leaking memory, comp
should be freed when an error is being handled after comp has been successfully
allocated memory in the for loop.
Fixes: 23e39f50ca7a (disk/ldm: Make sure comp data is freed before exiting from make_vg())
Fixes: CID 73804
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Commit 1fc860bb76bb (commands/probe: Fix a resource leak when probing disks),
missed other cases where grub_device_close() should be called before a return
statement is called. Also found that grub_disk_close() wasn't being called when
an error is being returned. To avoid conflict with grub_errno, grub_error_push()
should be called before either grub_device_close() or grub_disk_close() is
called and grub_error_pop() should be called before grub_errno is returned.
Fixes: 1fc860bb76bb (commands/probe: Fix a resource leak when probing disks)
Fixes: CID 292443
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In huft_build() it is possible to reach the for loop where "r" is being
assigned to "q[j]" without "r.v" ever being initialized.
Fixes: CID 314024
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In zap_leaf_array_get() the chunk size passed in is considered tainted
by Coverity, and is being used before it is tested for validity. To fix
this the assignment of "la" is moved until after the test of the value
of "chunk".
Fixes: CID 314014
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Prior to this change, the GRUB would only indicate that the check had
been failed, but not by what module. This made it difficult to track
down either the problem module, or debug the false positive further.
Before performing the license check, resolve the module name so that
it can be printed if the license check fails.
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Adding the conditional to debug log messages allows the GRUB user to
construct the $debug variable without needing to consult the source to
find the conditional (especially useful for situations where the source
is not readily available).
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
GET_ARRAY_INFO's info.nr_disks does not map to GET_DISK_INFO's
disk.number, which is an internal kernel index. If an array has had drives
added, removed, etc., there may be gaps in GET_DISK_INFO's results. But
since the consumer of devicelist cannot tolerate gaps (it expects to walk
a NULL-terminated list of device name strings), the devicelist index (j)
must be tracked separately from the disk.number index (i).
As part of this, since GRUB wants to only examine active (i.e. present
and non-failed) disks, the count of remaining disks (remaining) must be
tracked separately from the devicelist index (j).
Additionally, drop a line with empty spaces only.
Fixes: 49de079bbe1c (... (grub_util_raid_getmembers): Handle "removed" disks)
Fixes: 2b00217369ac (... Added support for RAID and LVM)
Fixes: https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/1912043
Fixes: https://savannah.gnu.org/bugs/index.php?59887
Signed-off-by: Kees Cook <kees@ubuntu.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Currently the grub_diskfilter_memberlist() function returns all physical
volumes added to a volume group to which a logical volume (LV) belongs.
However, this is suboptimal as it doesn't fit the intended behavior of
returning underlying devices that make up the LV. To give a clear
picture, the result should be identical to running commands below to
display the logical volumes with underlying physical volumes in use.
localhost:~ # lvs -o lv_name,vg_name,devices /dev/system/root
LV VG Devices
root system /dev/vda2(512)
localhost:~ # lvdisplay --maps /dev/system/root
--- Logical volume ---
...
--- Segments ---
Logical extents 0 to 4604:
Type linear
Physical volume /dev/vda2
Physical extents 512 to 5116
As shown above, we can know system-root LV uses only /dev/vda2 to
allocate it's extents, or we can say that /dev/vda2 is the member device
comprising the system-root LV.
It is important to be precise on the member devices, because that helps
to avoid pulling in excessive dependency. Let's use an example to
demonstrate why it is needed.
localhost:~ # findmnt /
TARGET SOURCE FSTYPE OPTIONS
/ /dev/mapper/system-root ext4 rw,relatime
localhost:~ # pvs
PV VG Fmt Attr PSize PFree
/dev/mapper/data system lvm2 a-- 1020.00m 0
/dev/vda2 system lvm2 a-- 19.99g 0
localhost:~ # cryptsetup status /dev/mapper/data
/dev/mapper/data is active and is in use.
type: LUKS1
cipher: aes-xts-plain64
keysize: 512 bits
key location: dm-crypt
device: /dev/vdb
sector size: 512
offset: 4096 sectors
size: 2093056 sectors
mode: read/write
localhost:~ # vgs
VG #PV #LV #SN Attr VSize VFree
system 2 3 0 wz--n- 20.98g 0
localhost:~ # lvs -o lv_name,vg_name,devices
LV VG Devices
data system /dev/mapper/data(0)
root system /dev/vda2(512)
swap system /dev/vda2(0)
We can learn from above that /dev/mapper/data is an encrypted volume and
also gets assigned to volume group "system" as one of it's physical
volumes. And also it is not used by root device, /dev/mapper/system-root,
for allocating extents, so it shouldn't be taking part in the process of
setting up GRUB to access root device.
However, running grub-install reports error as volume group "system"
contains encrypted volume.
error: attempt to install to encrypted disk without cryptodisk
enabled. Set `GRUB_ENABLE_CRYPTODISK=y' in file `/etc/default/grub'.
Certainly we can enable GRUB_ENABLE_CRYPTODISK=y and move on, but that
is not always acceptable since the server may need to be booted unattended.
Additionally, typing passphrase for every system startup can be a big
hassle of which most users would like to avoid.
This patch solves the problem by returning exact physical volume, /dev/vda2,
rightly used by system-root from the example above, thus grub-install will
not error out because the excessive encrypted device to boot the root device
is not configured.
Signed-off-by: Michael Chang <mchang@suse.com>
Tested-by: Olav Reinert <seroton10@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
When a file on ext4 is stored as sparse the data belonging to
zero-filled blocks is not written to storage and the extent map is
missing entries for these blocks. Such case can happen both for depth
0 extents (leafs) as well as higher-level tables.
Consider a scenario of a file which has a zero-filled beginning (e.g.
ISO image). In such case real data starts at block 8. If such a file is
stored using 2-level extent structure the extent list in the inode will
be depth 1 and will have an entry to a depth 0 (leaf) extent header for
blocks 8-n.
Unfortunately existing GRUB2 ext2 driver is only able to handle missing
entries in leaf extent tables, for which the grub_ext2_read_block()
function returns 0. In case the whole leaf extent list is missing for
a block the function fails with "invalid extent" error.
The fix for this problem relies on the grub_ext4_find_leaf() helper
function to distinguish two error cases: missing extent and error
walking through the extent tree. The existing error message is raised
only for the latter case, while for the missing leaf extent zero is
returned from grub_ext2_read_block() indicating a sparse block.
Signed-off-by: Krzysztof Nowicki <krzysztof.nowicki@nokia.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Open Hack'Ware was the only user. It added a lot of complexity.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Open Hack'Ware was an alternative firmware of powerpc under QEMU.
The last commit to any Open Hack'Ware repo I can find is from 2014 [1].
Open Hack'Ware was used for the QEMU "prep" machine type, which was
deprecated in QEMU in commit 54c86f5a4844 (hw/ppc: deprecate the
machine type 'prep', replaced by '40p') in QEMU v3.1, and had reportedly
been broken for years before without anyone noticing. Support was removed
in February 2020 by commit b2ce76a0730e (hw/ppc/prep: Remove the
deprecated "prep" machine and the OpenHackware BIOS).
Open Hack'Ware's limitations require some messy code in GRUB. This
complexity is not worth carrying any more.
Remove detection of Open Hack'Ware. We will clean up the feature flags
in following commits.
[1]: https://github.com/qemu/openhackware and
https://repo.or.cz/w/openhackware.git are QEMU submodules. They have
only small changes on top of OHW v0.4.1, which was imported into
QEMU SCM in 2010. I can't find anything resembling an official repo
any more.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This code was broken by commit 3f05d693 (malloc: Use overflow checking
primitives where we do complex allocations), which added overflow
checking in many areas. The problem here is that the changes update the
local variable sz, which was already in use and which was not updated
before the change. So the code using sz was getting a different value of
than it would have previously for the same UDF image. This causes the
logic getting the destination of the symlink to not realize that its
gotten the full destination, but keeps trying to read past the end of
the destination. The bytes after the end are generally NULL padding
bytes, but that's not a valid component type (ECMA-167 14.16.1.1). So
grub_udf_read_symlink() branches to error logic, returning NULL, instead
of the symlink destination path.
The result of this bug is that the UDF filesystem tests were failing in
the symlink test with the grub-fstest error message:
grub-fstest: error: cannot open `(loop0)/sym': invalid symlink.
This change stores the result of doubling sz in another local variable s,
so as not to modify sz. Also remove unnecessary grub_add(), which increased
the output by 1, presumably to account for a NULL byte. This isn't needed
because an output buffer of size twice sz is already guaranteed to be more
than enough to contain the path components converted to UTF-8. The value of
sz contains at least 4 bytes for the path component header (ECMA-167 14.16.1),
which means that 2 * 4 bytes are allocated but will not be used for UTF-8
characters, so the NULL byte is accounted for.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The gcc by default assumes executable stack is required if the source
object file doesn't have .note.GNU-stack section in place. If any of the
source objects doesn't incorporate the GNU-stack note, the resulting
program will have executable stack flag set in PT_GNU_STACK program
header to instruct program loader or kernel to set up the executable
stack when program loads to memory.
Usually the .note.GNU-stack section will be generated by gcc
automatically if it finds that executable stack is not required. However
it doesn't take care of generating .note.GNU-stack section for those
object files built from assembler sources. This leads to unnecessary
risk of security of exploiting the executable stack because those
assembler sources don't actually require stack to be executable to work.
The grub-emu and grub-emu-lite are found to flag stack as executable
revealed by execstack tool.
$ mkdir -p build-emu && cd build-emu
$ ../configure --with-platform=emu && make
$ execstack -q grub-core/grub-emu grub-core/grub-emu-lite
X grub-core/grub-emu
X grub-core/grub-emu-lite
This patch will add the missing GNU-stack note to the assembler source
used by both utilities, therefore the result doesn't count on gcc
default behavior and the executable stack is disabled.
$ execstack -q grub-core/grub-emu grub-core/grub-emu-lite
- grub-core/grub-emu
- grub-core/grub-emu-lite
Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This conforms to the behavior of the -s option of the Bash read command.
docs/grub: Document the -s option for the read command.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This is primarily useful to do something like "loopback newdev (dev)8+" to
create a device that skips the first 4 KiB, which may contain a container
header, e.g. a non-standard RAID1 header, that GRUB does not recognize. This
would allow that container data to be potentially accessed up to the end of
container, which may be necessary for some layouts that store data at the
end. There is currently not a good way to programmatically get the number
of sectors on a disk to set the appropriate length of the blocklist.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The commit 8b1e5d193 (fs/xfs: Add bigtime incompat feature support)
introduced the bigtime support by adding some features in v3 inodes.
This change extended grub_xfs_inode struct by 76 bytes but also changed
the computation of XFS_V2_INODE_SIZE and XFS_V3_INODE_SIZE. Prior this
commit, XFS_V2_INODE_SIZE was 100 bytes. After the commit it's 84 bytes
XFS_V2_INODE_SIZE becomes 16 bytes too small.
As a result, the data structures aren't properly aligned and the GRUB
generates "attempt to read or write outside of partition" errors when
trying to read the XFS filesystem:
GNU GRUB version 2.11
....
grub> set debug=efi,gpt,xfs
grub> insmod part_gpt
grub> ls (hd0,gpt1)/
partmap/gpt.c:93: Read a valid GPT header
partmap/gpt.c:115: GPT entry 0: start=4096, length=1953125
fs/xfs.c:931: Reading sb
fs/xfs.c:270: Validating superblock
fs/xfs.c:295: XFS v4 superblock detected
fs/xfs.c:962: Reading root ino 128
fs/xfs.c:515: Reading inode (128) - 64, 0
fs/xfs.c:515: Reading inode (739521961424144223) - 344365866970255880, 3840
error: attempt to read or write outside of partition.
This commit change the XFS_V2_INODE_SIZE computation by subtracting 76
bytes instead of 92 bytes from the actual size of grub_xfs_inode struct.
This 76 bytes value comes from added members:
20 grub_uint8_t unused5
1 grub_uint64_t flags2
48 grub_uint8_t unused6
This patch explicitly splits the v2 and v3 parts of the structure.
The unused4 is still ending of the v2 structures and the v3 starts
at unused5. Thanks to this we will avoid future corruptions of v2
or v3 inodes.
The XFS_V2_INODE_SIZE is returning to its expected size and the
filesystem is back to a readable state:
GNU GRUB version 2.11
....
grub> set debug=efi,gpt,xfs
grub> insmod part_gpt
grub> ls (hd0,gpt1)/
partmap/gpt.c:93: Read a valid GPT header
partmap/gpt.c:115: GPT entry 0: start=4096, length=1953125
fs/xfs.c:931: Reading sb
fs/xfs.c:270: Validating superblock
fs/xfs.c:295: XFS v4 superblock detected
fs/xfs.c:962: Reading root ino 128
fs/xfs.c:515: Reading inode (128) - 64, 0
fs/xfs.c:515: Reading inode (128) - 64, 0
fs/xfs.c:931: Reading sb
fs/xfs.c:270: Validating superblock
fs/xfs.c:295: XFS v4 superblock detected
fs/xfs.c:962: Reading root ino 128
fs/xfs.c:515: Reading inode (128) - 64, 0
fs/xfs.c:515: Reading inode (128) - 64, 0
fs/xfs.c:515: Reading inode (128) - 64, 0
fs/xfs.c:515: Reading inode (131) - 64, 768
efi/ fs/xfs.c:515: Reading inode (3145856) - 1464904, 0
grub2/ fs/xfs.c:515: Reading inode (132) - 64, 1024
grub/ fs/xfs.c:515: Reading inode (139) - 64, 2816
grub>
Fixes: 8b1e5d193 (fs/xfs: Add bigtime incompat feature support)
Signed-off-by: Erwan Velu <e.velu@criteo.com>
Tested-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Avoid a warning
lib/libgcrypt-grub/cipher/rijndael.c:229:9:
warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
229 | ;
| ^
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Avoid a warning
lib/libgcrypt-grub/cipher/rijndael.c:352:21: warning:
comparison of integer expressions of different signedness:
‘int’ and ‘unsigned int’ [-Wsign-compare]
352 | for (i = 0; i < keylen; i++)
|
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In the case that one passes a write mask with ":" the write_mask is
obtained from grub_strtoul() and then promptly overwritten by 0xffffffff
three lines later.
This appears to have been so since the initial version of setpci in 2009.
I'm surprised no one else has hit this issue in the past 12 years...
Signed-off-by: Wouter van Kesteren <woutershep@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The sysfs_partition_path() calls udevadm to resolve the sysfs path for
a block device. That can be accomplished by stating the device node
and using the major/minor to follow the symlinks in /sys/dev/block/.
This cuts the execution time of grub-mkconfig to somewhere near 55% on
system without LVM (which uses libdevmapper instead sysfs_partition_path()).
Remove udevadm call as it does not help us more than calling stat() directly.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
... to factor out fix for glibc 2.25 introduced in 7a5b301e3 (build: Use
AC_HEADER_MAJOR to find device macros).
Note: Once glibc 2.25 is old enough and this fix is not needed also
AC_HEADER_MAJOR in configure.ac should be removed.
Signed-off-by: Petr Vorel <pvorel@suse.cz>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The HEAP_MAX_ADDR is confusing. Currently it is set to 32MB, except on
ieee1275 on x86, where it is 64MB.
There is a comment which purports to explain it:
/* If possible, we will avoid claiming heap above this address, because it
seems to cause relocation problems with OSes that link at 4 MiB */
This doesn't make a lot of sense when the constants are well above 4MB
already. It was not always this way. Prior to commit 7b5d0fe4440c
(Increase heap limit) in 2010, HEAP_MAX_SIZE and HEAP_MAX_ADDR were
indeed 4MB. However, when the constants were increased the comment was
left unchanged.
It's been over a decade. It doesn't seem like we have problems with
claims over 4MB on powerpc or x86 ieee1275. The SPARC does things
completely differently and never used the constant.
Drop the constant and the check.
The only use of HEAP_MIN_SIZE was to potentially override the
HEAP_MAX_ADDR check. It is now unused. Remove it too.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Tested-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This incompat feature is used to denote that the filesystem stored its
metadata checksum seed in the superblock. This is used to allow tune2fs
changing the UUID on a mounted metdata_csum filesystem without having
to rewrite all the disk metadata. However, the GRUB doesn't use the
metadata checksum at all. So, it can just ignore this feature if it
is enabled. This is consistent with the GRUB filesystem code in general
which just does a best effort to access the filesystem's data.
The checksum seed incompat feature has to be removed from the ignore
list if the support for metadata checksum verification is added to the
GRUB ext2 driver later.
Suggested-by: Eric Sandeen <esandeen@redhat.com>
Suggested-by: Lukas Czerner <lczerner@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Lukas Czerner <lczerner@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The underlying type of 1ULL does not change across architectures but
grub_uint64_t does. This allows using the BF64_*CODE() macros as
arguments to format string functions that use the PRI* format string
macros that also vary with architecture.
Change the grub_error() call, where this was previously an issue and
temporarily fixed by casting and using a format string literal code,
to now use PRI* macros and remove casting.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The functions grub_util_exec_pipe() and grub_util_exec_pipe_stderr()
currently call execvp(). If the call fails for any reason, the child
currently calls exit(127). This in turn executes the parents
atexit() handlers from the forked child, and then the same handlers
are called again from parent. This is usually not desired, and can
lead to deadlocks, and undesired behavior. So, change the exit() calls
to _exit() calls to avoid calling atexit() handlers from child.
Fixes: e75cf4a58 (unix exec: avoid atexit handlers when child exits)
Signed-off-by: Dimitri John Ledkov <xnox@ubuntu.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
