linux-kernel - Re: [PATCH] btrfs: fix potential null pointer dereference bug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 24 Jan 2013 18:46:42 -0500
From:	Cong Ding <dinggnu@...il.com>
To:	Josef Bacik <jbacik@...ionio.com>
Cc:	Chris Mason <clmason@...ionio.com>,
	"linux-btrfs@...r.kernel.org" <linux-btrfs@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] btrfs: fix potential null pointer dereference bug

On Thu, Jan 24, 2013 at 10:34:20AM -0500, Josef Bacik wrote:
> On Sat, Jan 19, 2013 at 08:27:45AM -0700, Cong Ding wrote:
> > 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).
> > 
> > Signed-off-by: Cong Ding <dinggnu@...il.com>
> 
> I don't want to add more BUG_ON()'s, just return an error.
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 3a5b4df67dd177b7cbc61c555349fd7e87ef6b54 Mon Sep 17 00:00:00 2001
From: Cong Ding <dinggnu@...il.com>
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.

Signed-off-by: Cong Ding <dinggnu@...il.com>
---
 fs/btrfs/relocation.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 17c306b..938b037 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/