[PATCH 0/6] squashfs: harden against crafted metadata

[PATCH 0/6] squashfs: harden against crafted metadata

[Ahmad Fatoum]

Richard reports[1] that barebox is susceptible to a number of memory safety
issues when parsing crafted squashfs files, which have been fixed in the
upstream Linux implementation in the meantime.

Import the mentioned commits from Linux to fix this:

  01cfb7937a9af ("squashfs: be more careful about metadata corruption")
  d512584780d3e ("squashfs: more metadata hardening")
  cdbb65c4c7ead ("squashfs metadata 2: electric boogaloo")
  71755ee5350b6 ("squashfs: more metadata hardening")
  a3f94cb99a854 ("Squashfs: Compute expected length from inode size rather than block length")

A full synchronization of the squashfs code is probably also in-order,
e.g. to support block sizes other than the default 128K, but
cherry-picking these changes is quite straight-forward, so let's do that
now.

[1]: https://lore.barebox.org/barebox/2572594.vzjCzTo3RI@somecomputer/


Ahmad Fatoum (6):
  squashfs: be more careful about metadata corruption
  squashfs: more metadata hardening
  squashfs metadata 2: electric boogaloo
  squashfs: more metadata hardening
  Squashfs: Compute expected length from inode size rather than block
    length
  squashfs: refuse mount of squashfs images with non-128K block size

 fs/squashfs/Kconfig          |  5 +---
 fs/squashfs/block.c          |  2 ++
 fs/squashfs/cache.c          |  3 ++
 fs/squashfs/file.c           | 57 ++++++++++++++++++++++--------------
 fs/squashfs/file_cache.c     |  7 +++--
 fs/squashfs/fragment.c       | 17 ++++++-----
 fs/squashfs/squashfs.h       |  4 +--
 fs/squashfs/squashfs_fs.h    | 11 +++++++
 fs/squashfs/squashfs_fs_sb.h |  1 +
 fs/squashfs/super.c          | 12 ++++++--
 10 files changed, 79 insertions(+), 40 deletions(-)

-- 
2.39.2

[PATCH 1/6] squashfs: be more careful about metadata corruption

[Ahmad Fatoum]

This is a port of Linux commit 01cfb7937a9af2abb1136c7e89fbf3fd92952956:

| Author:     Linus Torvalds <torvalds@linux-foundation.org>
| AuthorDate: Sun Jul 29 12:44:46 2018 -0700
|
| Anatoly Trosinenko reports that a corrupted squashfs image can cause a
| kernel oops.  It turns out that squashfs can end up being confused about
| negative fragment lengths.
|
| The regular squashfs_read_data() does check for negative lengths, but
| squashfs_read_metadata() did not, and the fragment size code just
| blindly trusted the on-disk value.  Fix both the fragment parsing and
| the metadata reading code.
|
| Reported-by: Anatoly Trosinenko <anatoly.trosinenko@gmail.com>
| Cc: Al Viro <viro@zeniv.linux.org.uk>
| Cc: Phillip Lougher <phillip@squashfs.org.uk>
| Cc: stable@kernel.org
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Reported-by: Richard Weinberger <richard@sigma-star.at>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 fs/squashfs/cache.c       |  3 +++
 fs/squashfs/file.c        |  8 ++++++--
 fs/squashfs/fragment.c    |  4 +---
 fs/squashfs/squashfs_fs.h | 11 +++++++++++
 4 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/fs/squashfs/cache.c b/fs/squashfs/cache.c
index 766bc99493b9..5a027d6fe5d0 100644
--- a/fs/squashfs/cache.c
+++ b/fs/squashfs/cache.c
@@ -284,6 +284,9 @@ int squashfs_read_metadata(struct super_block *sb, void *buffer,
 
 	TRACE("Entered squashfs_read_metadata [%llx:%x]\n", *block, *offset);
 
+	if (unlikely(length < 0))
+		return -EIO;
+
 	while (length) {
 		entry = squashfs_cache_get(sb, msblk->block_cache, *block, 0);
 		if (entry->error) {
diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index 0806f90b9667..1413ef7ecbd8 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -169,7 +169,11 @@ static long long read_indexes(struct super_block *sb, int n,
 		}
 
 		for (i = 0; i < blocks; i++) {
-			int size = le32_to_cpu(blist[i]);
+			int size = squashfs_block_size(blist[i]);
+			if (size < 0) {
+				err = size;
+				goto failure;
+			}
 			block += SQUASHFS_COMPRESSED_SIZE_BLOCK(size);
 		}
 		n -= blocks;
@@ -340,7 +344,7 @@ static int read_blocklist(struct inode *inode, int index, u64 *block)
 			sizeof(size));
 	if (res < 0)
 		return res;
-	return le32_to_cpu(size);
+	return squashfs_block_size(size);
 }
 
 /* Copy data into page cache  */
diff --git a/fs/squashfs/fragment.c b/fs/squashfs/fragment.c
index 2b99ff52e334..343444000e02 100644
--- a/fs/squashfs/fragment.c
+++ b/fs/squashfs/fragment.c
@@ -56,9 +56,7 @@ int squashfs_frag_lookup(struct super_block *sb, unsigned int fragment,
 		return size;
 
 	*fragment_block = le64_to_cpu(fragment_entry.start_block);
-	size = le32_to_cpu(fragment_entry.size);
-
-	return size;
+	return squashfs_block_size(fragment_entry.size);
 }
 
 
diff --git a/fs/squashfs/squashfs_fs.h b/fs/squashfs/squashfs_fs.h
index 279a3db1bcb2..6ce6ed01ba76 100644
--- a/fs/squashfs/squashfs_fs.h
+++ b/fs/squashfs/squashfs_fs.h
@@ -1,5 +1,10 @@
 #ifndef SQUASHFS_FS
 #define SQUASHFS_FS
+
+#include <asm/byteorder.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+
 /*
  * Squashfs
  *
@@ -125,6 +130,12 @@
 
 #define SQUASHFS_COMPRESSED_BLOCK(B)	(!((B) & SQUASHFS_COMPRESSED_BIT_BLOCK))
 
+static inline int squashfs_block_size(__le32 raw)
+{
+	u32 size = le32_to_cpu(raw);
+	return (size >> 25) ? -EIO : size;
+}
+
 /*
  * Inode number ops.  Inodes consist of a compressed block number, and an
  * uncompressed offset within that block
-- 
2.39.2

[PATCH 2/6] squashfs: more metadata hardening

[Ahmad Fatoum]

This is a port of Linux commit d512584780d3e6a7cacb2f482834849453d444a1:

| Author:     Linus Torvalds <torvalds@linux-foundation.org>
| AuthorDate: Mon Jul 30 14:27:15 2018 -0700
|
| Anatoly reports another squashfs fuzzing issue, where the decompression
| parameters themselves are in a compressed block.
|
| This causes squashfs_read_data() to be called in order to read the
| decompression options before the decompression stream having been set
| up, making squashfs go sideways.
|
| Reported-by: Anatoly Trosinenko <anatoly.trosinenko@gmail.com>
| Acked-by: Phillip Lougher <phillip.lougher@gmail.com>
| Cc: stable@kernel.org
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Reported-by: Richard Weinberger <richard@sigma-star.at>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 fs/squashfs/block.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c
index 3e2b9a5ebda8..d65035cead54 100644
--- a/fs/squashfs/block.c
+++ b/fs/squashfs/block.c
@@ -164,6 +164,8 @@ int squashfs_read_data(struct super_block *sb, u64 index, int length,
 	}
 
 	if (compressed) {
+		if (!msblk->stream)
+			goto read_failure;
 		length = squashfs_decompress(msblk, buf, b, offset, length,
 			output);
 		if (length < 0)
-- 
2.39.2

[PATCH 3/6] squashfs metadata 2: electric boogaloo

[Ahmad Fatoum]

This is a port of Linux commit cdbb65c4c7ead680ebe54f4f0d486e2847a500ea:

| Author:     Linus Torvalds <torvalds@linux-foundation.org>
| AuthorDate: Wed Aug 1 10:38:43 2018 -0700
|
| Anatoly continues to find issues with fuzzed squashfs images.
|
| This time, corrupt, missing, or undersized data for the page filling
| wasn't checked for, because the squashfs_{copy,read}_cache() functions
| did the squashfs_copy_data() call without checking the resulting data
| size.
|
| Which could result in the page cache pages being incompletely filled in,
| and no error indication to the user space reading garbage data.
|
| So make a helper function for the "fill in pages" case, because the
| exact same incomplete sequence existed in two places.
|
| [ I should have made a squashfs branch for these things, but I didn't
|   intend to start doing them in the first place.
|
|   My historical connection through cramfs is why I got into looking at
|   these issues at all, and every time I (continue to) think it's a
|   one-off.
|
|   Because _this_ time is always the last time. Right?   - Linus ]
|
| Reported-by: Anatoly Trosinenko <anatoly.trosinenko@gmail.com>
| Tested-by: Willy Tarreau <w@1wt.eu>
| Cc: Al Viro <viro@zeniv.linux.org.uk>
| Cc: Phillip Lougher <phillip@squashfs.org.uk>
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Reported-by: Richard Weinberger <richard@sigma-star.at>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 fs/squashfs/file.c       | 23 +++++++++++++++++++----
 fs/squashfs/file_cache.c |  5 +++--
 fs/squashfs/squashfs.h   |  2 +-
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index 1413ef7ecbd8..8316f0edb645 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -347,12 +347,23 @@ static int read_blocklist(struct inode *inode, int index, u64 *block)
 	return squashfs_block_size(size);
 }
 
+static int squashfs_fill_page(char *dest, struct squashfs_cache_entry *buffer,
+			      int offset, int avail)
+{
+	int copied;
+
+	copied = squashfs_copy_data(dest, buffer, offset, avail);
+	memset(dest + avail, 0, PAGE_CACHE_SIZE - avail);
+
+	return copied == avail ? 0 : -EILSEQ;
+}
+
 /* Copy data into page cache  */
-void squashfs_copy_cache(struct page *page, struct squashfs_cache_entry *buffer,
+int squashfs_copy_cache(struct page *page, struct squashfs_cache_entry *buffer,
 	int bytes, int offset)
 {
-	int i;
 	struct squashfs_page *sq_page = squashfs_page(page);
+	int ret, i;
 
 	/*
 	 * Loop copying datablock into pages.  As the datablock likely covers
@@ -366,10 +377,14 @@ void squashfs_copy_cache(struct page *page, struct squashfs_cache_entry *buffer,
 
 		TRACE("bytes %d, i %d, available_bytes %d\n", bytes, i, avail);
 
-		squashfs_copy_data(sq_page->buf[i], buffer, offset, avail);
-		memset(sq_page->buf[i] + avail, 0, PAGE_CACHE_SIZE - avail);
+		ret = squashfs_fill_page(sq_page->buf[i], buffer, offset, avail);
+		if (ret)
+			return ret;
+
 		sq_page->idx++;
 	}
+
+	return 0;
 }
 
 /* Read datablock stored packed inside a fragment (tail-end packed block) */
diff --git a/fs/squashfs/file_cache.c b/fs/squashfs/file_cache.c
index 9443e4558d1a..fbd829849d18 100644
--- a/fs/squashfs/file_cache.c
+++ b/fs/squashfs/file_cache.c
@@ -24,11 +24,12 @@ int squashfs_readpage_block(struct page *page, u64 block, int bsize)
 		block, bsize);
 	int res = buffer->error;
 
+	if (!res)
+		res = squashfs_copy_cache(page, buffer, buffer->length, 0);
+
 	if (res)
 		ERROR("Unable to read page, block %llx, size %x\n", block,
 			bsize);
-	else
-		squashfs_copy_cache(page, buffer, buffer->length, 0);
 
 	squashfs_cache_put(buffer);
 	return res;
diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h
index d22e83dc3c55..456021dcfd3e 100644
--- a/fs/squashfs/squashfs.h
+++ b/fs/squashfs/squashfs.h
@@ -93,7 +93,7 @@ extern int squashfs_frag_lookup(struct super_block *, unsigned int, u64 *);
 extern __le64 *squashfs_read_fragment_index_table(struct super_block *,
 				u64, u64, unsigned int);
 /* file.c */
-void squashfs_copy_cache(struct page *, struct squashfs_cache_entry *, int,
+int squashfs_copy_cache(struct page *, struct squashfs_cache_entry *, int,
 				int);
 extern int squashfs_readpage(struct file *file, struct page *page);
 
-- 
2.39.2

[PATCH 4/6] squashfs: more metadata hardening

[Ahmad Fatoum]

This is a port of Linux commit 71755ee5350b63fb1f283de8561cdb61b47f4d1d:

| Author:     Linus Torvalds <torvalds@linux-foundation.org>
| AuthorDate: Thu Aug 2 08:43:35 2018 -0700
|
| The squashfs fragment reading code doesn't actually verify that the
| fragment is inside the fragment table.  The end result _is_ verified to
| be inside the image when actually reading the fragment data, but before
| that is done, we may end up taking a page fault because the fragment
| table itself might not even exist.
|
| Another report from Anatoly and his endless squashfs image fuzzing.
|
| Reported-by: Анатолий Тросиненко <anatoly.trosinenko@gmail.com>
| Acked-by:: Phillip Lougher <phillip.lougher@gmail.com>,
| Cc: Willy Tarreau <w@1wt.eu>
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Reported-by: Richard Weinberger <richard@sigma-star.at>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 fs/squashfs/fragment.c       | 13 +++++++++----
 fs/squashfs/squashfs_fs_sb.h |  1 +
 fs/squashfs/super.c          |  5 +++--
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/fs/squashfs/fragment.c b/fs/squashfs/fragment.c
index 343444000e02..9d60741ce5f0 100644
--- a/fs/squashfs/fragment.c
+++ b/fs/squashfs/fragment.c
@@ -44,11 +44,16 @@ int squashfs_frag_lookup(struct super_block *sb, unsigned int fragment,
 				u64 *fragment_block)
 {
 	struct squashfs_sb_info *msblk = sb->s_fs_info;
-	int block = SQUASHFS_FRAGMENT_INDEX(fragment);
-	int offset = SQUASHFS_FRAGMENT_INDEX_OFFSET(fragment);
-	u64 start_block = le64_to_cpu(msblk->fragment_index[block]);
+	int block, offset, size;
 	struct squashfs_fragment_entry fragment_entry;
-	int size;
+	u64 start_block;
+
+	if (fragment >= msblk->fragments)
+		return -EIO;
+	block = SQUASHFS_FRAGMENT_INDEX(fragment);
+	offset = SQUASHFS_FRAGMENT_INDEX_OFFSET(fragment);
+
+	start_block = le64_to_cpu(msblk->fragment_index[block]);
 
 	size = squashfs_read_metadata(sb, &fragment_entry, &start_block,
 					&offset, sizeof(fragment_entry));
diff --git a/fs/squashfs/squashfs_fs_sb.h b/fs/squashfs/squashfs_fs_sb.h
index c6fc37d48f25..8ac5a1f3b6e3 100644
--- a/fs/squashfs/squashfs_fs_sb.h
+++ b/fs/squashfs/squashfs_fs_sb.h
@@ -75,6 +75,7 @@ struct squashfs_sb_info {
 	unsigned short				block_log;
 	long long				bytes_used;
 	unsigned int				inodes;
+	unsigned int				fragments;
 	int					xattr_ids;
 	struct cdev				*cdev;
 	struct device				*dev;
diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
index 2e34c0e540df..f3dac6de1527 100644
--- a/fs/squashfs/super.c
+++ b/fs/squashfs/super.c
@@ -189,6 +189,7 @@ static int squashfs_fill_super(struct super_block *sb, void *data, int silent)
 	msblk->inode_table = le64_to_cpu(sblk->inode_table_start);
 	msblk->directory_table = le64_to_cpu(sblk->directory_table_start);
 	msblk->inodes = le32_to_cpu(sblk->inodes);
+	msblk->fragments = le32_to_cpu(sblk->fragments);
 	flags = le16_to_cpu(sblk->flags);
 
 	TRACE("Found valid superblock on %pg\n", sb->s_bdev);
@@ -199,7 +200,7 @@ static int squashfs_fill_super(struct super_block *sb, void *data, int silent)
 	TRACE("Filesystem size %lld bytes\n", msblk->bytes_used);
 	TRACE("Block size %d\n", msblk->block_size);
 	TRACE("Number of inodes %d\n", msblk->inodes);
-	TRACE("Number of fragments %d\n", le32_to_cpu(sblk->fragments));
+	TRACE("Number of fragments %d\n", msblk->fragments);
 	TRACE("Number of ids %d\n", le16_to_cpu(sblk->no_ids));
 	TRACE("sblk->inode_table_start %llx\n", msblk->inode_table);
 	TRACE("sblk->directory_table_start %llx\n", msblk->directory_table);
@@ -253,7 +254,7 @@ static int squashfs_fill_super(struct super_block *sb, void *data, int silent)
 		goto handle_fragments;
 
 handle_fragments:
-	fragments = le32_to_cpu(sblk->fragments);
+	fragments = msblk->fragments;
 	if (fragments == 0)
 		goto check_directory_table;
 	msblk->fragment_cache = squashfs_cache_init("fragment",
-- 
2.39.2

[PATCH 5/6] Squashfs: Compute expected length from inode size rather than block length

[Ahmad Fatoum]

This is a port of Linux commit a3f94cb99a854fa381fe7fadd97c4f61633717a5:

| Author:     Phillip Lougher <phillip@squashfs.org.uk>
| AuthorDate: Thu Aug 2 16:45:15 2018 +0100
|
| Previously in squashfs_readpage() when copying data into the page
| cache, it used the length of the datablock read from the filesystem
| (after decompression).  However, if the filesystem has been corrupted
| this data block may be short, which will leave pages unfilled.
|
| The fix for this is to compute the expected number of bytes to copy
| from the inode size, and use this to detect if the block is short.
|
| Signed-off-by: Phillip Lougher <phillip@squashfs.org.uk>
| Tested-by: Willy Tarreau <w@1wt.eu>
| Cc: Анатолий Тросиненко <anatoly.trosinenko@gmail.com>
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Reported-by: Richard Weinberger <richard@sigma-star.at>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 fs/squashfs/file.c       | 26 ++++++++++----------------
 fs/squashfs/file_cache.c |  4 ++--
 fs/squashfs/squashfs.h   |  2 +-
 3 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/fs/squashfs/file.c b/fs/squashfs/file.c
index 8316f0edb645..16914415299a 100644
--- a/fs/squashfs/file.c
+++ b/fs/squashfs/file.c
@@ -388,10 +388,9 @@ int squashfs_copy_cache(struct page *page, struct squashfs_cache_entry *buffer,
 }
 
 /* Read datablock stored packed inside a fragment (tail-end packed block) */
-static int squashfs_readpage_fragment(struct page *page)
+static int squashfs_readpage_fragment(struct page *page, int expected)
 {
 	struct inode *inode = page->inode;
-	struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
 	struct squashfs_cache_entry *buffer = squashfs_get_fragment(inode->i_sb,
 		squashfs_i(inode)->fragment_block,
 		squashfs_i(inode)->fragment_size);
@@ -404,24 +403,16 @@ static int squashfs_readpage_fragment(struct page *page)
 			squashfs_i(inode)->fragment_block,
 			squashfs_i(inode)->fragment_size);
 	else
-		squashfs_copy_cache(page, buffer, i_size_read(inode) &
-			(msblk->block_size - 1),
+		squashfs_copy_cache(page, buffer, expected,
 			squashfs_i(inode)->fragment_offset);
 
 	squashfs_cache_put(buffer);
 	return res;
 }
 
-static int squashfs_readpage_sparse(struct page *page, int index, int file_end)
+static int squashfs_readpage_sparse(struct page *page, int expected)
 {
-	struct inode *inode = page->inode;
-	struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
-	int bytes = index == file_end ?
-			(i_size_read(inode) & (msblk->block_size - 1)) :
-			 msblk->block_size;
-
-	squashfs_copy_cache(page, NULL, bytes, 0);
-
+	squashfs_copy_cache(page, NULL, expected, 0);
 	return 0;
 }
 
@@ -431,6 +422,9 @@ int squashfs_readpage(struct file *file, struct page *page)
 	struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info;
 	int index = page->index >> (msblk->block_log - PAGE_CACHE_SHIFT);
 	int file_end = i_size_read(inode) >> msblk->block_log;
+	int expected = index == file_end ?
+			(i_size_read(inode) & (msblk->block_size - 1)) :
+			 msblk->block_size;
 	int res;
 
 	TRACE("Entered squashfs_readpage, page index %lx, start block %llx\n",
@@ -448,11 +442,11 @@ int squashfs_readpage(struct file *file, struct page *page)
 			goto out;
 
 		if (bsize == 0)
-			res = squashfs_readpage_sparse(page, index, file_end);
+			res = squashfs_readpage_sparse(page, expected);
 		else
-			res = squashfs_readpage_block(page, block, bsize);
+			res = squashfs_readpage_block(page, block, bsize, expected);
 	} else
-		res = squashfs_readpage_fragment(page);
+		res = squashfs_readpage_fragment(page, expected);
 
 	if (!res)
 		return 0;
diff --git a/fs/squashfs/file_cache.c b/fs/squashfs/file_cache.c
index fbd829849d18..0b64be0994c7 100644
--- a/fs/squashfs/file_cache.c
+++ b/fs/squashfs/file_cache.c
@@ -17,7 +17,7 @@
 #include "squashfs.h"
 
 /* Read separately compressed datablock and memcopy into page cache */
-int squashfs_readpage_block(struct page *page, u64 block, int bsize)
+int squashfs_readpage_block(struct page *page, u64 block, int bsize, int expected)
 {
 	struct inode *i = page->inode;
 	struct squashfs_cache_entry *buffer = squashfs_get_datablock(i->i_sb,
@@ -25,7 +25,7 @@ int squashfs_readpage_block(struct page *page, u64 block, int bsize)
 	int res = buffer->error;
 
 	if (!res)
-		res = squashfs_copy_cache(page, buffer, buffer->length, 0);
+		res = squashfs_copy_cache(page, buffer, expected, 0);
 
 	if (res)
 		ERROR("Unable to read page, block %llx, size %x\n", block,
diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h
index 456021dcfd3e..1f2fee59d688 100644
--- a/fs/squashfs/squashfs.h
+++ b/fs/squashfs/squashfs.h
@@ -98,7 +98,7 @@ int squashfs_copy_cache(struct page *, struct squashfs_cache_entry *, int,
 extern int squashfs_readpage(struct file *file, struct page *page);
 
 /* file_xxx.c */
-extern int squashfs_readpage_block(struct page *, u64, int);
+extern int squashfs_readpage_block(struct page *, u64, int, int);
 
 /* id.c */
 extern int squashfs_get_id(struct super_block *, unsigned int, unsigned int *);
-- 
2.39.2

[PATCH 6/6] squashfs: refuse mount of squashfs images with non-128K block size

[Ahmad Fatoum]

Mounting a block of any size, but 128K, leads to erroneous data in blocks
beyond the first. It would be nice to support the full-range of block
sizes supported by Linux, but for now, let's just enforce that the
default block size of 128K is used and refuse the mount with an error
message if the image has been generated with a non-default block size.

Reported-by: Michael Olbrich <mol@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 fs/squashfs/Kconfig | 5 +----
 fs/squashfs/super.c | 7 +++++++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/squashfs/Kconfig b/fs/squashfs/Kconfig
index af187a2a8ac7..c57ef0044cdd 100644
--- a/fs/squashfs/Kconfig
+++ b/fs/squashfs/Kconfig
@@ -9,10 +9,7 @@ menuconfig FS_SQUASHFS
 	  filesystem for Linux.  It uses zlib, lzo or xz compression to
 	  compress both files, inodes and directories.  Inodes in the system
 	  are very small and all blocks are packed to minimise data overhead.
-	  Block sizes greater than 4K are supported up to a maximum of 1 Mbytes
-	  (default block size 128K).  SquashFS 4.0 supports 64 bit filesystems
-	  and files (larger than 4GB), full uid/gid information, hard links and
-	  timestamps.
+	  Only the default block sizes of 128K is supported.
 
 	  Squashfs is intended for general read-only filesystem use, for
 	  archival use (i.e. in cases where a .tar.gz file may be used), and in
diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
index f3dac6de1527..7695c9e0fd25 100644
--- a/fs/squashfs/super.c
+++ b/fs/squashfs/super.c
@@ -31,6 +31,7 @@
 #include <linux/pagemap.h>
 #include <linux/magic.h>
 #include <linux/bitops.h>
+#include <linux/sizes.h>
 
 #include "page_actor.h"
 #include "squashfs_fs.h"
@@ -172,6 +173,12 @@ static int squashfs_fill_super(struct super_block *sb, void *data, int silent)
 		goto failed_mount;
 	}
 
+	if (msblk->block_size != SZ_128K) {
+		ERROR("filesystem block size (%d) != 128K.  This is "
+			"currently not supported!\n", msblk->block_size);
+		goto failed_mount;
+	}
+
 	/* Check block log for sanity */
 	msblk->block_log = le16_to_cpu(sblk->block_log);
 	if (msblk->block_log > SQUASHFS_FILE_MAX_LOG)
-- 
2.39.2

Re: [PATCH 0/6] squashfs: harden against crafted metadata

[Sascha Hauer]

On Wed, 17 Jul 2024 08:33:22 +0200, Ahmad Fatoum wrote:
> Richard reports[1] that barebox is susceptible to a number of memory safety
> issues when parsing crafted squashfs files, which have been fixed in the
> upstream Linux implementation in the meantime.
> 
> Import the mentioned commits from Linux to fix this:
> 
>   01cfb7937a9af ("squashfs: be more careful about metadata corruption")
>   d512584780d3e ("squashfs: more metadata hardening")
>   cdbb65c4c7ead ("squashfs metadata 2: electric boogaloo")
>   71755ee5350b6 ("squashfs: more metadata hardening")
>   a3f94cb99a854 ("Squashfs: Compute expected length from inode size rather than block length")
> 
> [...]

Applied, thanks!

[1/6] squashfs: be more careful about metadata corruption
      https://git.pengutronix.de/cgit/barebox/commit/?id=526642ffecd0 (link may not be stable)
[2/6] squashfs: more metadata hardening
      https://git.pengutronix.de/cgit/barebox/commit/?id=2b601e956dc6 (link may not be stable)
[3/6] squashfs metadata 2: electric boogaloo
      https://git.pengutronix.de/cgit/barebox/commit/?id=2bd8da6174f7 (link may not be stable)
[4/6] squashfs: more metadata hardening
      https://git.pengutronix.de/cgit/barebox/commit/?id=2b601e956dc6 (link may not be stable)
[5/6] Squashfs: Compute expected length from inode size rather than block length
      https://git.pengutronix.de/cgit/barebox/commit/?id=0f180583cb2d (link may not be stable)
[6/6] squashfs: refuse mount of squashfs images with non-128K block size
      https://git.pengutronix.de/cgit/barebox/commit/?id=56d5844b1c83 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>