| Age | Commit message (Collapse) | Author |
|---|
|
|
|
A crafted squashfs image could embed a huge number of empty metadata
blocks in order to make the amount of malloc()'d memory overflow and be
much smaller than expected. Because of this flaw, any random code
positioned at the right location in the squashfs image could be memcpy'd
from the squashfs structures into U-Boot code location while trying to
access the rearmost blocks, before being executed.
In order to prevent this vulnerability from being exploited in eg. a
secure boot environment, let's add a check over the amount of data
that is going to be allocated. Such a check could look like:
if (!elem_size || n > SIZE_MAX / elem_size)
return NULL;
The right way to do it would be to enhance the calloc() implementation
but this is quite an impacting change for such a small fix. Another
solution would be to add the check before the malloc call in the
squashfs implementation, but this does not look right. So for now, let's
use the kcalloc() compatibility function from Linux, which has this
check.
Fixes: c5100613037 ("fs/squashfs: new filesystem")
Reported-by: Tatsuhiko Yasumatsu <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
Tested-by: Tatsuhiko Yasumatsu <[email protected]>
|
|
Merge in v2022.07-rc5.
|
|
Following Jincheng's report, an out-of-band write leading to arbitrary
code execution is possible because on one side the squashfs logic
accepts directory names up to 65535 bytes (u16), while U-Boot fs logic
accepts directory names up to 255 bytes long.
Prevent such an exploit from happening by capping directory name sizes
to 255. Use a define for this purpose so that developers can link the
limitation to its source and eventually kill it some day by dynamically
allocating this array (if ever desired).
Link: https://lore.kernel.org/all/CALO=DHFB+yBoXxVr5KcsK0iFdg+e7ywko4-e+72kjbcS8JBfPw@mail.gmail.com
Reported-by: Jincheng Wang <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
Tested-by: Jincheng Wang <[email protected]>
|
|
Setting sblk = NULL has no effect on the caller.
We want to set *sblk = NULL if an error occurrs to avoid usage after free.
Signed-off-by: Heinrich Schuchardt <[email protected]>
|
|
Signed-off-by: Pali Rohár <[email protected]>
Reviewed-by: Miquel Raynal <[email protected]>
|
|
When compling for x86:
ld.bfd: fs/squashfs/sqfs.o: in function `sqfs_read':
u-boot/fs/squashfs/sqfs.c:1443: undefined reference to `__udivmoddi4'
ld.bfd: u-boot/fs/squashfs/sqfs.c:1521: undefined reference to `__udivmoddi4'
Signed-off-by: Sean Nyekjaer <[email protected]>
Reviewed-by: Miquel Raynal <[email protected]>
Reviewed-by: Pali Rohár <[email protected]>
|
|
* Don't check argument of free(). Free does this itself.
* Reduce scope of data_buffer. Remove duplicate free().
* Avoid superfluous NULL assignment.
Signed-off-by: Heinrich Schuchardt <[email protected]>
Reviewed-by: Miquel Raynal <[email protected]>
|
|
Signed-off-by: Lars Weber <[email protected]>
|
|
This message comes up a lot when scanning filesystems. It suggests to the
user that there is some sort of error, but in fact there is no reason to
expect that a particular partition has a sqfs filesystem. Other
filesystems don't print this error.
Turn it into a debug message.
Signed-off-by: Simon Glass <[email protected]>
Reviewed-by: Miquel Raynal <[email protected]>
|
|
In SquashFS, the contents of a directory is stored by
squashfs_directory_entry structures which contain the file's name, inode
and position within the filesystem.
The inode number is not stored directly; instead each directory has one
or more headers which set a base inode number, and files store the
offset from that to the file's inode number.
In mksquashfs, each inode is allocated a number in the same order as
they are written to the directory table; thus the offset from the
header's base inode number to the file's inode number is usually
positive.
Hardlinks are simply stored with two directory entries referencing the
same file. This means the second entry will thus have an inode number
much lower than the surrounding files. Since the header's base inode
number comes from the first entry that uses the header, this delta will
usually be negative.
Previously, U-Boot's squashfs_directory_entry.inode_offset field was
declared as an unsigned value. Thus when a negative value was found, it
would either resolve to an invalid inode number or to that of an
unrelated file.
A squashfs image to test this can be created like so:
echo hi > sqfs_test_files/001-root-file
mkdir sqfs_test_files/002-subdir
touch sqfs_test_files/002-subdir/003-file
ln sqfs_test_files/{001-root-file,002-subdir/004-link}
mksquashfs sqfs_test_files/ test.sqfs -noappend
Note that squashfs sorts the files ASCIIbetacally, so we can use the
names to control the order they appear in. The ordering is important -
the first reference to the file must have a lower inode number than the
directory in which the second reference resides, and the second
reference cannot be the first file in the directory.
Listing this sample image in U-Boot results in:
=> sqfsls virtio 2 002-subdir
0 003-file
Inode not found.
0 004-link
Signed-off-by: Campbell Suter <[email protected]>
Reviewed-by: Miquel Raynal <[email protected]>
|
|
The fragmented files were not correctly read because of two issues:
- The squashfs_file_info struct has a field named 'comp', which tells if
the file's fragment is compressed or not. This field was always set to
'true' in sqfs_get_regfile_info and sqfs_get_lregfile_info. It should
actually take sqfs_frag_lookup's return value. This patch addresses
these two assignments.
- In sqfs_read, the fragments (compressed or not) were copied to the
output buffer through a for loop which was reading data at the wrong
offset. Replace these loops by equivalent calls to memcpy, with the
right parameters.
I tested this patch by comparing the MD5 checksum of a few fragmented
files with the respective md5sum output in sandbox, considering both
compressed and uncompressed fragments.
Signed-off-by: Joao Marcos Costa <[email protected]>
Tested-by: Richard Genoud <[email protected]>
Reviewed-by: Miquel Raynal <[email protected]>
|
|
When reading directories the UEFI sub-system must supply file attributes
and timestamps. These fields will have to be added to struct fs_dirent.
SquashFS should not fill these fields with random data. Ensure that they
are zeroed out.
Signed-off-by: Heinrich Schuchardt <[email protected]>
Reviewed-by: Miquel Raynal <[email protected]>
|
|
Commit 401d1c4f5d2d29c4bc4beaec95402ca23eb63295 ("common: Drop
asm/global_data.h from common header") broke compilation of squashfs
filesystem when CONFIG_CMD_SQUASHFS=y is enabled.
Compilation is failing on error:
aarch64-linux-gnu-ld.bfd: u-boot/fs/squashfs/sqfs_inode.c:121: undefined reference to `le32_to_cpu'
Fixes: 401d1c4f5d2d29c4bc4beaec95402ca23eb63295 ("common: Drop asm/global_data.h from common header")
Suggested-by: Tom Rini <[email protected]>
Signed-off-by: Pali Rohár <[email protected]>
Reviewed-by: Tom Rini <[email protected]>
|
|
sqfs_opendir() called in sqfs_size(), sqfs_read(), sqfs_exists() may fail
leading to sqfs_closedir(NULL) being called. Do not dereference NULL.
Signed-off-by: Heinrich Schuchardt <[email protected]>
|
|
SquashFS supports sprase blocks in files - that is, if a given block is
composed only of zeros, it's not written to the output file to save
space and it's on-disk length field is set to zero to indicate that.
Previously the squashfs driver did not recognise that, and would attempt
to read and decompress a zero-sized block, which obviously failed.
The following command may be used to create a file for testing:
cat <(dd if=/dev/urandom of=/dev/stdout bs=1M count=1) \
<(dd if=/dev/zero of=/dev/stdout bs=1M count=1) \
<(dd if=/dev/urandom of=/dev/stdout bs=1k count=200) >test_file
Signed-off-by: Campbell Suter <[email protected]>
|
|
This will prevent a double free error if sqfs_close() is called twice.
Signed-off-by: Richard Genoud <[email protected]>
|
|
This permits to find a file and use the distro_bootcmd
Reviewed-by: Joao Marcos Costa <[email protected]>
Signed-off-by: Richard Genoud <[email protected]>
|
|
offset is the offset in the file read, not the offset in the destination
buffer.
If the offset is not null, this will lead to a memory corruption.
So, for now, we are returning an error if the offset is used.
Signed-off-by: Richard Genoud <[email protected]>
|
|
The length of the buffer wasn't taken into account when writing to the
given buffer.
Signed-off-by: Richard Genoud <[email protected]>
|
|
sqfs_decompressor_init() returns a value, so it's better to use it than
to force the return value to EINVAL (it could be ENOMEM)
Reviewed-by: Joao Marcos Costa <[email protected]>
Signed-off-by: Richard Genoud <[email protected]>
|
|
Resetting the context on error will prevent some checks like:
if (!ctx.cur_dev)
To pass when the probe method has failed
Signed-off-by: Richard Genoud <[email protected]>
|
|
If SquashFS magic number is invalid, there's a memory leak.
Reviewed-by: Joao Marcos Costa <[email protected]>
Signed-off-by: Richard Genoud <[email protected]>
|
|
finfo.blk_sizes may not be freed in case of error in the for loop
Setting it to null and freeing it at the end makes prevents that from
happening.
Reviewed-by: Joao Marcos Costa <[email protected]>
Signed-off-by: Richard Genoud <[email protected]>
|
|
if sqfs_tokenize(rel_tokens, rc, rel); fails, the function exits
without freeing the array base_tokens.
Reviewed-by: Joao Marcos Costa <[email protected]>
Signed-off-by: Richard Genoud <[email protected]>
|
|
the return value of sqfs_tokenize(rel_tokens, rc, rel); wasn't checked.
(but "ret" value was !)
This is obviouly a typo.
Reviewed-by: Joao Marcos Costa <[email protected]>
Signed-off-by: Richard Genoud <[email protected]>
|
|
For consistency with other functions.
Reviewed-by: Joao Marcos Costa <[email protected]>
Signed-off-by: Richard Genoud <[email protected]>
|
|
data_buffer was allocated in a loop and freed only once.
Signed-off-by: Richard Genoud <[email protected]>
|
|
sqfs_closedir() should be called to free memory allocated by
sqfs_opendir()
Reviewed-by: Joao Marcos Costa <[email protected]>
Signed-off-by: Richard Genoud <[email protected]>
|
|
as sqfs_opendir failed, there's no need to call sqfs_closedir
Signed-off-by: Richard Genoud <[email protected]>
|
|
dirs->entry shouldn't be left dangling as it could be freed twice.
Signed-off-by: Richard Genoud <[email protected]>
|
|
as sqfs_opendir failed, there's no need to call sqfs_closedir
Signed-off-by: Richard Genoud <[email protected]>
|
|
dirs->entry shouldn't be left dangling as it could be freed twice.
Signed-off-by: Richard Genoud <[email protected]>
|
|
memory allocation should always be checked
Reviewed-by: Joao Marcos Costa <[email protected]>
Signed-off-by: Richard Genoud <[email protected]>
|
|
inode_table should not be left dangling as it may be freed in sqfs_opendir
Signed-off-by: Richard Genoud <[email protected]>
|
|
path, target, res, rem and sym_tokens were not free on error nor success.
Reviewed-by: Joao Marcos Costa <[email protected]>
Signed-off-by: Richard Genoud <[email protected]>
|
|
dirs->entry shouldn't be left dangling as it could be freed twice.
Signed-off-by: Richard Genoud <[email protected]>
|
|
pos_list wasn't freed on every error
Reviewed-by: Joao Marcos Costa <[email protected]>
Signed-off-by: Richard Genoud <[email protected]>
|
|
*file and *dir were not freed on error
Reviewed-by: Joao Marcos Costa <[email protected]>
Signed-off-by: Richard Genoud <[email protected]>
|
|
sqfs_dirs wasn't freed anywhere.
Reviewed-by: Joao Marcos Costa <[email protected]>
Signed-off-by: Richard Genoud <[email protected]>
|
|
Using only one label permits to prevents bugs when moving code around.
Reviewed-by: Joao Marcos Costa <[email protected]>
Signed-off-by: Richard Genoud <[email protected]>
|
|
When trying to load an non-existing file, the cpu hangs!
Signed-off-by: Richard Genoud <[email protected]>
|
|
I've created a squashfs file system with Yocto (it use squashfs-tools)
and u-boot command sqfsls give the error:'Error while searching inode:
unknown type.'
After some digging in the code I found that the index is off by 1.
This patch fix this issue and I can successful use the sqfsls command.
After search for the squashfs format I found a link talk about a
similar issue but this time in the documentation. The link is:
https://github.com/AgentD/squashfs-tools-ng/commit/e6588526838caece9529
Signed-off-by: Gerard Koskamp <[email protected]>
Tested-by: Joao Marcos Costa <[email protected]>
|
|
We should check if the incoming parameter file_mapping is not NULL instead
of checking after adding an offset.
Reported-by: Coverity CID 307210
Signed-off-by: Heinrich Schuchardt <[email protected]>
Acked-by: Thomas Petazzoni <[email protected]>
|
|
Fix control flow issues and null pointer dereferences.
Signed-off-by: Joao Marcos Costa <[email protected]>
|
|
Fix defects such as uninitialized variables and untrusted pointer
operations. Most part of the tainted variables and the related defects
actually comes from Linux's macro get_unaligned_le**, extensively used
in SquashFS code. Add sanity checks for those variables.
Signed-off-by: Joao Marcos Costa <[email protected]>
|
|
Add call to lzo's lzo1x_decompress_safe() into sqfs_decompress().
U-Boot's LZO sources may still have some unsolved issues that could make the
decompression crash when dealing with fragmented files, so those should be
avoided. The "-no-fragments" option can be passed to mksquashfs.
Signed-off-by: Joao Marcos Costa <[email protected]>
|
|
Add call to ZSTD's ZSTD_decompressDCtx(). In this use case, the caller
can upper bound the decompressed size, which will be the SquashFS data
block (or metadata block) size, so there is no need to use streaming
API. Add ZSTD's worskpace to squashfs_ctxt structure.
Signed-off-by: Joao Marcos Costa <[email protected]>
|
|
Replace 'u16 comp_type' by a reference to squashfs_ctxt structure.
Signed-off-by: Joao Marcos Costa <[email protected]>
|
|
Add sqfs_decompressor_init() and sqfs_decompressor_cleanup(). These
functions are called respectively in sqfs_probe() and sqfs_close(). For
now, only ZSTD requires an initialization logic. ZSTD support will be
added in a follow-up commit.
Move squashfs_ctxt definition to sqfs_filesystem.h. This structure is
passed to sqfs_decompressor_init() and sqfs_decompressor_cleanup(), so
it can no longer be local to sqfs.c.
Signed-off-by: Joao Marcos Costa <[email protected]>
|
