Btrfs devel - Jan 2013 - [PATCH] Btrfs: do not merge logged extents if we've removed them from the tree

You can run into this problem where if somebody is fsyncing and writing out
the existing extents you will have removed the extent map from the em tree,
but it''s still valid for the current fsync so we go ahead and write it.
The
problem is we unconditionally try to merge it back into the em tree, but if
we''ve removed it from the em tree that will cause problems.  So fix
this by
only trying to merge it in if it is still a part of the tree.  Thanks,

Signed-off-by: Josef Bacik <jbacik@fusionio.com>
---
 fs/btrfs/extent_map.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
index ed88f5e..9759911 100644
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -289,7 +289,8 @@ out:
 void clear_em_logging(struct extent_map_tree *tree, struct extent_map *em)
 {
 	clear_bit(EXTENT_FLAG_LOGGING, &em->flags);
-	try_merge_map(tree, em);
+	if (em->in_tree)
+		try_merge_map(tree, em);
 }
 
 /**
-- 
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

> problem is we unconditionally try to merge it back into the em tree, but if
> we''ve removed it from the em tree that will cause problems.  So
fix this by
What problems?  If I were staring at the symptoms of this bug how would
I find that this commit fixes it?

- z
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Mon, Jan 28, 2013 at 11:52:26AM -0700, Zach Brown
wrote:
> > problem is we unconditionally try to merge it back into the em tree,
but if
> > we''ve removed it from the em tree that will cause problems. 
So fix this by
> 
> What problems?  If I were staring at the symptoms of this bug how would
> I find that this commit fixes it?
> 
I''ll fix the commit message, we just add something to the tree that
doesn''t have
a ref so it''s a basic use after free sort of scenario. Thanks,

Josef
> - z
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

> I''ll fix the commit message, we just add something to the tree
that doesn''t have
> a ref so it''s a basic use after free sort of scenario. Thanks,
Thanks, complete commit messages help us old farts keep up.

- z
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Mon, Jan 28, 2013 at 09:52:56AM -0500, Josef Bacik
wrote:
> You can run into this problem where if somebody is fsyncing and writing out
> the existing extents you will have removed the extent map from the em tree,
> but it''s still valid for the current fsync so we go ahead and
write it.  The
> problem is we unconditionally try to merge it back into the em tree, but if
> we''ve removed it from the em tree that will cause problems.  So
fix this by
> only trying to merge it in if it is still a part of the tree.  Thanks,
> 
Reviewed-by: Liu Bo <bo.li.liu@oracle.com>
> Signed-off-by: Josef Bacik <jbacik@fusionio.com>
> ---
>  fs/btrfs/extent_map.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
> index ed88f5e..9759911 100644
> --- a/fs/btrfs/extent_map.c
> +++ b/fs/btrfs/extent_map.c
> @@ -289,7 +289,8 @@ out:
>  void clear_em_logging(struct extent_map_tree *tree, struct extent_map *em)
>  {
>  	clear_bit(EXTENT_FLAG_LOGGING, &em->flags);
> -	try_merge_map(tree, em);
> +	if (em->in_tree)
> +		try_merge_map(tree, em);
>  }
>  
>  /**
> -- 
> 1.7.7.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe
linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html