The corpus was generating issues in grub_btrfs_read_logical() when
attempting to iterate over stripe entries in the superblock's
bootmapping.
In most cases the reason for the failure was that the number of stripes
in chunk->nstripes exceeded the possible space statically allocated in
superblock bootmapping space. Each stripe entry in the bootmapping block
consists of a grub_btrfs_key followed by a grub_btrfs_chunk_stripe.
Another issue that came up was that while calculating the chunk size,
in an earlier piece of code in that function, depending on the data
provided in the btrfs file system, it would end up calculating a size
that was too small to contain even 1 grub_btrfs_chunk_item, which is
obviously invalid too.
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The fuzzer is generating btrfs file systems that have chunks with
invalid combinations of stripes and substripes for the given RAID
configurations.
After examining the Linux kernel fs/btrfs/tree-checker.c code, it
appears that sub-stripes should only be applied to RAID10, and in that
case there should only ever be 2 of them.
Similarly, RAID single should only have 1 stripe, and RAID1/1C3/1C4
should have 2. 3 or 4 stripes respectively, which is what redundancy
corresponds.
Some of the chunks ended up with a size of 0, which grub_malloc() still
returned memory for and in turn generated ASAN errors later when
accessed.
While it would be possible to specifically limit the number of stripes,
a more correct test was on the combination of the chunk item, and the
number of stripes by the size of the chunk stripe structure in
comparison to the size of the chunk itself.
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
According to the btrfs code in Linux, the structure of a directory item
leaf should be of the form:
|struct btrfs_dir_item|name|data|
in GRUB the name len and data len are in the grub_btrfs_dir_item
structure's n and m fields respectively.
The combined size of the structure, name and data should be less than
the allocated memory, a difference to the Linux kernel's struct
btrfs_dir_item is that the grub_btrfs_dir_item has an extra field for
where the name is stored, so we adjust for that too.
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
A corrupt f2fs file system might specify a name length which is greater
than the maximum name length supported by the GRUB f2fs driver.
We will allocate enough memory to store the overly long name, but there
are only F2FS_NAME_LEN bytes in the source, so we would read past the end
of the source.
While checking directory entries, do not copy a file name with an invalid
length.
Signed-off-by: Sudhakar Kuppusamy <sudhakar@linux.ibm.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
A corrupt f2fs filesystem could have a block offset or a bitmap
offset that would cause us to read beyond the bounds of the nat
bitmap.
Introduce the nat_bitmap_size member in grub_f2fs_data which holds
the size of nat bitmap.
Set the size when loading the nat bitmap in nat_bitmap_ptr(), and
catch when an invalid offset would create a pointer past the end of
the allocated space.
Check against the bitmap size in grub_f2fs_test_bit() test bit to avoid
reading past the end of the nat bitmap.
Signed-off-by: Sudhakar Kuppusamy <sudhakar@linux.ibm.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
A corrupt f2fs file system could specify a nat journal entry count
that is beyond the maximum NAT_JOURNAL_ENTRIES.
Check if the specified nat journal entry count before accessing the
array, and throw an error if it is too large.
Signed-off-by: Sudhakar Kuppusamy <sudhakar@linux.ibm.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
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>
Origin: upstream, https://git.savannah.gnu.org/cgit/grub.git/commit/?id=a4b495520e4dc41a896a8b916a64eda9970c50ea
Last-Update: 2021-09-24
Patch-Name: xfs-fix-v4-superblock.patch
The XFS now has an incompat feature flag to indicate that a filesystem
needs to be repaired. The Linux kernel refuses to mount the filesystem
that has it set and only the xfs_repair tool is able to clear that flag.
The GRUB doesn't have the concept of mounting filesystems and just
attempts to read the files. But it does some sanity checking before
attempting to read from the filesystem. Among the things which are tested,
is if the super block only has set of incompatible features flags that
are supported by GRUB. If it contains any flags that are not listed as
supported, reading the XFS filesystem fails.
Since the GRUB doesn't attempt to detect if the filesystem is inconsistent
nor replays the journal, the filesystem access is a best effort. For this
reason, ignore if the filesystem needs to be repaired and just print a debug
message. That way, if reading or booting fails later, the user is able to
figure out that the failures can be related to broken XFS filesystem.
Suggested-by: Eric Sandeen <esandeen@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The XFS filesystem supports a bigtime feature to overcome y2038 problem.
This patch makes the GRUB able to support the XFS filesystems with this
feature enabled.
The XFS counter for the bigtime enabled timestamps starts at 0, which
translates to GRUB_INT32_MIN (Dec 31 20:45:52 UTC 1901) in the legacy
timestamps. The conversion to Unix timestamps is made before passing the
value to other GRUB functions.
For this to work properly, GRUB requires an access to flags2 field in the
XFS ondisk inode. So, the grub_xfs_inode structure has been updated to
cover full ondisk inode.
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Some filesystems nowadays use 64-bit types for timestamps. So, update
grub_dirhook_info struct to use an grub_int64_t type to store mtime.
This also updates the grub_unixtime2datetime() function to receive
a 64-bit timestamp argument and do 64-bit-safe divisions.
All the remaining conversion from 32-bit to 64-bit should be safe, as
32-bit to 64-bit attributions will be implicitly casted. The most
critical part in the 32-bit to 64-bit conversion is in the function
grub_unixtime2datetime() where it needs to deal with the 64-bit type.
So, for that, the grub_divmod64() helper has been used.
These changes enables the GRUB to support dates beyond y2038.
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The ext2 (and ext3, ext4) filesystems write the number of free inodes to
location 0x410.
On a MINIX filesystem, that same location is used for the MINIX superblock
magic number.
If the number of free inodes on an ext2 filesystem is equal to any
of the four MINIX superblock magic values plus any multiple of 65536,
GRUB's MINIX filesystem code will probe it as a MINIX filesystem.
In the case of an OS using ext2 as the root filesystem, since there will
ordinarily be some amount of file creation and deletion on every bootup,
it effectively means that this situation has a 1:16384 chance of being hit
on every reboot.
This will cause GRUB's filesystem probing code to mistakenly identify an
ext2 filesystem as MINIX. This can be seen by e.g. "search --label"
incorrectly indicating that no such ext2 partition with matching label
exists, whereas in fact it does.
After spotting the rough cause of the issue I was facing here, I borrowed
much of the diagnosis/explanation from meierfra who found and investigated
the same issue in util-linux in 2010:
https://bugs.launchpad.net/ubuntu/+source/util-linux/+bug/518582
This was fixed in util-linux by having the MINIX code check for the
ext2 magic. Do the same here.
Signed-off-by: Daniel Drake <drake@endlessm.com>
Reviewed-by: Derek Foreman <derek@endlessos.org>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This is a temporary, less-intrusive change to get the build to success with
compiler format string checking turned on. There is a better fix which
addresses this issue, but it needs more testing. Use this change so that
format string checking on grub_error() can be turned on until the better
change is fully tested.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
We encountered a file not found error when the symlink filesize is
equal to 60:
$ ls -l initrd
lrwxrwxrwx 1 root root 60 Jan 6 16:37 initrd -> secure-core-image-initramfs-5.10.2-yoctodev-standard.cpio.gz
When booting, we got the following error in the GRUB:
error: file `/initrd' not found
The root cause is that the size of diro->inode.symlink is equal to 60
and a symlink name has to be terminated with NUL there. So, if the
symlink filesize is exactly 60 then it is also stored in a separate
block rather than in the inode itself.
Signed-off-by: Yi Zhao <yi.zhao@windriver.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
We need to check errors before calling into a function that uses the result.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This prevents a divide by zero if nstripes == nparities, and
also prevents propagation of invalid values if nstripes ends up
less than nparities.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
We just introduced an error return in grub_nilfs2_btree_node_lookup().
Make sure the callers catch it.
At the same time, make sure that grub_nilfs2_btree_node_lookup() always
inits the index pointer passed to it.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
NILFS2 reads the number of children a node has from the node. Unfortunately,
that's not trustworthy. Check if it's beyond what the filesystem permits and
reject it if so.
This blocks some OOB reads. I'm not sure how controllable the read is and what
could be done with invalidly read data later on.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
NILFS2 has up to 7 keys, per the data structure. Do not permit array
indices in excess of that.
This catches some OOB reads. I don't know how controllable the invalidly
read data is or if that could be used later in the program.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
It's possible with a fuzzed filesystem for JFS to keep getblk()-ing
the same data over and over again, leading to stack exhaustion.
Check if we'd be calling the function with exactly the same data as
was passed in, and if so abort.
I'm not sure what the performance impact of this is and am open to
better ideas.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
getblk() implicitly trusts that treehead->count is an accurate count of
the number of extents. However, that value is read from disk and is not
trustworthy, leading to OOB reads and crashes. I am not sure to what
extent the data read from OOB can influence subsequent program execution.
Require callers to pass in the maximum number of extents for which
they have storage.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Fuzzing JFS revealed crashes where a negative number would be passed
to le_to_cpu16_copy(). There it would be cast to a large positive number
and the copy would read and write off the end of the respective buffers.
Catch this at the top as well as the bottom of the loop.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
There's a read of the name of the root object that assumes that the name
is nul-terminated within the root block. This isn't guaranteed - it seems
SFS would require you to read multiple blocks to get a full name in general,
but maybe that doesn't apply to the root object.
Either way, figure out how much space is left in the root block and don't
over-read it. This fixes some OOB reads.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
HFS has issues such as infinite mutual recursion that are simply too
complex to fix for such a legacy format. So simply do not permit
it to be loaded under lockdown.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Valgrind identified the following use of uninitialized data:
==2782220== Conditional jump or move depends on uninitialised value(s)
==2782220== at 0x42B364: grub_hfsplus_btree_search (hfsplus.c:566)
==2782220== by 0x42B21D: grub_hfsplus_read_block (hfsplus.c:185)
==2782220== by 0x42A693: grub_fshelp_read_file (fshelp.c:386)
==2782220== by 0x42C598: grub_hfsplus_read_file (hfsplus.c:219)
==2782220== by 0x42C598: grub_hfsplus_mount (hfsplus.c:330)
==2782220== by 0x42B8C5: grub_hfsplus_dir (hfsplus.c:958)
==2782220== by 0x4C1AE6: grub_fs_probe (fs.c:73)
==2782220== by 0x407C94: grub_ls_list_files (ls.c:186)
==2782220== by 0x407C94: grub_cmd_ls (ls.c:284)
==2782220== by 0x4D7130: grub_extcmd_dispatcher (extcmd.c:55)
==2782220== by 0x4045A6: execute_command (grub-fstest.c:59)
==2782220== by 0x4045A6: fstest (grub-fstest.c:433)
==2782220== by 0x4045A6: main (grub-fstest.c:772)
==2782220== Uninitialised value was created by a heap allocation
==2782220== at 0x483C7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==2782220== by 0x4C0305: grub_malloc (mm.c:42)
==2782220== by 0x42C21D: grub_hfsplus_mount (hfsplus.c:239)
==2782220== by 0x42B8C5: grub_hfsplus_dir (hfsplus.c:958)
==2782220== by 0x4C1AE6: grub_fs_probe (fs.c:73)
==2782220== by 0x407C94: grub_ls_list_files (ls.c:186)
==2782220== by 0x407C94: grub_cmd_ls (ls.c:284)
==2782220== by 0x4D7130: grub_extcmd_dispatcher (extcmd.c:55)
==2782220== by 0x4045A6: execute_command (grub-fstest.c:59)
==2782220== by 0x4045A6: fstest (grub-fstest.c:433)
==2782220== by 0x4045A6: main (grub-fstest.c:772)
This happens when the process of reading the catalog file goes sufficiently
wrong that there's an attempt to read the extent overflow file, which has
not yet been loaded. Keep track of when the extent overflow file is
fully loaded and refuse to use it before then.
The load valgrind doesn't like is btree->nodesize, and that's then used
to allocate a data structure. It looks like there are subsequently a lot
of reads based on that pointer so OOB reads are likely, and indeed crashes
(albeit difficult-to-replicate ones) have been observed in fuzzing.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Otherwise you get a wild pointer, leading to a bunch of invalid reads.
Check it falls inside the given node.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
A fuzzed HFS+ filesystem had log2blocksize = 22. This gave
log2blocksize + GRUB_DISK_SECTOR_BITS = 31. 1 << 31 = 0x80000000,
which is -1 as an int. This caused some wacky behavior later on in
the function, leading to out-of-bounds writes on the destination buffer.
Catch log2blocksize + GRUB_DISK_SECTOR_BITS >= 31. We could be stricter,
but this is the minimum that will prevent integer size weirdness.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The node structure reference is being allocated but not freed if it
reaches the end of the function. If any of the hooks had returned
a non-zero value, then node would have been copied in to the context
reference, but otherwise node is not stored and should be freed.
Similarly, the call to grub_affs_create_node() replaces the allocated
memory in node with a newly allocated structure, leaking the existing
memory pointed by node.
Finally, when dir->parent is set, then we again replace node with newly
allocated memory, which seems unnecessary when we copy in the values
from dir->parent immediately after.
Fixes: CID 73759
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
While arguably the check for grub_errno is correct, we should really be
checking the return value from the function since it is always possible
that grub_errno was set elsewhere, making this code behave incorrectly.
Fixes: CID 73668
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In all cases the problem is that the value being acted upon by
a left-shift is a 32-bit number which is then being used in the
context of a 64-bit number.
To avoid overflow we ensure that the number being shifted is 64-bit
before the shift is done.
Fixes: CID 73684, CID 73695, CID 73764
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
There are several exit points in dnode_get_path() that are causing possible
memory leaks.
In the while(1) the correct exit mechanism should not be to do a direct return,
but to instead break out of the loop, setting err first if it is not already set.
The reason behind this is that the dnode_path is a linked list, and while doing
through this loop, it is being allocated and built up - the only way to
correctly unravel it is to traverse it, which is what is being done at the end
of the function outside of the loop.
Several of the existing exit points correctly did a break, but not all so this
change makes that more consistent and should resolve the leaking of memory as
found by Coverity.
Fixes: CID 73741
Signed-off-by: Paulo Flabiano Smorigo <pfsmorigo@canonical.com>
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
While it is possible for the return value from zfs_log2() to be zero
(0), it is quite unlikely, given that the previous assignment to blksz
is shifted up by SPA_MINBLOCKSHIFT (9) before 9 is subtracted at the
assignment to epbs.
But, while unlikely during a normal operation, it may be that a carefully
crafted ZFS filesystem could result in a zero (0) value to the
dn_datalbkszsec field, which means that the shift left does nothing
and assigns zero (0) to blksz, resulting in a negative epbs value.
Fixes: CID 73608
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
HFS+ documentation suggests that the maximum filename and volume name is
255 Unicode characters in length.
So, when converting from big-endian to little-endian, we should ensure
that the name of the volume has a length that is between 0 and 255,
inclusive.
Fixes: CID 73641
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
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>
Both node->size and node->namelen come from the supplied filesystem,
which may be user-supplied. We can't trust them for the math unless we
know they don't overflow. Making sure they go through grub_add() or
grub_calloc() first will give us that.
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This attempts to fix the places where we do the following where
arithmetic_expr may include unvalidated data:
X = grub_malloc(arithmetic_expr);
It accomplishes this by doing the arithmetic ahead of time using grub_add(),
grub_sub(), grub_mul() and testing for overflow before proceeding.
Among other issues, this fixes:
- allocation of integer overflow in grub_video_bitmap_create()
reported by Chris Coulson,
- allocation of integer overflow in grub_png_decode_image_header()
reported by Chris Coulson,
- allocation of integer overflow in grub_squash_read_symlink()
reported by Chris Coulson,
- allocation of integer overflow in grub_ext2_read_symlink()
reported by Chris Coulson,
- allocation of integer overflow in read_section_as_string()
reported by Chris Coulson.
Fixes: CVE-2020-14309, CVE-2020-14310, CVE-2020-14311
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This modifies most of the places we do some form of:
X = malloc(Y * Z);
to use calloc(Y, Z) instead.
Among other issues, this fixes:
- allocation of integer overflow in grub_png_decode_image_header()
reported by Chris Coulson,
- allocation of integer overflow in luks_recover_key()
reported by Chris Coulson,
- allocation of integer overflow in grub_lvm_detect()
reported by Chris Coulson.
Fixes: CVE-2020-14308
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
We bumped into the build error while testing gcc-10 pre-release.
In file included from ../../include/grub/file.h:22,
from ../../grub-core/fs/zfs/zfs.c:34:
../../grub-core/fs/zfs/zfs.c: In function 'zap_leaf_lookup':
../../grub-core/fs/zfs/zfs.c:2263:44: error: array subscript '<unknown>' is outside the bounds of an interior zero-length array 'grub_uint16_t[0]' {aka 'short unsigned int[0]'} [-Werror=zero-length-bounds]
2263 | for (chunk = grub_zfs_to_cpu16 (l->l_hash[LEAF_HASH (blksft, h, l)], endian);
../../include/grub/types.h:241:48: note: in definition of macro 'grub_le_to_cpu16'
241 | # define grub_le_to_cpu16(x) ((grub_uint16_t) (x))
| ^
../../grub-core/fs/zfs/zfs.c:2263:16: note: in expansion of macro 'grub_zfs_to_cpu16'
2263 | for (chunk = grub_zfs_to_cpu16 (l->l_hash[LEAF_HASH (blksft, h, l)], endian);
| ^~~~~~~~~~~~~~~~~
In file included from ../../grub-core/fs/zfs/zfs.c:48:
../../include/grub/zfs/zap_leaf.h:72:16: note: while referencing 'l_hash'
72 | grub_uint16_t l_hash[0];
| ^~~~~~
Here I'd like to quote from the gcc document [1] which seems best to
explain what is going on here.
"Although the size of a zero-length array is zero, an array member of
this kind may increase the size of the enclosing type as a result of
tail padding. The offset of a zero-length array member from the
beginning of the enclosing structure is the same as the offset of an
array with one or more elements of the same type. The alignment of a
zero-length array is the same as the alignment of its elements.
Declaring zero-length arrays in other contexts, including as interior
members of structure objects or as non-member objects, is discouraged.
Accessing elements of zero-length arrays declared in such contexts is
undefined and may be diagnosed."
The l_hash[0] is apparnetly an interior member to the enclosed structure
while l_entries[0] is the trailing member. And the offending code tries
to access members in l_hash[0] array that triggers the diagnose.
Given that the l_entries[0] is used to get proper alignment to access
leaf chunks, we can accomplish the same thing through the ALIGN_UP macro
thus eliminating l_entries[0] from the structure. In this way we can
pacify the warning as l_hash[0] now becomes the last member to the
enclosed structure.
[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This allows comparing file ages on EFI system partitions.
Signed-off-by: David Michael <fedora.dm0@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This provides the node's attributes outside the iterator function
so the file modification time can be accessed and reported.
Signed-off-by: David Michael <fedora.dm0@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
gcc says:
grub-core/fs/squash4.c: In function ‘direct_read’:
grub-core/fs/squash4.c:868:10: error: ‘err’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
868 | if (err)
| ^
cc1: all warnings being treated as errors
This patch initializes it to GRUB_ERR_NONE.
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
New 3- and 4-copy variants of RAID1 were merged into Linux kernel 5.5.
Add the two new profiles to the list of recognized ones. As this builds
on the same code as RAID1, only the redundancy level needs to be
adjusted, the rest is done by the existing code.
Signed-off-by: David Sterba <dsterba@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Disable the -Wadress-of-packaed-member diagnostic for the grub_f2fs_label
function since the result is found to be false postive.
A pointer to the 'volume_name' member of 'struct grub_f2fs_superblock' is
guaranteed to be aligned as the offset of 'volume_name' within the struct
is dividable by the natural alignment on both 32- and 64-bit targets.
grub-core/fs/f2fs.c: In function ‘grub_f2fs_label’:
grub-core/fs/f2fs.c:1253:60: error: taking address of packed member of ‘struct grub_f2fs_superblock’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
1253 | *label = (char *) grub_f2fs_utf16_to_utf8 (data->sblock.volume_name);
| ~~~~~~~~~~~~^~~~~~~~~~~~
cc1: all warnings being treated as errors
Reported-by: Neil MacLeod <neil@nmacleod.com>
Signed-off-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Tested-by: Neil MacLeod <neil@nmacleod.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The function grub_get_node_path() could return uninitialized offset with
level == 0 if the block is greater than direct_index + 2 * direct_blks +
2 * indirect_blks + dindirect_blks. The uninitialized offset is then used
by function grub_f2fs_get_block() because level == 0 is valid and
meaningful return to be processed.
The fix is to set level = -1 as return value by grub_get_node_path() to
signify an error that the input block cannot be handled. Any caller
should therefore check level is negative or not before processing the
output.
Reported-by: Neil MacLeod <neil@nmacleod.com>
Signed-off-by: Michael Chang <mchang@suse.com>
Tested-by: Neil MacLeod <neil@nmacleod.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The catkey->name could be unaligned since the address of 'void* record'
is calculated as offset in bytes to a malloc buffer.
The fix is using aligned buffer allocated by grub_malloc for holding
the UTF16 string copied from catkey->name. And use that buffer as
argument for grub_utf16_to_utf8 to convert to UTF8 strings.
In addition, using a new copy of buffer rather than catkey->name itself
for processing the endianess conversion, we can also get rid of the hunk
restoring byte order of catkey->name to what it was previously.
[ 59s] ../grub-core/fs/hfsplus.c: In function 'list_nodes':
[ 59s] ../grub-core/fs/hfsplus.c:738:57: error: taking address of packed member of 'struct grub_hfsplus_catkey' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[ 59s] 738 | *grub_utf16_to_utf8 ((grub_uint8_t *) filename, catkey->name,
[ 59s] | ~~~~~~^~~~~~
[ 59s] ../grub-core/fs/hfsplus.c: In function 'grub_hfsplus_label':
[ 59s] ../grub-core/fs/hfsplus.c:1019:57: error: taking address of packed member of 'struct grub_hfsplus_catkey' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[ 59s] 1019 | *grub_utf16_to_utf8 ((grub_uint8_t *) (*label), catkey->name,
[ 59s] | ~~~~~~^~~~~~
Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Disable the -Wadress-of-packaed-member diagnostic for the
grub_jfs_getent function since the result is found to be false postive.
The leaf is read into memory as continous chunks in size of 32 bytes and
the pointer to its base is aligned, which also guarentee its member
leaf->namepart is aligned.
[ 60s] ../grub-core/fs/jfs.c: In function 'grub_jfs_getent':
[ 60s] ../grub-core/fs/jfs.c:557:44: error: taking address of packed member of 'struct grub_jfs_leaf_dirent' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[ 60s] 557 | le_to_cpu16_copy (filename + strpos, leaf->namepart, len < diro->data->namecomponentlen ? len
[ 60s] | ~~~~^~~~~~~~~~
[ 60s] ../grub-core/fs/jfs.c:570:48: error: taking address of packed member of 'struct grub_jfs_leaf_next_dirent' may result in an unaligned pointer value [-Werror=address-of-packed-member]
[ 60s] 570 | le_to_cpu16_copy (filename + strpos, next_leaf->namepart, len < 15 ? len : 15);
[ 60s] | ~~~~~~~~~^~~~~~~~~~
[ 60s] cc1: all warnings being treated as errors
Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
