[PATCH] btrfs: Fix out of bounds access in btrfs_search_slot

[PATCH] btrfs: Fix out of bounds access in btrfs_search_slot

[Nikolay Borisov]

When modifying a tree where the root is at BTRFS_MAX_LEVEL - 1 then
the level variable is going to be 7 (this is the max height of the
tree). On the other hand btrfs_cow_block is always called with
"level + 1" as an index into the nodes and slots arrays. This leads to
an out of bounds access. Admittdely this will be benign since an OOB
access of the nodes array will likely read the 0th element from the
slots array, which in this case is going to be 0 (since we start CoW at
the top of the tree). The OOB access into the slots array in turn will
read the 0th and 1st values of the locks array, which would both be 0
at the time. However, this benign behavior relies on the fact that the 
path being passed hasn't been initialised, if it has already been used to 
query a btree then it could potentially have populated the nodes/slots arrays.

Fix it by explicitly checking if we are at level 7 (the maximum allowed
index in nodes/slots arrays) and explicitly call the CoW routine with
NULL for parent's node/slot.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Fixes-coverity-id: 711515
---
 fs/btrfs/ctree.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 1f001d31bda8..076b704798c9 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -2780,6 +2780,8 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		 * contention with the cow code
 		 */
 		if (cow) {
+			bool last_level = (level == (BTRFS_MAX_LEVEL - 1));
+
 			/*
 			 * if we don't really need to cow this block
 			 * then we don't want to set the path blocking,
@@ -2804,9 +2806,13 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 			}
 
 			btrfs_set_path_blocking(p);
-			err = btrfs_cow_block(trans, root, b,
-					      p->nodes[level + 1],
-					      p->slots[level + 1], &b);
+			if (last_level)
+				err = btrfs_cow_block(trans, root, b, NULL, 0,
+						      &b);
+			else
+				err = btrfs_cow_block(trans, root, b,
+						      p->nodes[level + 1],
+						      p->slots[level + 1], &b);
 			if (err) {
 				ret = err;
 				goto done;
-- 
2.7.4

Re: [PATCH] btrfs: Fix out of bounds access in btrfs_search_slot

[David Sterba]

On Tue, Dec 12, 2017 at 11:14:49AM +0200, Nikolay Borisov wrote:
> When modifying a tree where the root is at BTRFS_MAX_LEVEL - 1 then
> the level variable is going to be 7 (this is the max height of the
> tree). On the other hand btrfs_cow_block is always called with
> "level + 1" as an index into the nodes and slots arrays. This leads to
> an out of bounds access. Admittdely this will be benign since an OOB
> access of the nodes array will likely read the 0th element from the
> slots array, which in this case is going to be 0 (since we start CoW at
> the top of the tree). The OOB access into the slots array in turn will
> read the 0th and 1st values of the locks array, which would both be 0
> at the time. However, this benign behavior relies on the fact that the 
> path being passed hasn't been initialised, if it has already been used to 
> query a btree then it could potentially have populated the nodes/slots arrays.
> 
> Fix it by explicitly checking if we are at level 7 (the maximum allowed
> index in nodes/slots arrays) and explicitly call the CoW routine with
> NULL for parent's node/slot.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> Fixes-coverity-id: 711515

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