[2/3] btrfs: check namelen with boundary in verify dir_item

Commit Message

Su Yue May 25, 2017, 2:09 a.m. UTC

Origin 'verify_dir_item' verify namelen of dir_item with fixed values
but no item boundary.
If corrupted namelen was not bigger than the fixed value, for example 255,
the function will think the dir_item is fine. And then reading beyond
boundary will cause crash.

Add a parameter 'slot' and check namelen with item boundary by
calling 'btrfs_check_namelen'.

Signed-off-by: Su Yue <[e-mail]>
---
 fs/btrfs/ctree.h    |  2 +-
 fs/btrfs/dir-item.c | 13 ++++++++++++-
 fs/btrfs/inode.c    |  2 +-
 fs/btrfs/tree-log.c |  4 ++--
 fs/btrfs/xattr.c    |  2 +-
 5 files changed, 17 insertions(+), 6 deletions(-)

Comments

David Sterba May 29, 2017, 3:24 p.m. UTC | #1

On Thu, May 25, 2017 at 10:09:07AM +0800, Su Yue wrote:
> @@ -472,6 +474,15 @@ int verify_dir_item(struct btrfs_fs_info *fs_info,
>  		return 1;
>  	}
>  
> +	namelen = btrfs_dir_name_len(leaf, dir_item);
> +	namelen_ret = btrfs_check_namelen(leaf, slot,
> +			(unsigned long)(dir_item + 1), namelen);
> +	if (namelen_ret != namelen) {
> +		btrfs_crit(fs_info, "invalid dir item name len: %u",
> +		       (unsigned)btrfs_dir_name_len(leaf, dir_item));
> +		return 1;
> +	}

Extending verify_dir_item is ok, as we always have the slot parameter to
pass, no problem here.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index f49e04e7612b..99dd2c25ac6b 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3031,7 +3031,7 @@  struct btrfs_dir_item *btrfs_lookup_xattr(struct btrfs_trans_handle *trans,
 					  const char *name, u16 name_len,
 					  int mod);
 int verify_dir_item(struct btrfs_fs_info *fs_info,
-		    struct extent_buffer *leaf,
+		    struct extent_buffer *leaf, int slot,
 		    struct btrfs_dir_item *dir_item);
 struct btrfs_dir_item *btrfs_match_dir_item_name(struct btrfs_fs_info *fs_info,
 						 struct btrfs_path *path,
diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c
index 7af5ad8e9a3c..82390f36101c 100644
--- a/fs/btrfs/dir-item.c
+++ b/fs/btrfs/dir-item.c
@@ -395,7 +395,7 @@  struct btrfs_dir_item *btrfs_match_dir_item_name(struct btrfs_fs_info *fs_info,
 
 	leaf = path->nodes[0];
 	dir_item = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_dir_item);
-	if (verify_dir_item(fs_info, leaf, dir_item))
+	if (verify_dir_item(fs_info, leaf, path->slots[0], dir_item))
 		return NULL;
 
 	total_len = btrfs_item_size_nr(leaf, path->slots[0]);
@@ -453,9 +453,11 @@  int btrfs_delete_one_dir_name(struct btrfs_trans_handle *trans,
 
 int verify_dir_item(struct btrfs_fs_info *fs_info,
 		    struct extent_buffer *leaf,
+		    int slot,
 		    struct btrfs_dir_item *dir_item)
 {
 	u16 namelen = BTRFS_NAME_LEN;
+	u16 namelen_ret;
 	u8 type = btrfs_dir_type(leaf, dir_item);
 
 	if (type >= BTRFS_FT_MAX) {
@@ -472,6 +474,15 @@  int verify_dir_item(struct btrfs_fs_info *fs_info,
 		return 1;
 	}
 
+	namelen = btrfs_dir_name_len(leaf, dir_item);
+	namelen_ret = btrfs_check_namelen(leaf, slot,
+			(unsigned long)(dir_item + 1), namelen);
+	if (namelen_ret != namelen) {
+		btrfs_crit(fs_info, "invalid dir item name len: %u",
+		       (unsigned)btrfs_dir_name_len(leaf, dir_item));
+		return 1;
+	}
+
 	/* BTRFS_MAX_XATTR_SIZE is the same for all dir items */
 	if ((btrfs_dir_data_len(leaf, dir_item) +
 	     btrfs_dir_name_len(leaf, dir_item)) >
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 17cbe9306faf..df948569c393 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5934,7 +5934,7 @@  static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
 		ctx->pos = found_key.offset;
 
 		di = btrfs_item_ptr(leaf, slot, struct btrfs_dir_item);
-		if (verify_dir_item(fs_info, leaf, di))
+		if (verify_dir_item(fs_info, leaf, slot, di))
 			goto next;
 
 		name_len = btrfs_dir_name_len(leaf, di);
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index ccfe9fe7754a..1930f28edcdd 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -1841,7 +1841,7 @@  static noinline int replay_one_dir_item(struct btrfs_trans_handle *trans,
 	ptr_end = ptr + item_size;
 	while (ptr < ptr_end) {
 		di = (struct btrfs_dir_item *)ptr;
-		if (verify_dir_item(fs_info, eb, di))
+		if (verify_dir_item(fs_info, eb, slot, di))
 			return -EIO;
 		name_len = btrfs_dir_name_len(eb, di);
 		ret = replay_one_name(trans, root, path, eb, di, key);
@@ -2017,7 +2017,7 @@  static noinline int check_item_in_log(struct btrfs_trans_handle *trans,
 	ptr_end = ptr + item_size;
 	while (ptr < ptr_end) {
 		di = (struct btrfs_dir_item *)ptr;
-		if (verify_dir_item(fs_info, eb, di)) {
+		if (verify_dir_item(fs_info, eb, slot, di)) {
 			ret = -EIO;
 			goto out;
 		}
diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
index b3cbf80c5acf..2c7e53f9ff1b 100644
--- a/fs/btrfs/xattr.c
+++ b/fs/btrfs/xattr.c
@@ -336,7 +336,7 @@  ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size)
 			u32 this_len = sizeof(*di) + name_len + data_len;
 			unsigned long name_ptr = (unsigned long)(di + 1);
 
-			if (verify_dir_item(fs_info, leaf, di)) {
+			if (verify_dir_item(fs_info, leaf, slot, di)) {
 				ret = -EIO;
 				goto err;
 			}