[PATCH] btrfs-progs: Don't return any fs_info that contain NULL tree_root or fs_root.

[PATCH] btrfs-progs: Don't return any fs_info that contain NULL tree_root or fs_root.

[Qu Wenruo]

[BUG]
btrfsck will segfault if it fails to open the fs tree or tree root.

[REPRODUCER]
Execute btrfsck on a highly damaged btrfs image.
fsfuzz can be used to make a junk btrfs image.

[REASON]
Current open_ctree() in btrfs-progs support OPEN_CTREE_PARTIAL flag to
allow return fs_info even some of the trees is missing.

However it is too loose and even allows fs_info containing no tree to be
returned.

And when it happens, fs_info->fs_root is NULL,
close_ctree(fs_info->fs_root) will cause the access to NULL pointer and
segfault.

[FIX]
This patch will add checks for fs_info->tree_root and fs_info->fs_root
before return fs_info.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 disk-io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/disk-io.c b/disk-io.c
index 26a532e..21a3083 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1134,7 +1134,8 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
 	return fs_info;
 
 out_failed:
-	if (flags & OPEN_CTREE_PARTIAL)
+	if (flags & OPEN_CTREE_PARTIAL &&
+	    fs_info->tree_root && fs_info->fs_root)
 		return fs_info;
 out_chunk:
 	btrfs_release_all_roots(fs_info);
-- 
2.1.1

Re: [PATCH] btrfs-progs: Don't return any fs_info that contain NULL tree_root or fs_root.

[David Sterba]

On Tue, Sep 30, 2014 at 10:39:22AM +0800, Qu Wenruo wrote:
> --- a/disk-io.c
> +++ b/disk-io.c
> @@ -1134,7 +1134,8 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
>  	return fs_info;
>  
>  out_failed:
> -	if (flags & OPEN_CTREE_PARTIAL)
> +	if (flags & OPEN_CTREE_PARTIAL &&
> +	    fs_info->tree_root && fs_info->fs_root)
>  		return fs_info;

I see a conflict with a pending patch
https://patchwork.kernel.org/patch/4254631/

that removes the check completely but fixes the crash in another way. I
like Wang's patch because it keeps the logic about partial open inside
btrfs_setup_all_roots(). Please test if it fixes the crash with the
corrupted image you have. Thanks.

>  out_chunk:
>  	btrfs_release_all_roots(fs_info);

Re: [PATCH] btrfs-progs: Don't return any fs_info that contain NULL tree_root or fs_root.

[Zach Brown]

> btrfs_setup_all_roots(). Please test if it fixes the crash with the
> corrupted image you have. Thanks.

Perhaps with a test in xfstests.

- z

Re: [PATCH] btrfs-progs: Don't return any fs_info that contain NULL tree_root or fs_root.

[Qu Wenruo]

Sorry for the late reply.

Wang's patch fixed all the NULL tree root related bugs.
So my patches are not needed and please ignore them.

I'll also reply to my patches to mark them unneeded.

Thanks,
Qu
-------- Original Message --------
Subject: Re: [PATCH] btrfs-progs: Don't return any fs_info that contain 
NULL tree_root or fs_root.
From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>
Date: 2014年09月30日 19:35
> On Tue, Sep 30, 2014 at 10:39:22AM +0800, Qu Wenruo wrote:
>> --- a/disk-io.c
>> +++ b/disk-io.c
>> @@ -1134,7 +1134,8 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
>>   	return fs_info;
>>   
>>   out_failed:
>> -	if (flags & OPEN_CTREE_PARTIAL)
>> +	if (flags & OPEN_CTREE_PARTIAL &&
>> +	    fs_info->tree_root && fs_info->fs_root)
>>   		return fs_info;
> I see a conflict with a pending patch
> https://patchwork.kernel.org/patch/4254631/
>
> that removes the check completely but fixes the crash in another way. I
> like Wang's patch because it keeps the logic about partial open inside
> btrfs_setup_all_roots(). Please test if it fixes the crash with the
> corrupted image you have. Thanks.
>
>>   out_chunk:
>>   	btrfs_release_all_roots(fs_info);
> --
> 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

Re: [PATCH] btrfs-progs: Don't return any fs_info that contain NULL tree_root or fs_root.

[Qu Wenruo]

Please ignore this patch since Wang's patch has already fixed them.

https://patchwork.kernel.org/patch/4254631/

Thanks,
Qu
-------- Original Message --------
Subject: [PATCH] btrfs-progs: Don't return any fs_info that contain NULL 
tree_root or fs_root.
From: Qu Wenruo <quwenruo@cn.fujitsu.com>
To: <linux-btrfs@vger.kernel.org>
Date: 2014年09月30日 10:39
> [BUG]
> btrfsck will segfault if it fails to open the fs tree or tree root.
>
> [REPRODUCER]
> Execute btrfsck on a highly damaged btrfs image.
> fsfuzz can be used to make a junk btrfs image.
>
> [REASON]
> Current open_ctree() in btrfs-progs support OPEN_CTREE_PARTIAL flag to
> allow return fs_info even some of the trees is missing.
>
> However it is too loose and even allows fs_info containing no tree to be
> returned.
>
> And when it happens, fs_info->fs_root is NULL,
> close_ctree(fs_info->fs_root) will cause the access to NULL pointer and
> segfault.
>
> [FIX]
> This patch will add checks for fs_info->tree_root and fs_info->fs_root
> before return fs_info.
>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
>   disk-io.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/disk-io.c b/disk-io.c
> index 26a532e..21a3083 100644
> --- a/disk-io.c
> +++ b/disk-io.c
> @@ -1134,7 +1134,8 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
>   	return fs_info;
>   
>   out_failed:
> -	if (flags & OPEN_CTREE_PARTIAL)
> +	if (flags & OPEN_CTREE_PARTIAL &&
> +	    fs_info->tree_root && fs_info->fs_root)
>   		return fs_info;
>   out_chunk:
>   	btrfs_release_all_roots(fs_info);