Re: [PATCH] NFS support for btrfs - v2

[David Woodhouse <dwmw2@infradead.org>]

On Tue, 2008-08-19 at 15:49 +0100, David Woodhouse wrote:
> On Tue, 2008-08-19 at 07:54 -0400, Chris Mason wrote:
> > 
> > > What if the parent inode actually _is_ inode #0xffffffffffffffff? Can
> > > that happen? In that case it would return zero, and I shouldn't subtract
> > > 1 from the slot number -- I've actually found what I'm looking for?
> > > 
> > 
> > The max inode will be 2^64 - 1
> 
> Which is what we're searching for -- so it's _possible_, albeit
> vanishingly unlikely, that btrfs_search_slot() will actually return
> zero, having found precisely what we wanted?
> 
> And in that case, path->slots[0] being zero is fine. And we shouldn't be
> subtracting one from it to find the slot we want?

Subject: [PATCH] Clean up btrfs_get_parent() a little more, fix a free-after-free bug

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 export.c |   33 +++++++++++++++++++--------------
 1 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/export.c b/export.c
index 36cbc68..5c75cbd 100644
--- a/export.c
+++ b/export.c
@@ -165,23 +165,32 @@ static struct dentry *btrfs_get_parent(struct dentry *child)
 	key.offset = (u64)-1;
 
 	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-	leaf = path->nodes[0];
-	slot = path->slots[0];
-	if (ret < 0 || slot == 0) {
+	if (ret < 0) {
+		/* Error */
 		btrfs_free_path(path);
-		goto out;
+		return ERR_PTR(ret);
+	}
+	if (ret) {
+		leaf = path->nodes[0];
+		slot = path->slots[0];
+		/* btrfs_search_slot() returns the slot where we'd want to
+		   insert a backref for parent inode #0xFFFFFFFFFFFFFFFF.
+		   The _real_ backref, telling us what the parent inode
+		   _actually_ is, will be in the slot _before_ the one
+		   that btrfs_search_slot() returns. */
+		if (!slot) {
+			/* Unless there is _no_ key in the tree before... */
+			btrfs_free_path(path);
+			return ERR_PTR(-EIO);
+		}
+		slot--;
 	}
-	/* btrfs_search_slot() returns the slot where we'd want to insert
-	   an INODE_REF_KEY for parent inode #0xFFFFFFFFFFFFFFFF. The _real_
-	   one, telling us what the parent inode _actually_ is, will be in
-	   the slot _before_ the one that btrfs_search_slot() returns. */
-	slot--;
 
 	btrfs_item_key_to_cpu(leaf, &key, slot);
 	btrfs_free_path(path);
 
 	if (key.objectid != dir->i_ino || key.type != BTRFS_INODE_REF_KEY)
-		goto out;
+		return ERR_PTR(-EINVAL);
 
 	objectid = key.offset;
 
@@ -201,10 +210,6 @@ static struct dentry *btrfs_get_parent(struct dentry *child)
 		parent = ERR_PTR(-ENOMEM);
 
 	return parent;
-
-out:
-	btrfs_free_path(path);
-	return ERR_PTR(-EINVAL);
 }
 
 const struct export_operations btrfs_export_ops = {
-- 
1.5.5.1


-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation