btrfs: fix potential null pointer dereference bug

The bug happens when rb_node == NULL. It causes variable node to be NULL and then the NULL pointer is dereferenced this line:

	BUG_ON((struct btrfs_root *)node->data != root);

Based on my analysis, function tree_search should not return NULL to variable rb_node in this case (otherwise here has to be something unknown thing wrong), so I replace "if (rb_node)" with UG_ON(!rb_node).

---
 fs/btrfs/relocation.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

index 17c306b..d674671 100644
@@ -1263,10 +1263,11 @@ static int __update_reloc_root(struct btrfs_root *root, int del)
 	spin_lock(&rc->reloc_root_tree.lock);
 	rb_node = tree_search(&rc->reloc_root_tree.rb_root,
 			      root->commit_root->start);
-	if (rb_node) {
-		node = rb_entry(rb_node, struct mapping_node, rb_node);
-		rb_erase(&node->rb_node, &rc->reloc_root_tree.rb_root);
-	}
+	BUG_ON(!rb_node);
+
+	node = rb_entry(rb_node, struct mapping_node, rb_node);
+	rb_erase(&node->rb_node, &rc->reloc_root_tree.rb_root);
+
 	spin_unlock(&rc->reloc_root_tree.lock);
 
 	BUG_ON((struct btrfs_root *)node->data != root);

1.7.10.4

I don't want to add more BUG_ON()'s, just return an error.

Josef

But rb_node really has no chance to be 0, so I think we should use BUG_ON rather than return an error. If we return an error number here, its caller should check the returned value and call BUG_ON(ret) - it makes no difference.

The file system doesn't have any way to handle this kind of error (and I think if it happens, there must be some hardware error or some other program directly operates on the file system bypassing linux system call). If you don't want to add more BUG_ON, I suggest to check variable "node" in the existing BUG_ON as the following code.
- BUG_ON((struct btrfs_root *)node->data != root);
+ BUG_ON(!node || (struct btrfs_root *)node->data != root);

What's your opinion, or do you have a better solution?

Thanks,
- cong

From: Cong Ding <[email protected]>
Date: Thu, 24 Jan 2013 18:30:45 -0500
Subject: [PATCH] btrfs: fix potential null pointer dereference bug

The bug happens when rb_node == NULL. It causes variable node to be NULL and then the NULL pointer is dereferenced this line:

        BUG_ON((struct btrfs_root *)node->data != root);

So we check node before the dereference.

---
 fs/btrfs/relocation.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 17c306b..938b037 100644
@@ -1269,7 +1269,7 @@ static int __update_reloc_root(struct btrfs_root *root, int del)
 	}
 	spin_unlock(&rc->reloc_root_tree.lock);
 
-	BUG_ON((struct btrfs_root *)node->data != root);
+	BUG_ON(!node || (struct btrfs_root *)node->data != root);
 
 	if (!del) {
 		spin_lock(&rc->reloc_root_tree.lock);

1.7.10.4