More mount-time checks
cover More mount-time checks
This series adds more superblock checks that turned out not to be enough after "btrfs: add more superblock checks". Would be good to get them into 3.18 as I'd like to submit them to stable kernels as well. This is not easily possible right now as it depends on a patch that's not merged yet (mentioned below).
Based on current master plus patch
"btrfs: fix typos in btrfs_check_super_valid"
(https://patchwork.kernel.org/patch/5206401/)
David Sterba (4):
btrfs: more superblock checks, lower bounds on devices and
sectorsize/nodesize
btrfs: add checks for sys_chunk_array sizes
btrfs: cleanup, rename a few variables in btrfs_read_sys_array
btrfs: add more checks to btrfs_read_sys_array
fs/btrfs/disk-io.c | 38 +++++++++++++++++++++++++++++++++++++ fs/btrfs/volumes.c | 55 +++++++++++++++++++++++++++++++++++++----------------- 2 files changed, 76 insertions(+), 17 deletions(-)
Signature · 1 line
1.8.4.5
PATCH 1/4 btrfs: more superblock checks, lower bounds on devices and sectorsize/nodesize
I received a few crafted images from Jiri, all got through the recently added superblock checks. The lower bounds checks for num_devices and sector/node -sizes were missing and caused a crash during mount.
Tools for symbolic code execution were used to prepare the images contents.
Reported-by Jiri Slaby <[email protected]>
Signed-off-by David Sterba <[email protected]>
--- fs/btrfs/disk-io.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
fs/btrfs/disk-io.c +19 -0
@@ -3847,6 +3847,21 @@ static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info,printk (KERN_WARNING "BTRFS: log_root block unaligned: %llu\n" ,btrfs_super_log_root (sb)); +/* +* Check the lower bound, the alignment and other constraints are +* checked later. +*/ +if (btrfs_super_nodesize (sb) <4096 ) { +printk (KERN_ERR "BTRFS: nodesize too small: %llu < 4096\n" , +btrfs_super_nodesize (sb)); + ret = -EINVAL ; + } +if (btrfs_super_sectorsize (sb) <4096 ) { +printk (KERN_ERR "BTRFS: sectorsize too small: %llu < 4096\n" , +btrfs_super_sectorsize (sb)); + ret = -EINVAL ; + } +if (memcmp (fs_info->fsid, sb->dev_item.fsid,BTRFS_UUID_SIZE ) !=0 ) {printk (KERN_ERR "BTRFS: dev_item UUID does not match fsid: %pU != %pU\n" , fs_info->fsid, sb->dev_item.fsid);
@@ -3860,6 +3875,10 @@ static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info,if (btrfs_super_num_devices (sb) > (1UL <<31 ))printk (KERN_WARNING "BTRFS: suspicious number of devices: %llu\n" ,btrfs_super_num_devices (sb)); +if (btrfs_super_num_devices (sb) ==0 ) { +printk (KERN_ERR "BTRFS: number of devices is 0\n" ); + ret = -EINVAL ; + }if (btrfs_super_bytenr (sb) !=BTRFS_SUPER_INFO_OFFSET ) { printk(KERN_ERR "BTRFS: super offset mismatch %llu != %u\n",
Signature · 1 line
1.8.4.5
PATCH 2/4 btrfs: add checks for sys_chunk_array sizes
In reply to David SterbaVerify that possible minimum and maximum size is set, validity of contents is checked in btrfs_read_sys_array.
Signed-off-by David Sterba <[email protected]>
--- fs/btrfs/disk-io.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
fs/btrfs/disk-io.c +19 -0
@@ -3887,6 +3887,25 @@ static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info, }/* +* Obvious sys_chunk_array corruptions, it must hold at least one key +* and one chunk +*/ +if (btrfs_super_sys_array_size (sb) >BTRFS_SYSTEM_CHUNK_ARRAY_SIZE ) { +printk (KERN_ERR "BTRFS: system chunk array too big %llu > %u\n" , +btrfs_super_sys_array_size (sb), +BTRFS_SYSTEM_CHUNK_ARRAY_SIZE ); + ret = -EINVAL ; + } +if (btrfs_super_sys_array_size (sb) <sizeof (struct btrfs_disk_key ) + +sizeof (struct btrfs_chunk )) { +printk (KERN_ERR "BTRFS: system chunk array too small %llu < %u\n" , +btrfs_super_sys_array_size (sb), +sizeof (struct btrfs_disk_key ) + +sizeof (struct btrfs_chunk )); + ret = -EINVAL ; + } + +/* * The generation is a global counter, we'll trust it more than the others * but it's still possible that it's the one that's wrong. */
Signature · 1 line
1.8.4.5
PATCH 3/4 btrfs: cleanup, rename a few variables in btrfs_read_sys_array
In reply to David SterbaThere's a pointer to buffer, integer offset and offset passed as pointer, try to find matching names for them.
Signed-off-by David Sterba <[email protected]>
--- fs/btrfs/volumes.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
fs/btrfs/volumes.c +16 -15
@@ -6212,13 +6212,13 @@ int btrfs_read_sys_array(struct btrfs_root *root)struct extent_buffer *sb;struct btrfs_disk_key *disk_key;struct btrfs_chunk *chunk; -u8 *ptr; -unsigned long sb_ptr; +u8 *array_ptr; +unsigned long sb_array_offset;int ret =0 ;u32 num_stripes;u32 array_size;u32 len =0 ; -u32 cur; +u32 cur_offset;struct btrfs_key key; sb = btrfs_find_create_tree_block(root, BTRFS_SUPER_INFO_OFFSET,
@@ -6245,20 +6245,21 @@ int btrfs_read_sys_array(struct btrfs_root *root)write_extent_buffer (sb, super_copy,0 ,BTRFS_SUPER_INFO_SIZE ); array_size =btrfs_super_sys_array_size (super_copy); - ptr = super_copy->sys_chunk_array; - sb_ptr = offsetof(struct btrfs_super_block , sys_chunk_array); - cur =0 ; + array_ptr = super_copy->sys_chunk_array; + sb_array_offset = offsetof(struct btrfs_super_block , sys_chunk_array); + cur_offset =0 ; -while (cur < array_size) { - disk_key = (struct btrfs_disk_key *)ptr; +while (cur_offset < array_size) { + disk_key = (struct btrfs_disk_key *)array_ptr;btrfs_disk_key_to_cpu (&key, disk_key); - len =sizeof (*disk_key); ptr += len; - sb_ptr += len; - cur += len; + len =sizeof (*disk_key); + array_ptr += len; + sb_array_offset += len; + cur_offset += len;if (key.type ==BTRFS_CHUNK_ITEM_KEY ) { - chunk = (struct btrfs_chunk *)sb_ptr; + chunk = (struct btrfs_chunk *)sb_array_offset; ret =read_one_chunk (root, &key, sb, chunk);if (ret)break ;
@@ -6268,9 +6269,9 @@ int btrfs_read_sys_array(struct btrfs_root *root) ret = -EIO ;break ; } - ptr += len; - sb_ptr += len; - cur += len; + array_ptr += len; + sb_array_offset += len; + cur_offset += len; }free_extent_buffer (sb);return ret;
Signature · 1 line
1.8.4.5
PATCH 4/4 btrfs: add more checks to btrfs_read_sys_array
In reply to David SterbaVerify that the sys_array has enough bytes to read the next item.
Signed-off-by David Sterba <[email protected]>
--- fs/btrfs/volumes.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-)
fs/btrfs/volumes.c +23 -3
@@ -6251,20 +6251,34 @@ int btrfs_read_sys_array(struct btrfs_root *root)while (cur_offset < array_size) { disk_key = (struct btrfs_disk_key *)array_ptr; + len =sizeof (*disk_key); +if (cur_offset + len > array_size) + goto out_short_read; +btrfs_disk_key_to_cpu (&key, disk_key); - len =sizeof (*disk_key); array_ptr += len; sb_array_offset += len; cur_offset += len;if (key.type ==BTRFS_CHUNK_ITEM_KEY ) { chunk = (struct btrfs_chunk *)sb_array_offset; +/* +* At least one btrfs_chunk with one stripe must be +* present, exact stripe count check comes afterwards +*/ + len =btrfs_chunk_item_size (1 ); +if (cur_offset + len > array_size) + goto out_short_read; + + num_stripes =btrfs_chunk_num_stripes (sb, chunk); + len =btrfs_chunk_item_size (num_stripes); +if (cur_offset + len > array_size) + goto out_short_read; + ret =read_one_chunk (root, &key, sb, chunk);if (ret)break ; - num_stripes =btrfs_chunk_num_stripes (sb, chunk); - len =btrfs_chunk_item_size (num_stripes); }else { ret = -EIO ;break ;
@@ -6275,6 +6289,12 @@ int btrfs_read_sys_array(struct btrfs_root *root) }free_extent_buffer (sb);return ret; + +out_short_read: +printk (KERN_ERR "BTRFS: sys_array too short to read %u bytes at offset %u\n" , + len, cur_offset); +free_extent_buffer (sb); +return -EIO ; }int btrfs_read_chunk_tree (struct btrfs_root *root)
Signature · 1 line
1.8.4.5