From patchwork Thu Jan 24 23:46:42 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cong Ding X-Patchwork-Id: 2037401 Return-Path: X-Original-To: patchwork-linux-btrfs@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 8CB6F3FDBC for ; Thu, 24 Jan 2013 23:47:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755523Ab3AXXqt (ORCPT ); Thu, 24 Jan 2013 18:46:49 -0500 Received: from mail-ea0-f179.google.com ([209.85.215.179]:43155 "EHLO mail-ea0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754932Ab3AXXqr (ORCPT ); Thu, 24 Jan 2013 18:46:47 -0500 Received: by mail-ea0-f179.google.com with SMTP id d12so3292306eaa.10 for ; Thu, 24 Jan 2013 15:46:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=EV1t9XQ5+vM+jRP8SCKQ5H9M2SuDlo5uciOCfqN4lI4=; b=lPOgIsrihQ7taa+E6cDbJ7kceOK1ysGyAaSvfxAMgljUKk+uxq8DXVlJQqkP9IRppX wAGGMy5fvB2ld5s+rAyhjjOHfF4TnCfu6bclB+8bdweXdKjtjpG9j2E2bdLV6Z1VWHi9 z/Cw9rPU0ELcuau38ZtBznIHOWf368zW6DJ1QhQAOmkpn6dtKvj0i7QyD0HvgUPQPmy5 XVZtSmiEMOr2Abh41kO1XP3jwFbFxDkEZ9OQG6bYU2ZycD3PpJxkBS5VrxWf58d7KMHJ 88iTUBjVmN1h+cy6FktzNwQTp+rf6eDsug5HbtDO8Q1tvAMzzTYvzBJ712Sq18x7zHuH QJJA== X-Received: by 10.14.194.195 with SMTP id m43mr11257533een.44.1359071206557; Thu, 24 Jan 2013 15:46:46 -0800 (PST) Received: from gmail.com (77.47.90.154.dynamic.cablesurf.de. [77.47.90.154]) by mx.google.com with ESMTPS id 3sm2931963eej.6.2013.01.24.15.46.44 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 24 Jan 2013 15:46:45 -0800 (PST) Date: Thu, 24 Jan 2013 18:46:42 -0500 From: Cong Ding To: Josef Bacik Cc: Chris Mason , "linux-btrfs@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] btrfs: fix potential null pointer dereference bug Message-ID: <20130124234642.GA29022@gmail.com> References: <1358609265-347-1-git-send-email-dinggnu@gmail.com> <20130124153420.GC2349@localhost.localdomain> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20130124153420.GC2349@localhost.localdomain> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org 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 > > 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 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 --- 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);