Squashfs_block_size() sanity checks the data and fragment block size read from disk. However, it doesn't do all the sanity checking that it could do. This means detectably invalid block sizes are passed down to squashfs_read_data(). This situation is sub-optimal because in the case of fragment blocks, a fragment cache buffer has to obtained (with possible waits for a free buffer etc.) before squashfs_read_data() is called. In both fragment and data block cases if the buffer is compressed, it means a free decompressor may be obtained (with possible waits for a decompressor) by squashfs_read_data(), only to have the decompression fail. In both these cases this adds unnecessary overhead which may impact performance, and this is with values that should have been detected earlier. This commit adds extra sanity checking to trap additional incorrect values. Reproducer ========== The reproducer generates a Squashfs filesystem and then corrupts the block size value for file zero block 0. If the exploit succeeds this corrupted value is not detected, and it is passed to the decompressor via squashfs_read_data() which then fails at that point. ==================================================================== #!/bin/bash ARGS="-quiet -no-progress -no-fragments -noI -noD -no-sparse -comp gzip" finish() { popd 2>&1 > /dev/null rm -rf $DIR exit 1 } DIR=$(mktemp -d) pushd $DIR 2>&1 > /dev/null echo Create Squashfs filesystem with one file echo "/ d 0777 0 0" > pseudo echo "zero f 0777 0 0 dd if=/dev/zero bs=1024 count=128 2>/dev/null" >> pseudo if ! mksquashfs - img.sqsh -pf pseudo $ARGS; then echo Failed to create Squashfs filesystem finish fi echo Corrupt the data block size for block 0 in file zero echo Flip the compression bit from uncompressed to compressed. echo "This is at byte offset 131207 (96 + 128 KiB + 37)" dd if=/dev/zero of=img.sqsh seek=131205 bs=1 count=1 conv=notrunc status=none echo Mount the filesystem to /mnt if ! mount -t squashfs img.sqsh /mnt; then echo Mount of Squashfs filesystem failed finish fi echo Read the file zero. echo If the exploit succeeds, dmesg should show the following: echo "SQUASHFS error: zlib decompression failed, data probably corrupt" echo "SQUASHFS error: Failed to read block 0x60: -5" echo "SQUASHFS error: zlib decompression failed, data probably corrupt" echo "SQUASHFS error: Failed to read block 0x60: -5" echo "SQUASHFS error: Unable to read data cache entry [60]" echo "SQUASHFS error: Unable to read page, block 60, size 20000" dd if=/mnt/zero popd 2>&1> /dev/null rm -rf $DIR ============================================================================== Fixes: 1701aecb6849c ("Squashfs: regular file operations") Signed-off-by: Phillip Lougher --- v2: Add additional sanity check, a zero size cannot have SQUASHFS_COMPRESSED_BIT_BLOCK set. --- fs/squashfs/file.c | 8 +++++--- fs/squashfs/fragment.c | 2 +- fs/squashfs/squashfs_fs.h | 6 ------ fs/squashfs/squashfs_helpers.h | 36 ++++++++++++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 10 deletions(-) diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c index 2396fad1de61..96c15088cdee 100644 --- a/fs/squashfs/file.c +++ b/fs/squashfs/file.c @@ -166,6 +166,7 @@ static long long read_indexes(struct super_block *sb, s64 n, { int err, i; long long block = 0; + struct squashfs_sb_info *msblk = sb->s_fs_info; __le32 *blist = kmalloc(PAGE_SIZE, GFP_KERNEL); if (blist == NULL) { @@ -185,7 +186,7 @@ static long long read_indexes(struct super_block *sb, s64 n, } for (i = 0; i < blocks; i++) { - int size = squashfs_block_size(blist[i]); + int size = squashfs_data_size(blist[i], msblk->block_size); if (size < 0) { err = size; goto failure; @@ -333,6 +334,7 @@ static int read_blocklist_ptrs(struct inode *inode, s64 index, u64 *start, { long long blks; __le32 size; + struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info; s64 res = fill_meta_index(inode, index, start, offset, block); TRACE("read_blocklist: res %lld, index %lld, start 0x%llx, offset 0x%x, block 0x%llx\n", @@ -362,7 +364,7 @@ static int read_blocklist_ptrs(struct inode *inode, s64 index, u64 *start, sizeof(size)); if (res < 0) return res; - return squashfs_block_size(size); + return squashfs_data_size(size, msblk->block_size); } static inline int read_blocklist(struct inode *inode, s64 index, u64 *block) @@ -720,7 +722,7 @@ static loff_t seek_hole_data(struct file *file, loff_t offset, int whence) goto finished; for (i = 0; i < indexes; i++) { - length = squashfs_block_size(blist[i]); + length = squashfs_data_size(blist[i], msblk->block_size); if (length < 0) { offset = length; goto finished; diff --git a/fs/squashfs/fragment.c b/fs/squashfs/fragment.c index 9f63e5b5c513..1f372374c29d 100644 --- a/fs/squashfs/fragment.c +++ b/fs/squashfs/fragment.c @@ -54,7 +54,7 @@ int squashfs_frag_lookup(struct super_block *sb, unsigned int fragment, return size; *fragment_block = le64_to_cpu(fragment_entry.start_block); - return squashfs_block_size(fragment_entry.size); + return squashfs_frag_size(fragment_entry.size, msblk->block_size); } diff --git a/fs/squashfs/squashfs_fs.h b/fs/squashfs/squashfs_fs.h index 3e7d0730b2d9..6ca8d7227a0e 100644 --- a/fs/squashfs/squashfs_fs.h +++ b/fs/squashfs/squashfs_fs.h @@ -101,12 +101,6 @@ #define SQUASHFS_XATTR_VALUE_OOL 256 #define SQUASHFS_XATTR_PREFIX_MASK 0xff -static inline int squashfs_block_size(__le32 raw) -{ - u32 size = le32_to_cpu(raw); - return (size >> 25) ? -EIO : size; -} - /* cached data constants for filesystem */ #define SQUASHFS_CACHED_BLKS 8 diff --git a/fs/squashfs/squashfs_helpers.h b/fs/squashfs/squashfs_helpers.h index 4fb13096fcbc..d46d123aefc4 100644 --- a/fs/squashfs/squashfs_helpers.h +++ b/fs/squashfs/squashfs_helpers.h @@ -204,4 +204,40 @@ static inline s16 squashfs_xattr_offset(s64 xattr) { return xattr & 0xffff; } + +static inline int squashfs_block_size(u32 size, s32 block_size) +{ + /* reject any size with bits set where they should not be */ + if ((size & ~SQUASHFS_COMPRESSED_BIT_BLOCK) > block_size) + return -EIO; + + /* a block can't be both full size and compressed */ + if (size == block_size) + return -EIO; + + /* a zero size block will never have SQUASHFS_COMPRESSED_BIT_BLOCK set */ + if (size == SQUASHFS_COMPRESSED_BIT_BLOCK) + return -EIO; + + return size; +} + +static inline int squashfs_data_size(__le32 raw, s32 block_size) +{ + return squashfs_block_size(le32_to_cpu(raw), block_size); +} + +static inline int squashfs_frag_size(__le32 raw, s32 block_size) +{ + int size = squashfs_block_size(le32_to_cpu(raw), block_size); + + if (size < 0) + return size; + + /* a fragment block can't be zero size */ + if (size == 0) + return -EIO; + + return size; +} #endif -- 2.47.3