From 28d7b5684ba98e163ba37779fd09de01fac5261d Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 17 Jul 2013 15:20:25 +0300 Subject: Squashfs: sanity check information from disk We read the size of the name from the disk, but a larger name than expected would cause memory corruption. Signed-off-by: Dan Carpenter Signed-off-by: Phillip Lougher --- fs/squashfs/namei.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/squashfs/namei.c b/fs/squashfs/namei.c index 7834a517f7f4..f866d42a8b6f 100644 --- a/fs/squashfs/namei.c +++ b/fs/squashfs/namei.c @@ -79,7 +79,8 @@ static int get_dir_index_using_name(struct super_block *sb, int len) { struct squashfs_sb_info *msblk = sb->s_fs_info; - int i, size, length = 0, err; + int i, length = 0, err; + unsigned int size; struct squashfs_dir_index *index; char *str; @@ -103,6 +104,10 @@ static int get_dir_index_using_name(struct super_block *sb, size = le32_to_cpu(index->size) + 1; + if (size > SQUASHFS_NAME_LEN) { + err = -EINVAL; + break; + } err = squashfs_read_metadata(sb, index->name, &index_start, &index_offset, size); -- cgit v1.2.3 From e0125262a2f5cefbfb3804117f8ab16e9ba13e29 Mon Sep 17 00:00:00 2001 From: Manish Sharma Date: Wed, 4 Sep 2013 22:31:23 +0530 Subject: Squashfs: Optimized uncompressed buffer loop Merged the two for loops. We might get a little gain by overlapping wait_on_bh and the memcpy operations. Signed-off-by: Manish Sharma Signed-off-by: Phillip Lougher --- fs/squashfs/block.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) (limited to 'fs') diff --git a/fs/squashfs/block.c b/fs/squashfs/block.c index fb50652e4e11..41d108ecc9be 100644 --- a/fs/squashfs/block.c +++ b/fs/squashfs/block.c @@ -167,17 +167,14 @@ int squashfs_read_data(struct super_block *sb, void **buffer, u64 index, /* * Block is uncompressed. */ - int i, in, pg_offset = 0; - - for (i = 0; i < b; i++) { - wait_on_buffer(bh[i]); - if (!buffer_uptodate(bh[i])) - goto block_release; - } + int in, pg_offset = 0; for (bytes = length; k < b; k++) { in = min(bytes, msblk->devblksize - offset); bytes -= in; + wait_on_buffer(bh[k]); + if (!buffer_uptodate(bh[k])) + goto block_release; while (in) { if (pg_offset == PAGE_CACHE_SIZE) { page++; -- cgit v1.2.3 From 9dbc41d5d371cb10099c8182552f3915920e69b6 Mon Sep 17 00:00:00 2001 From: Phillip Lougher Date: Tue, 3 Sep 2013 04:02:53 +0100 Subject: Squashfs: fix corruption check in get_dir_index_using_name() Patch "Squashfs: sanity check information from disk" from Dan Carpenter adds a missing check for corruption in the "size" field while reading the directory index from disk. It, however, sets err to -EINVAL, this value is not used later, and so setting it is completely redundant. So remove it. Errors in reading the index are deliberately non-fatal. If we get an error in reading the index we just return the part of the index we have managed to read - the index isn't essential, just quicker. Signed-off-by: Phillip Lougher --- fs/squashfs/namei.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'fs') diff --git a/fs/squashfs/namei.c b/fs/squashfs/namei.c index f866d42a8b6f..342a5aa5a0e4 100644 --- a/fs/squashfs/namei.c +++ b/fs/squashfs/namei.c @@ -104,10 +104,8 @@ static int get_dir_index_using_name(struct super_block *sb, size = le32_to_cpu(index->size) + 1; - if (size > SQUASHFS_NAME_LEN) { - err = -EINVAL; + if (size > SQUASHFS_NAME_LEN) break; - } err = squashfs_read_metadata(sb, index->name, &index_start, &index_offset, size); -- cgit v1.2.3 From 52e9ce1c0f2060661e147ffaf701a17f2fc0a153 Mon Sep 17 00:00:00 2001 From: Phillip Lougher Date: Tue, 3 Sep 2013 04:21:52 +0100 Subject: Squashfs: fix corruption checks in squashfs_lookup() The dir_count and size fields when read from disk are sanity checked for correctness. However, the sanity checks only check the values are not greater than expected. As dir_count and size were incorrectly defined as signed ints, this can lead to corrupted values appearing as negative which are not trapped. Signed-off-by: Phillip Lougher --- fs/squashfs/namei.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/squashfs/namei.c b/fs/squashfs/namei.c index 342a5aa5a0e4..67cad77fefb4 100644 --- a/fs/squashfs/namei.c +++ b/fs/squashfs/namei.c @@ -147,7 +147,8 @@ static struct dentry *squashfs_lookup(struct inode *dir, struct dentry *dentry, struct squashfs_dir_entry *dire; u64 block = squashfs_i(dir)->start + msblk->directory_table; int offset = squashfs_i(dir)->offset; - int err, length, dir_count, size; + int err, length; + unsigned int dir_count, size; TRACE("Entered squashfs_lookup [%llx:%x]\n", block, offset); -- cgit v1.2.3 From 68e7f412370ecfeb1bd667d0d174fad34517516e Mon Sep 17 00:00:00 2001 From: Phillip Lougher Date: Tue, 3 Sep 2013 04:38:43 +0100 Subject: Squashfs: fix corruption checks in squashfs_readdir() The dir_count and size fields when read from disk are sanity checked for correctness. However, the sanity checks only check the values are not greater than expected. As dir_count and size were incorrectly defined as signed ints, this can lead to corrupted values appearing as negative which are not trapped. Signed-off-by: Phillip Lougher --- fs/squashfs/dir.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'fs') diff --git a/fs/squashfs/dir.c b/fs/squashfs/dir.c index f7f527bf8c10..119208422260 100644 --- a/fs/squashfs/dir.c +++ b/fs/squashfs/dir.c @@ -105,9 +105,8 @@ static int squashfs_readdir(struct file *file, struct dir_context *ctx) struct inode *inode = file_inode(file); struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info; u64 block = squashfs_i(inode)->start + msblk->directory_table; - int offset = squashfs_i(inode)->offset, length, dir_count, size, - type, err; - unsigned int inode_number; + int offset = squashfs_i(inode)->offset, length, type, err; + unsigned int inode_number, dir_count, size; struct squashfs_dir_header dirh; struct squashfs_dir_entry *dire; -- cgit v1.2.3 From f960cae5357d8e52b8af91e8b1621cae565dffb3 Mon Sep 17 00:00:00 2001 From: Phillip Lougher Date: Tue, 3 Sep 2013 04:52:52 +0100 Subject: Squashfs: add corruption check in get_dir_index_using_offset() We read the size (of the name) field from disk. This value should be sanity checked for correctness to avoid blindly reading huge amounts of unnecessary data from disk on corruption. Note, here we're not actually reading the name into a buffer, but skipping it, and so corruption doesn't cause buffer overflow, merely lots of unnecessary amounts of data to be read. Signed-off-by: Phillip Lougher --- fs/squashfs/dir.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/squashfs/dir.c b/fs/squashfs/dir.c index 119208422260..bd7155b198a9 100644 --- a/fs/squashfs/dir.c +++ b/fs/squashfs/dir.c @@ -54,6 +54,7 @@ static int get_dir_index_using_offset(struct super_block *sb, { struct squashfs_sb_info *msblk = sb->s_fs_info; int err, i, index, length = 0; + unsigned int size; struct squashfs_dir_index dir_index; TRACE("Entered get_dir_index_using_offset, i_count %d, f_pos %lld\n", @@ -81,8 +82,14 @@ static int get_dir_index_using_offset(struct super_block *sb, */ break; + size = le32_to_cpu(dir_index.size) + 1; + + /* size should never be larger than SQUASHFS_NAME_LEN */ + if (size > SQUASHFS_NAME_LEN) + break; + err = squashfs_read_metadata(sb, NULL, &index_start, - &index_offset, le32_to_cpu(dir_index.size) + 1); + &index_offset, size); if (err < 0) break; -- cgit v1.2.3 From 9e012423869e1efbae3762b87ceab509027231c9 Mon Sep 17 00:00:00 2001 From: Phillip Lougher Date: Wed, 4 Sep 2013 02:58:12 +0100 Subject: Squashfs: add corruption check for type in squashfs_readdir() We read the type field from disk. This value should be sanity checked for correctness to avoid an out of bounds access when reading the squashfs_filetype_table array. Signed-off-by: Phillip Lougher --- fs/squashfs/dir.c | 7 +++++-- fs/squashfs/squashfs_fs.h | 5 ++++- 2 files changed, 9 insertions(+), 3 deletions(-) (limited to 'fs') diff --git a/fs/squashfs/dir.c b/fs/squashfs/dir.c index bd7155b198a9..d8c2d747be28 100644 --- a/fs/squashfs/dir.c +++ b/fs/squashfs/dir.c @@ -112,8 +112,8 @@ static int squashfs_readdir(struct file *file, struct dir_context *ctx) struct inode *inode = file_inode(file); struct squashfs_sb_info *msblk = inode->i_sb->s_fs_info; u64 block = squashfs_i(inode)->start + msblk->directory_table; - int offset = squashfs_i(inode)->offset, length, type, err; - unsigned int inode_number, dir_count, size; + int offset = squashfs_i(inode)->offset, length, err; + unsigned int inode_number, dir_count, size, type; struct squashfs_dir_header dirh; struct squashfs_dir_entry *dire; @@ -206,6 +206,9 @@ static int squashfs_readdir(struct file *file, struct dir_context *ctx) ((short) le16_to_cpu(dire->inode_number)); type = le16_to_cpu(dire->type); + if (type > SQUASHFS_MAX_DIR_TYPE) + goto failed_read; + if (!dir_emit(ctx, dire->name, size, inode_number, squashfs_filetype_table[type])) diff --git a/fs/squashfs/squashfs_fs.h b/fs/squashfs/squashfs_fs.h index 9e2349d07cb1..4b2beda49498 100644 --- a/fs/squashfs/squashfs_fs.h +++ b/fs/squashfs/squashfs_fs.h @@ -87,7 +87,7 @@ #define SQUASHFS_COMP_OPTS(flags) SQUASHFS_BIT(flags, \ SQUASHFS_COMP_OPT) -/* Max number of types and file types */ +/* Inode types including extended types */ #define SQUASHFS_DIR_TYPE 1 #define SQUASHFS_REG_TYPE 2 #define SQUASHFS_SYMLINK_TYPE 3 @@ -103,6 +103,9 @@ #define SQUASHFS_LFIFO_TYPE 13 #define SQUASHFS_LSOCKET_TYPE 14 +/* Max type value stored in directory entry */ +#define SQUASHFS_MAX_DIR_TYPE 7 + /* Xattr types */ #define SQUASHFS_XATTR_USER 0 #define SQUASHFS_XATTR_TRUSTED 1 -- cgit v1.2.3