[PATCH v2] btrfs: fix validation of XATTR_ITEM dir items

[PATCH v2] btrfs: fix validation of XATTR_ITEM dir items

[David Sterba]

The XATTR_ITEM is a type of a directory item so we use the common
validator helper. Unlike other dir items, it can have data. The way the
name len validation is currently implemented does not reflect that. We'd
have to adjust by the data_len when comparing the read and item limits.

However, this will not work for multi-item xattr dir items.

Example from tree dump of generic/337:

        item 7 key (257 XATTR_ITEM 751495445) itemoff 15667 itemsize 147
                location key (0 UNKNOWN.0 0) type XATTR
                transid 8 data_len 3 name_len 11
                name: user.foobar
                data 123
                location key (0 UNKNOWN.0 0) type XATTR
                transid 8 data_len 6 name_len 13
                name: user.WvG1c1Td
                data qwerty
                location key (0 UNKNOWN.0 0) type XATTR
                transid 8 data_len 5 name_len 19
                name: user.J3__T_Km3dVsW_
                data hello

At the point of btrfs_is_name_len_valid call we don't have access to the
data_len value of the 2nd and 3rd sub-item. So simple btrfs_dir_data_len(leaf,
di) would always return 3, although we'd need to get 6 and 5 respectively to
get the claculations right. (read_end + name_len + data_len vs item_end)

We'd have to also pass data_len externally, which is not point of the
name validation. The last check is supposed to test if there's at least
one dir item space after the one we're processing. I don't think this is
particularly useful, validation of the next item would catch that too.
So the check is removed and we don't weaken the validation. Now tests
btrfs/048, btrfs/053, generic/273 and generic/337 pass.

Signed-off-by: David Sterba <dsterba@suse.com>
---

 fs/btrfs/dir-item.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c
index 2b00dd746118..41cb9196eaa8 100644
--- a/fs/btrfs/dir-item.c
+++ b/fs/btrfs/dir-item.c
@@ -550,14 +550,6 @@ bool btrfs_is_name_len_valid(struct extent_buffer *leaf, int slot,
 		goto out;
 	}
 
-	/*
-	 * This may be the last item in the slot
-	 * Or same type item(s) is left between read_end and item_end
-	 */
-	if (item_end != read_end && item_end - read_end < size) {
-		ret = false;
-		goto out;
-	}
 out:
 	if (!ret)
 		btrfs_crit(fs_info, "invalid dir item name len: %u",
-- 
2.13.0

[PATCH] btrfs: fix integer overflow in calc_reclaim_items_nr

[David Sterba]

From: Chris Mason <clm@fb.com>

Dave Jones hit a WARN_ON(nr < 0) in btrfs_wait_ordered_roots() with
v4.12-rc6.  This was because commit 70e7af244 made it possible for
calc_reclaim_items_nr() to return a negative number.  It's not really a
bug in that commit, it just didn't go far enough down the stack to find
all the possible 64->32 bit overflows.

This switches calc_reclaim_items_nr() to return a u64 and changes everyone
that uses the results of that math to u64 as well.

Reported-by: Dave Jones <davej@codemonkey.org.uk>
Fixes: 70e7af2 ("Btrfs: fix delalloc accounting leak caused by u32 overflow")
Signed-off-by: Chris Mason <clm@fb.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/dev-replace.c  |  4 ++--
 fs/btrfs/extent-tree.c  | 12 ++++++------
 fs/btrfs/ioctl.c        |  2 +-
 fs/btrfs/ordered-data.c | 17 ++++++++---------
 fs/btrfs/ordered-data.h |  4 ++--
 fs/btrfs/qgroup.c       |  2 +-
 fs/btrfs/relocation.c   |  2 +-
 fs/btrfs/scrub.c        |  2 +-
 fs/btrfs/super.c        |  2 +-
 fs/btrfs/transaction.c  |  2 +-
 10 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 5fe1ca8abc70..bee3edeea7a3 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -388,7 +388,7 @@ int btrfs_dev_replace_start(struct btrfs_fs_info *fs_info,
 	if (ret)
 		btrfs_err(fs_info, "kobj add dev failed %d", ret);
 
-	btrfs_wait_ordered_roots(fs_info, -1, 0, (u64)-1);
+	btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1);
 
 	/* force writing the updated state information to disk */
 	trans = btrfs_start_transaction(root, 0);
@@ -507,7 +507,7 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
 		mutex_unlock(&dev_replace->lock_finishing_cancel_unmount);
 		return ret;
 	}
-	btrfs_wait_ordered_roots(fs_info, -1, 0, (u64)-1);
+	btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1);
 
 	trans = btrfs_start_transaction(root, 0);
 	if (IS_ERR(trans)) {
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index a3339acec28c..375f8c728d91 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -4288,7 +4288,7 @@ int btrfs_alloc_data_chunk_ondemand(struct btrfs_inode *inode, u64 bytes)
 
 			if (need_commit > 0) {
 				btrfs_start_delalloc_roots(fs_info, 0, -1);
-				btrfs_wait_ordered_roots(fs_info, -1, 0,
+				btrfs_wait_ordered_roots(fs_info, U64_MAX, 0,
 							 (u64)-1);
 			}
 
@@ -4748,14 +4748,14 @@ static void btrfs_writeback_inodes_sb_nr(struct btrfs_fs_info *fs_info,
 	}
 }
 
-static inline int calc_reclaim_items_nr(struct btrfs_fs_info *fs_info,
+static inline u64 calc_reclaim_items_nr(struct btrfs_fs_info *fs_info,
 					u64 to_reclaim)
 {
 	u64 bytes;
-	int nr;
+	u64 nr;
 
 	bytes = btrfs_calc_trans_metadata_size(fs_info, 1);
-	nr = (int)div64_u64(to_reclaim, bytes);
+	nr = div64_u64(to_reclaim, bytes);
 	if (!nr)
 		nr = 1;
 	return nr;
@@ -4774,15 +4774,15 @@ static void shrink_delalloc(struct btrfs_fs_info *fs_info, u64 to_reclaim,
 	struct btrfs_trans_handle *trans;
 	u64 delalloc_bytes;
 	u64 max_reclaim;
+	u64 items;
 	long time_left;
 	unsigned long nr_pages;
 	int loops;
-	int items;
 	enum btrfs_reserve_flush_enum flush;
 
 	/* Calc the number of the pages we need flush for space reservation */
 	items = calc_reclaim_items_nr(fs_info, to_reclaim);
-	to_reclaim = (u64)items * EXTENT_SIZE_PER_ITEM;
+	to_reclaim = items * EXTENT_SIZE_PER_ITEM;
 
 	trans = (struct btrfs_trans_handle *)current->journal_info;
 	block_rsv = &fs_info->delalloc_block_rsv;
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index b4e9941efb60..fa1b78cf25f6 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -689,7 +689,7 @@ static int create_snapshot(struct btrfs_root *root, struct inode *dir,
 	if (ret)
 		goto dec_and_free;
 
-	btrfs_wait_ordered_extents(root, -1, 0, (u64)-1);
+	btrfs_wait_ordered_extents(root, U64_MAX, 0, (u64)-1);
 
 	btrfs_init_block_rsv(&pending_snapshot->block_rsv,
 			     BTRFS_BLOCK_RSV_TEMP);
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 7b40e2e7292a..a3aca495e33e 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -663,7 +663,7 @@ static void btrfs_run_ordered_extent_work(struct btrfs_work *work)
  * wait for all the ordered extents in a root.  This is done when balancing
  * space between drives.
  */
-int btrfs_wait_ordered_extents(struct btrfs_root *root, int nr,
+u64 btrfs_wait_ordered_extents(struct btrfs_root *root, u64 nr,
 			       const u64 range_start, const u64 range_len)
 {
 	struct btrfs_fs_info *fs_info = root->fs_info;
@@ -671,7 +671,7 @@ int btrfs_wait_ordered_extents(struct btrfs_root *root, int nr,
 	LIST_HEAD(skipped);
 	LIST_HEAD(works);
 	struct btrfs_ordered_extent *ordered, *next;
-	int count = 0;
+	u64 count = 0;
 	const u64 range_end = range_start + range_len;
 
 	mutex_lock(&root->ordered_extent_mutex);
@@ -701,7 +701,7 @@ int btrfs_wait_ordered_extents(struct btrfs_root *root, int nr,
 
 		cond_resched();
 		spin_lock(&root->ordered_extent_lock);
-		if (nr != -1)
+		if (nr != U64_MAX)
 			nr--;
 		count++;
 	}
@@ -720,13 +720,13 @@ int btrfs_wait_ordered_extents(struct btrfs_root *root, int nr,
 	return count;
 }
 
-int btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, int nr,
-			      const u64 range_start, const u64 range_len)
+u64 btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, u64 nr,
+			     const u64 range_start, const u64 range_len)
 {
 	struct btrfs_root *root;
 	struct list_head splice;
-	int done;
-	int total_done = 0;
+	u64 total_done = 0;
+	u64 done;
 
 	INIT_LIST_HEAD(&splice);
 
@@ -748,9 +748,8 @@ int btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, int nr,
 		total_done += done;
 
 		spin_lock(&fs_info->ordered_root_lock);
-		if (nr != -1) {
+		if (nr != U64_MAX) {
 			nr -= done;
-			WARN_ON(nr < 0);
 		}
 	}
 	list_splice_tail(&splice, &fs_info->ordered_roots);
diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
index e0c1d5b8d859..56c4c0ee6381 100644
--- a/fs/btrfs/ordered-data.h
+++ b/fs/btrfs/ordered-data.h
@@ -200,9 +200,9 @@ int btrfs_ordered_update_i_size(struct inode *inode, u64 offset,
 				struct btrfs_ordered_extent *ordered);
 int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
 			   u32 *sum, int len);
-int btrfs_wait_ordered_extents(struct btrfs_root *root, int nr,
+u64 btrfs_wait_ordered_extents(struct btrfs_root *root, u64 nr,
 			       const u64 range_start, const u64 range_len);
-int btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, int nr,
+u64 btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, u64 nr,
 			      const u64 range_start, const u64 range_len);
 void btrfs_get_logged_extents(struct btrfs_inode *inode,
 			      struct list_head *logged_list,
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index fc9dffaa9524..4ce351efe281 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -2405,7 +2405,7 @@ static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes, bool enforce)
 				ret = btrfs_start_delalloc_inodes(root, 0);
 				if (ret)
 					return ret;
-				btrfs_wait_ordered_extents(root, -1, 0, (u64)-1);
+				btrfs_wait_ordered_extents(root, U64_MAX, 0, (u64)-1);
 				trans = btrfs_join_transaction(root);
 				if (IS_ERR(trans))
 					return PTR_ERR(trans);
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index dc69b6ba29af..65661d1aae4e 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4373,7 +4373,7 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
 
 	btrfs_wait_block_group_reservations(rc->block_group);
 	btrfs_wait_nocow_writers(rc->block_group);
-	btrfs_wait_ordered_roots(fs_info, -1,
+	btrfs_wait_ordered_roots(fs_info, U64_MAX,
 				 rc->block_group->key.objectid,
 				 rc->block_group->key.offset);
 
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 738e784ba20d..0cebeb5eb5d0 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -3836,7 +3836,7 @@ int scrub_enumerate_chunks(struct scrub_ctx *sctx,
 			 */
 			btrfs_wait_block_group_reservations(cache);
 			btrfs_wait_nocow_writers(cache);
-			ret = btrfs_wait_ordered_roots(fs_info, -1,
+			ret = btrfs_wait_ordered_roots(fs_info, U64_MAX,
 						       cache->key.objectid,
 						       cache->key.offset);
 			if (ret > 0) {
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index ba2cb7f96986..74e47794e63f 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1177,7 +1177,7 @@ int btrfs_sync_fs(struct super_block *sb, int wait)
 		return 0;
 	}
 
-	btrfs_wait_ordered_roots(fs_info, -1, 0, (u64)-1);
+	btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1);
 
 	trans = btrfs_attach_transaction_barrier(root);
 	if (IS_ERR(trans)) {
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 309b73da756b..f615d59b0489 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1923,7 +1923,7 @@ static inline int btrfs_start_delalloc_flush(struct btrfs_fs_info *fs_info)
 static inline void btrfs_wait_delalloc_flush(struct btrfs_fs_info *fs_info)
 {
 	if (btrfs_test_opt(fs_info, FLUSHONCOMMIT))
-		btrfs_wait_ordered_roots(fs_info, -1, 0, (u64)-1);
+		btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1);
 }
 
 static inline void
-- 
2.13.0

Re: [PATCH] btrfs: fix integer overflow in calc_reclaim_items_nr

[David Sterba]

Please ignore this mail, not intended to be sent. Extra -1 on the
command line also picked the top patch.

On Thu, Jun 29, 2017 at 08:20:15PM +0200, David Sterba wrote:
> From: Chris Mason <clm@fb.com>
> 
> Dave Jones hit a WARN_ON(nr < 0) in btrfs_wait_ordered_roots() with
> v4.12-rc6.  This was because commit 70e7af244 made it possible for
> calc_reclaim_items_nr() to return a negative number.  It's not really a
> bug in that commit, it just didn't go far enough down the stack to find
> all the possible 64->32 bit overflows.
> 
> This switches calc_reclaim_items_nr() to return a u64 and changes everyone
> that uses the results of that math to u64 as well.
> 
> Reported-by: Dave Jones <davej@codemonkey.org.uk>
> Fixes: 70e7af2 ("Btrfs: fix delalloc accounting leak caused by u32 overflow")
> Signed-off-by: Chris Mason <clm@fb.com>
> Reviewed-by: David Sterba <dsterba@suse.com>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/dev-replace.c  |  4 ++--
>  fs/btrfs/extent-tree.c  | 12 ++++++------
>  fs/btrfs/ioctl.c        |  2 +-
>  fs/btrfs/ordered-data.c | 17 ++++++++---------
>  fs/btrfs/ordered-data.h |  4 ++--
>  fs/btrfs/qgroup.c       |  2 +-
>  fs/btrfs/relocation.c   |  2 +-
>  fs/btrfs/scrub.c        |  2 +-
>  fs/btrfs/super.c        |  2 +-
>  fs/btrfs/transaction.c  |  2 +-
>  10 files changed, 24 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
> index 5fe1ca8abc70..bee3edeea7a3 100644
> --- a/fs/btrfs/dev-replace.c
> +++ b/fs/btrfs/dev-replace.c
> @@ -388,7 +388,7 @@ int btrfs_dev_replace_start(struct btrfs_fs_info *fs_info,
>  	if (ret)
>  		btrfs_err(fs_info, "kobj add dev failed %d", ret);
>  
> -	btrfs_wait_ordered_roots(fs_info, -1, 0, (u64)-1);
> +	btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1);
>  
>  	/* force writing the updated state information to disk */
>  	trans = btrfs_start_transaction(root, 0);
> @@ -507,7 +507,7 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
>  		mutex_unlock(&dev_replace->lock_finishing_cancel_unmount);
>  		return ret;
>  	}
> -	btrfs_wait_ordered_roots(fs_info, -1, 0, (u64)-1);
> +	btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1);
>  
>  	trans = btrfs_start_transaction(root, 0);
>  	if (IS_ERR(trans)) {
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index a3339acec28c..375f8c728d91 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -4288,7 +4288,7 @@ int btrfs_alloc_data_chunk_ondemand(struct btrfs_inode *inode, u64 bytes)
>  
>  			if (need_commit > 0) {
>  				btrfs_start_delalloc_roots(fs_info, 0, -1);
> -				btrfs_wait_ordered_roots(fs_info, -1, 0,
> +				btrfs_wait_ordered_roots(fs_info, U64_MAX, 0,
>  							 (u64)-1);
>  			}
>  
> @@ -4748,14 +4748,14 @@ static void btrfs_writeback_inodes_sb_nr(struct btrfs_fs_info *fs_info,
>  	}
>  }
>  
> -static inline int calc_reclaim_items_nr(struct btrfs_fs_info *fs_info,
> +static inline u64 calc_reclaim_items_nr(struct btrfs_fs_info *fs_info,
>  					u64 to_reclaim)
>  {
>  	u64 bytes;
> -	int nr;
> +	u64 nr;
>  
>  	bytes = btrfs_calc_trans_metadata_size(fs_info, 1);
> -	nr = (int)div64_u64(to_reclaim, bytes);
> +	nr = div64_u64(to_reclaim, bytes);
>  	if (!nr)
>  		nr = 1;
>  	return nr;
> @@ -4774,15 +4774,15 @@ static void shrink_delalloc(struct btrfs_fs_info *fs_info, u64 to_reclaim,
>  	struct btrfs_trans_handle *trans;
>  	u64 delalloc_bytes;
>  	u64 max_reclaim;
> +	u64 items;
>  	long time_left;
>  	unsigned long nr_pages;
>  	int loops;
> -	int items;
>  	enum btrfs_reserve_flush_enum flush;
>  
>  	/* Calc the number of the pages we need flush for space reservation */
>  	items = calc_reclaim_items_nr(fs_info, to_reclaim);
> -	to_reclaim = (u64)items * EXTENT_SIZE_PER_ITEM;
> +	to_reclaim = items * EXTENT_SIZE_PER_ITEM;
>  
>  	trans = (struct btrfs_trans_handle *)current->journal_info;
>  	block_rsv = &fs_info->delalloc_block_rsv;
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index b4e9941efb60..fa1b78cf25f6 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -689,7 +689,7 @@ static int create_snapshot(struct btrfs_root *root, struct inode *dir,
>  	if (ret)
>  		goto dec_and_free;
>  
> -	btrfs_wait_ordered_extents(root, -1, 0, (u64)-1);
> +	btrfs_wait_ordered_extents(root, U64_MAX, 0, (u64)-1);
>  
>  	btrfs_init_block_rsv(&pending_snapshot->block_rsv,
>  			     BTRFS_BLOCK_RSV_TEMP);
> diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
> index 7b40e2e7292a..a3aca495e33e 100644
> --- a/fs/btrfs/ordered-data.c
> +++ b/fs/btrfs/ordered-data.c
> @@ -663,7 +663,7 @@ static void btrfs_run_ordered_extent_work(struct btrfs_work *work)
>   * wait for all the ordered extents in a root.  This is done when balancing
>   * space between drives.
>   */
> -int btrfs_wait_ordered_extents(struct btrfs_root *root, int nr,
> +u64 btrfs_wait_ordered_extents(struct btrfs_root *root, u64 nr,
>  			       const u64 range_start, const u64 range_len)
>  {
>  	struct btrfs_fs_info *fs_info = root->fs_info;
> @@ -671,7 +671,7 @@ int btrfs_wait_ordered_extents(struct btrfs_root *root, int nr,
>  	LIST_HEAD(skipped);
>  	LIST_HEAD(works);
>  	struct btrfs_ordered_extent *ordered, *next;
> -	int count = 0;
> +	u64 count = 0;
>  	const u64 range_end = range_start + range_len;
>  
>  	mutex_lock(&root->ordered_extent_mutex);
> @@ -701,7 +701,7 @@ int btrfs_wait_ordered_extents(struct btrfs_root *root, int nr,
>  
>  		cond_resched();
>  		spin_lock(&root->ordered_extent_lock);
> -		if (nr != -1)
> +		if (nr != U64_MAX)
>  			nr--;
>  		count++;
>  	}
> @@ -720,13 +720,13 @@ int btrfs_wait_ordered_extents(struct btrfs_root *root, int nr,
>  	return count;
>  }
>  
> -int btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, int nr,
> -			      const u64 range_start, const u64 range_len)
> +u64 btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, u64 nr,
> +			     const u64 range_start, const u64 range_len)
>  {
>  	struct btrfs_root *root;
>  	struct list_head splice;
> -	int done;
> -	int total_done = 0;
> +	u64 total_done = 0;
> +	u64 done;
>  
>  	INIT_LIST_HEAD(&splice);
>  
> @@ -748,9 +748,8 @@ int btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, int nr,
>  		total_done += done;
>  
>  		spin_lock(&fs_info->ordered_root_lock);
> -		if (nr != -1) {
> +		if (nr != U64_MAX) {
>  			nr -= done;
> -			WARN_ON(nr < 0);
>  		}
>  	}
>  	list_splice_tail(&splice, &fs_info->ordered_roots);
> diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
> index e0c1d5b8d859..56c4c0ee6381 100644
> --- a/fs/btrfs/ordered-data.h
> +++ b/fs/btrfs/ordered-data.h
> @@ -200,9 +200,9 @@ int btrfs_ordered_update_i_size(struct inode *inode, u64 offset,
>  				struct btrfs_ordered_extent *ordered);
>  int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
>  			   u32 *sum, int len);
> -int btrfs_wait_ordered_extents(struct btrfs_root *root, int nr,
> +u64 btrfs_wait_ordered_extents(struct btrfs_root *root, u64 nr,
>  			       const u64 range_start, const u64 range_len);
> -int btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, int nr,
> +u64 btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, u64 nr,
>  			      const u64 range_start, const u64 range_len);
>  void btrfs_get_logged_extents(struct btrfs_inode *inode,
>  			      struct list_head *logged_list,
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index fc9dffaa9524..4ce351efe281 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -2405,7 +2405,7 @@ static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes, bool enforce)
>  				ret = btrfs_start_delalloc_inodes(root, 0);
>  				if (ret)
>  					return ret;
> -				btrfs_wait_ordered_extents(root, -1, 0, (u64)-1);
> +				btrfs_wait_ordered_extents(root, U64_MAX, 0, (u64)-1);
>  				trans = btrfs_join_transaction(root);
>  				if (IS_ERR(trans))
>  					return PTR_ERR(trans);
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index dc69b6ba29af..65661d1aae4e 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -4373,7 +4373,7 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
>  
>  	btrfs_wait_block_group_reservations(rc->block_group);
>  	btrfs_wait_nocow_writers(rc->block_group);
> -	btrfs_wait_ordered_roots(fs_info, -1,
> +	btrfs_wait_ordered_roots(fs_info, U64_MAX,
>  				 rc->block_group->key.objectid,
>  				 rc->block_group->key.offset);
>  
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 738e784ba20d..0cebeb5eb5d0 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -3836,7 +3836,7 @@ int scrub_enumerate_chunks(struct scrub_ctx *sctx,
>  			 */
>  			btrfs_wait_block_group_reservations(cache);
>  			btrfs_wait_nocow_writers(cache);
> -			ret = btrfs_wait_ordered_roots(fs_info, -1,
> +			ret = btrfs_wait_ordered_roots(fs_info, U64_MAX,
>  						       cache->key.objectid,
>  						       cache->key.offset);
>  			if (ret > 0) {
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index ba2cb7f96986..74e47794e63f 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1177,7 +1177,7 @@ int btrfs_sync_fs(struct super_block *sb, int wait)
>  		return 0;
>  	}
>  
> -	btrfs_wait_ordered_roots(fs_info, -1, 0, (u64)-1);
> +	btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1);
>  
>  	trans = btrfs_attach_transaction_barrier(root);
>  	if (IS_ERR(trans)) {
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 309b73da756b..f615d59b0489 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -1923,7 +1923,7 @@ static inline int btrfs_start_delalloc_flush(struct btrfs_fs_info *fs_info)
>  static inline void btrfs_wait_delalloc_flush(struct btrfs_fs_info *fs_info)
>  {
>  	if (btrfs_test_opt(fs_info, FLUSHONCOMMIT))
> -		btrfs_wait_ordered_roots(fs_info, -1, 0, (u64)-1);
> +		btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1);
>  }
>  
>  static inline void
> -- 
> 2.13.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] btrfs: fix validation of XATTR_ITEM dir items

[Su Yue]

Thanks a lot.
I'm sorry for that I hadn't noticed those emails during the vacation.

On 06/30/2017 02:20 AM, David Sterba wrote:
> The XATTR_ITEM is a type of a directory item so we use the common
> validator helper. Unlike other dir items, it can have data. The way the
> name len validation is currently implemented does not reflect that. We'd
> have to adjust by the data_len when comparing the read and item limits.
> 
> However, this will not work for multi-item xattr dir items.
> 
> Example from tree dump of generic/337:
> 
>          item 7 key (257 XATTR_ITEM 751495445) itemoff 15667 itemsize 147
>                  location key (0 UNKNOWN.0 0) type XATTR
>                  transid 8 data_len 3 name_len 11
>                  name: user.foobar
>                  data 123
>                  location key (0 UNKNOWN.0 0) type XATTR
>                  transid 8 data_len 6 name_len 13
>                  name: user.WvG1c1Td
>                  data qwerty
>                  location key (0 UNKNOWN.0 0) type XATTR
>                  transid 8 data_len 5 name_len 19
>                  name: user.J3__T_Km3dVsW_
>                  data hello
> 
> At the point of btrfs_is_name_len_valid call we don't have access to the
> data_len value of the 2nd and 3rd sub-item. So simple btrfs_dir_data_len(leaf,
> di) would always return 3, although we'd need to get 6 and 5 respectively to
> get the claculations right. (read_end + name_len + data_len vs item_end)
> 
> We'd have to also pass data_len externally, which is not point of the
> name validation. The last check is supposed to test if there's at least
> one dir item space after the one we're processing. I don't think this is
> particularly useful, validation of the next item would catch that too.
> So the check is removed and we don't weaken the validation. Now tests
> btrfs/048, btrfs/053, generic/273 and generic/337 pass. >
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
> 
>   fs/btrfs/dir-item.c | 8 --------
>   1 file changed, 8 deletions(-)
> 
> diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c
> index 2b00dd746118..41cb9196eaa8 100644
> --- a/fs/btrfs/dir-item.c
> +++ b/fs/btrfs/dir-item.c
> @@ -550,14 +550,6 @@ bool btrfs_is_name_len_valid(struct extent_buffer *leaf, int slot,
>   		goto out;
>   	}
>   
> -	/*
> -	 * This may be the last item in the slot
> -	 * Or same type item(s) is left between read_end and item_end
> -	 */
> -	if (item_end != read_end && item_end - read_end < size) {
> -		ret = false;
> -		goto out;
> -	}
>   out:
>   	if (!ret)
>   		btrfs_crit(fs_info, "invalid dir item name len: %u",
> 

[PATCH] btrfs: fix integer overflow in calc_reclaim_items_nr

[Chris Mason]

Dave Jones hit a WARN_ON(nr < 0) in btrfs_wait_ordered_roots() with
v4.12-rc6.  This was because commit 70e7af244 made it possible for
calc_reclaim_items_nr() to return a negative number.  It's not really a
bug in that commit, it just didn't go far enough down the stack to find
all the possible 64->32 bit overflows.

This switches calc_reclaim_items_nr() to return a u64 and changes everyone
that uses the results of that math to u64 as well.

Reported-by: Dave Jones <davej@codemonkey.org.uk>
Fixes: 70e7af244f24c94604ef6eca32ad297632018583
Signed-off-by: Chris Mason <clm@fb.com>
---
 fs/btrfs/dev-replace.c  |  4 ++--
 fs/btrfs/extent-tree.c  | 10 +++++-----
 fs/btrfs/ioctl.c        |  2 +-
 fs/btrfs/ordered-data.c | 17 ++++++++---------
 fs/btrfs/ordered-data.h |  4 ++--
 fs/btrfs/qgroup.c       |  2 +-
 fs/btrfs/relocation.c   |  2 +-
 fs/btrfs/scrub.c        |  2 +-
 fs/btrfs/super.c        |  2 +-
 fs/btrfs/transaction.c  |  2 +-
 10 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 5fe1ca8..bee3ede 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -388,7 +388,7 @@ int btrfs_dev_replace_start(struct btrfs_fs_info *fs_info,
 	if (ret)
 		btrfs_err(fs_info, "kobj add dev failed %d", ret);
 
-	btrfs_wait_ordered_roots(fs_info, -1, 0, (u64)-1);
+	btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1);
 
 	/* force writing the updated state information to disk */
 	trans = btrfs_start_transaction(root, 0);
@@ -507,7 +507,7 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
 		mutex_unlock(&dev_replace->lock_finishing_cancel_unmount);
 		return ret;
 	}
-	btrfs_wait_ordered_roots(fs_info, -1, 0, (u64)-1);
+	btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1);
 
 	trans = btrfs_start_transaction(root, 0);
 	if (IS_ERR(trans)) {
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 33d979e9..d675324 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -4238,7 +4238,7 @@ int btrfs_alloc_data_chunk_ondemand(struct btrfs_inode *inode, u64 bytes)
 
 			if (need_commit > 0) {
 				btrfs_start_delalloc_roots(fs_info, 0, -1);
-				btrfs_wait_ordered_roots(fs_info, -1, 0,
+				btrfs_wait_ordered_roots(fs_info, U64_MAX, 0,
 							 (u64)-1);
 			}
 
@@ -4698,11 +4698,11 @@ static void btrfs_writeback_inodes_sb_nr(struct btrfs_fs_info *fs_info,
 	}
 }
 
-static inline int calc_reclaim_items_nr(struct btrfs_fs_info *fs_info,
+static inline u64 calc_reclaim_items_nr(struct btrfs_fs_info *fs_info,
 					u64 to_reclaim)
 {
 	u64 bytes;
-	int nr;
+	u64 nr;
 
 	bytes = btrfs_calc_trans_metadata_size(fs_info, 1);
 	nr = (int)div64_u64(to_reclaim, bytes);
@@ -4725,15 +4725,15 @@ static void shrink_delalloc(struct btrfs_root *root, u64 to_reclaim, u64 orig,
 	struct btrfs_trans_handle *trans;
 	u64 delalloc_bytes;
 	u64 max_reclaim;
+	u64 items;
 	long time_left;
 	unsigned long nr_pages;
 	int loops;
-	int items;
 	enum btrfs_reserve_flush_enum flush;
 
 	/* Calc the number of the pages we need flush for space reservation */
 	items = calc_reclaim_items_nr(fs_info, to_reclaim);
-	to_reclaim = (u64)items * EXTENT_SIZE_PER_ITEM;
+	to_reclaim = items * EXTENT_SIZE_PER_ITEM;
 
 	trans = (struct btrfs_trans_handle *)current->journal_info;
 	block_rsv = &fs_info->delalloc_block_rsv;
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index e176375..7712217 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -689,7 +689,7 @@ static int create_snapshot(struct btrfs_root *root, struct inode *dir,
 	if (ret)
 		goto dec_and_free;
 
-	btrfs_wait_ordered_extents(root, -1, 0, (u64)-1);
+	btrfs_wait_ordered_extents(root, U64_MAX, 0, (u64)-1);
 
 	btrfs_init_block_rsv(&pending_snapshot->block_rsv,
 			     BTRFS_BLOCK_RSV_TEMP);
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 7b40e2e..a3aca49 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -663,7 +663,7 @@ static void btrfs_run_ordered_extent_work(struct btrfs_work *work)
  * wait for all the ordered extents in a root.  This is done when balancing
  * space between drives.
  */
-int btrfs_wait_ordered_extents(struct btrfs_root *root, int nr,
+u64 btrfs_wait_ordered_extents(struct btrfs_root *root, u64 nr,
 			       const u64 range_start, const u64 range_len)
 {
 	struct btrfs_fs_info *fs_info = root->fs_info;
@@ -671,7 +671,7 @@ int btrfs_wait_ordered_extents(struct btrfs_root *root, int nr,
 	LIST_HEAD(skipped);
 	LIST_HEAD(works);
 	struct btrfs_ordered_extent *ordered, *next;
-	int count = 0;
+	u64 count = 0;
 	const u64 range_end = range_start + range_len;
 
 	mutex_lock(&root->ordered_extent_mutex);
@@ -701,7 +701,7 @@ int btrfs_wait_ordered_extents(struct btrfs_root *root, int nr,
 
 		cond_resched();
 		spin_lock(&root->ordered_extent_lock);
-		if (nr != -1)
+		if (nr != U64_MAX)
 			nr--;
 		count++;
 	}
@@ -720,13 +720,13 @@ int btrfs_wait_ordered_extents(struct btrfs_root *root, int nr,
 	return count;
 }
 
-int btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, int nr,
-			      const u64 range_start, const u64 range_len)
+u64 btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, u64 nr,
+			     const u64 range_start, const u64 range_len)
 {
 	struct btrfs_root *root;
 	struct list_head splice;
-	int done;
-	int total_done = 0;
+	u64 total_done = 0;
+	u64 done;
 
 	INIT_LIST_HEAD(&splice);
 
@@ -748,9 +748,8 @@ int btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, int nr,
 		total_done += done;
 
 		spin_lock(&fs_info->ordered_root_lock);
-		if (nr != -1) {
+		if (nr != U64_MAX) {
 			nr -= done;
-			WARN_ON(nr < 0);
 		}
 	}
 	list_splice_tail(&splice, &fs_info->ordered_roots);
diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
index e0c1d5b..56c4c0e 100644
--- a/fs/btrfs/ordered-data.h
+++ b/fs/btrfs/ordered-data.h
@@ -200,9 +200,9 @@ int btrfs_ordered_update_i_size(struct inode *inode, u64 offset,
 				struct btrfs_ordered_extent *ordered);
 int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
 			   u32 *sum, int len);
-int btrfs_wait_ordered_extents(struct btrfs_root *root, int nr,
+u64 btrfs_wait_ordered_extents(struct btrfs_root *root, u64 nr,
 			       const u64 range_start, const u64 range_len);
-int btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, int nr,
+u64 btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, u64 nr,
 			      const u64 range_start, const u64 range_len);
 void btrfs_get_logged_extents(struct btrfs_inode *inode,
 			      struct list_head *logged_list,
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index deffbeb..25ef976 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -2376,7 +2376,7 @@ static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes, bool enforce)
 				ret = btrfs_start_delalloc_inodes(root, 0);
 				if (ret)
 					return ret;
-				btrfs_wait_ordered_extents(root, -1, 0, (u64)-1);
+				btrfs_wait_ordered_extents(root, U64_MAX, 0, (u64)-1);
 				trans = btrfs_join_transaction(root);
 				if (IS_ERR(trans))
 					return PTR_ERR(trans);
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index d60df51..c16abd7 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4372,7 +4372,7 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
 
 	btrfs_wait_block_group_reservations(rc->block_group);
 	btrfs_wait_nocow_writers(rc->block_group);
-	btrfs_wait_ordered_roots(fs_info, -1,
+	btrfs_wait_ordered_roots(fs_info, U64_MAX,
 				 rc->block_group->key.objectid,
 				 rc->block_group->key.offset);
 
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index c7b45eb..b8a079e 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -3859,7 +3859,7 @@ int scrub_enumerate_chunks(struct scrub_ctx *sctx,
 			 */
 			btrfs_wait_block_group_reservations(cache);
 			btrfs_wait_nocow_writers(cache);
-			ret = btrfs_wait_ordered_roots(fs_info, -1,
+			ret = btrfs_wait_ordered_roots(fs_info, U64_MAX,
 						       cache->key.objectid,
 						       cache->key.offset);
 			if (ret > 0) {
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 4f1cdd5..0066f2d 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1187,7 +1187,7 @@ int btrfs_sync_fs(struct super_block *sb, int wait)
 		return 0;
 	}
 
-	btrfs_wait_ordered_roots(fs_info, -1, 0, (u64)-1);
+	btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1);
 
 	trans = btrfs_attach_transaction_barrier(root);
 	if (IS_ERR(trans)) {
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 2168654..0a3023a 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1926,7 +1926,7 @@ static inline int btrfs_start_delalloc_flush(struct btrfs_fs_info *fs_info)
 static inline void btrfs_wait_delalloc_flush(struct btrfs_fs_info *fs_info)
 {
 	if (btrfs_test_opt(fs_info, FLUSHONCOMMIT))
-		btrfs_wait_ordered_roots(fs_info, -1, 0, (u64)-1);
+		btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1);
 }
 
 static inline void
-- 
2.9.3

Re: [PATCH] btrfs: fix integer overflow in calc_reclaim_items_nr

[Holger Hoffstätte]

On 06/23/17 16:32, Chris Mason wrote:
[..]
> -static inline int calc_reclaim_items_nr(struct btrfs_fs_info *fs_info,
> +static inline u64 calc_reclaim_items_nr(struct btrfs_fs_info *fs_info,
>  					u64 to_reclaim)
>  {
>  	u64 bytes;
> -	int nr;
> +	u64 nr;
>  
>  	bytes = btrfs_calc_trans_metadata_size(fs_info, 1);
>  	nr = (int)div64_u64(to_reclaim, bytes);
        ^^^^^^^^^^

Isn't this a bit odd? I can't even tell whether it matters, just randomly
noticed it because I genuinely dislike type casts..

-h

Re: [PATCH] btrfs: fix integer overflow in calc_reclaim_items_nr

[David Sterba]

On Fri, Jun 23, 2017 at 05:29:56PM +0200, Holger Hoffstätte wrote:
> On 06/23/17 16:32, Chris Mason wrote:
> [..]
> > -static inline int calc_reclaim_items_nr(struct btrfs_fs_info *fs_info,
> > +static inline u64 calc_reclaim_items_nr(struct btrfs_fs_info *fs_info,
> >  					u64 to_reclaim)
> >  {
> >  	u64 bytes;
> > -	int nr;
> > +	u64 nr;
> >  
> >  	bytes = btrfs_calc_trans_metadata_size(fs_info, 1);
> >  	nr = (int)div64_u64(to_reclaim, bytes);
>         ^^^^^^^^^^
> 
> Isn't this a bit odd? I can't even tell whether it matters, just randomly
> noticed it because I genuinely dislike type casts..

I think the typecast shold be dropped, now that nr is u64.

Re: [PATCH] btrfs: fix integer overflow in calc_reclaim_items_nr

[Chris Mason]

On 06/23/2017 11:29 AM, Holger Hoffstätte wrote:
> On 06/23/17 16:32, Chris Mason wrote:
> [..]
>> -static inline int calc_reclaim_items_nr(struct btrfs_fs_info *fs_info,
>> +static inline u64 calc_reclaim_items_nr(struct btrfs_fs_info *fs_info,
>>  					u64 to_reclaim)
>>  {
>>  	u64 bytes;
>> -	int nr;
>> +	u64 nr;
>>
>>  	bytes = btrfs_calc_trans_metadata_size(fs_info, 1);
>>  	nr = (int)div64_u64(to_reclaim, bytes);
>         ^^^^^^^^^^
>
> Isn't this a bit odd? I can't even tell whether it matters, just randomly
> noticed it because I genuinely dislike type casts..
>

Crud, yes that's broken.  Thanks!

-chris