All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 6/7] Squashfs: add sanity checks to fragment reading at mount time
@ 2011-05-25  3:21 Phillip Lougher
  0 siblings, 0 replies; only message in thread
From: Phillip Lougher @ 2011-05-25  3:21 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-kernel

Fsfuzzer generates corrupted filesystems which throw a warn_on in
kmalloc.  One of these is due to a corrupted superblock fragments field.
Fix this by checking that the number of bytes to be read (and allocated)
does not extend into the next filesystem structure.

Also add a couple of other sanity checks of the mount-time fragment table
structures.

Signed-off-by: Phillip Lougher <phillip@lougher.demon.co.uk>
---
 fs/squashfs/fragment.c |   24 ++++++++++++++++++++++--
 fs/squashfs/squashfs.h |    2 +-
 fs/squashfs/super.c    |    3 ++-
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/fs/squashfs/fragment.c b/fs/squashfs/fragment.c
index 567093d..8f84049 100644
--- a/fs/squashfs/fragment.c
+++ b/fs/squashfs/fragment.c
@@ -71,9 +71,29 @@ int squashfs_frag_lookup(struct super_block *sb, unsigned int fragment,
  * Read the uncompressed fragment lookup table indexes off disk into memory
  */
 __le64 *squashfs_read_fragment_index_table(struct super_block *sb,
-	u64 fragment_table_start, unsigned int fragments)
+	u64 fragment_table_start, u64 next_table, unsigned int fragments)
 {
 	unsigned int length = SQUASHFS_FRAGMENT_INDEX_BYTES(fragments);
+	__le64 *table;
 
-	return squashfs_read_table(sb, fragment_table_start, length);
+	/*
+	 * Sanity check, length bytes should not extend into the next table -
+	 * this check also traps instances where fragment_table_start is
+	 * incorrectly larger than the next table start
+	 */
+	if (fragment_table_start + length > next_table)
+		return ERR_PTR(-EINVAL);
+
+	table = squashfs_read_table(sb, fragment_table_start, length);
+
+	/*
+	 * table[0] points to the first fragment table metadata block, this
+	 * should be less than fragment_table_start
+	 */
+	if (table[0] >= fragment_table_start) {
+		kfree(table);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return table;
 }
diff --git a/fs/squashfs/squashfs.h b/fs/squashfs/squashfs.h
index 3b705f1..647b81b 100644
--- a/fs/squashfs/squashfs.h
+++ b/fs/squashfs/squashfs.h
@@ -57,7 +57,7 @@ extern __le64 *squashfs_read_inode_lookup_table(struct super_block *, u64, u64,
 /* fragment.c */
 extern int squashfs_frag_lookup(struct super_block *, unsigned int, u64 *);
 extern __le64 *squashfs_read_fragment_index_table(struct super_block *,
-				u64, unsigned int);
+				u64, u64, unsigned int);
 
 /* id.c */
 extern int squashfs_get_id(struct super_block *, unsigned int, unsigned int *);
diff --git a/fs/squashfs/super.c b/fs/squashfs/super.c
index 80a7119..efa8118 100644
--- a/fs/squashfs/super.c
+++ b/fs/squashfs/super.c
@@ -261,6 +261,7 @@ allocate_id_index_table:
 		msblk->inode_lookup_table = NULL;
 		goto failed_mount;
 	}
+	next_table = msblk->inode_lookup_table[0];
 
 	sb->s_export_op = &squashfs_export_ops;
 
@@ -278,7 +279,7 @@ handle_fragments:
 
 	/* Allocate and read fragment index table */
 	msblk->fragment_index = squashfs_read_fragment_index_table(sb,
-		le64_to_cpu(sblk->fragment_table_start), fragments);
+		le64_to_cpu(sblk->fragment_table_start), next_table, fragments);
 	if (IS_ERR(msblk->fragment_index)) {
 		ERROR("unable to read fragment index table\n");
 		err = PTR_ERR(msblk->fragment_index);
-- 
1.7.1


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2011-05-25  3:21 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-25  3:21 [PATCH 6/7] Squashfs: add sanity checks to fragment reading at mount time Phillip Lougher

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.