squashfs: be more careful about metadata corruption

torvalds · torvalds · commit 01cfb7937a9a · 2018-07-29T12:44:46.000-07:00
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 &lt;anatoly.trosinenko@gmail.com&gt;
Cc: Al Viro &lt;viro@zeniv.linux.org.uk&gt;
Cc: Phillip Lougher &lt;phillip@squashfs.org.uk&gt;
Cc: stable@kernel.org
Signed-off-by: Linus Torvalds &lt;torvalds@linux-foundation.org&gt;
diff --git a/fs/squashfs/cache.c b/fs/squashfs/cache.c
@@ -350,6 +350,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
@@ -194,7 +194,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;
@@ -367,7 +371,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
@@ -61,9 +61,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
@@ -129,6 +129,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

Original file line number	Diff line number	Diff line change
`@@ -194,7 +194,11 @@ static long long read_indexes(struct super_block *sb, int n,`
`194`	`194`	`}`
`195`	`195`
`196`	`196`	`for (i = 0; i < blocks; i++) {`
`197`		`- int size = le32_to_cpu(blist[i]);`
	`197`	`+ int size = squashfs_block_size(blist[i]);`
	`198`	`+ if (size < 0) {`
	`199`	`+ err = size;`
	`200`	`+ goto failure;`
	`201`	`+ }`
`198`	`202`	`block += SQUASHFS_COMPRESSED_SIZE_BLOCK(size);`
`199`	`203`	`}`
`200`	`204`	`n -= blocks;`
`@@ -367,7 +371,7 @@ static int read_blocklist(struct inode inode, int index, u64 block)`
`367`	`371`	`sizeof(size));`
`368`	`372`	`if (res < 0)`
`369`	`373`	`return res;`
`370`		`- return le32_to_cpu(size);`
	`374`	`+ return squashfs_block_size(size);`
`371`	`375`	`}`
`372`	`376`
`373`	`377`	`/* Copy data into page cache */`
Original file line number	Diff line number	Diff line change
`@@ -61,9 +61,7 @@ int squashfs_frag_lookup(struct super_block *sb, unsigned int fragment,`
`61`	`61`	`return size;`
`62`	`62`
`63`	`63`	`*fragment_block = le64_to_cpu(fragment_entry.start_block);`
`64`		`- size = le32_to_cpu(fragment_entry.size);`
`65`		`-`
`66`		`- return size;`
	`64`	`+ return squashfs_block_size(fragment_entry.size);`
`67`	`65`	`}`
`68`	`66`
`69`	`67`