[PATCH] btrfs: Fix wrong first_key parameter in replace_path

[PATCH] btrfs: Fix wrong first_key parameter in replace_path

[Qu Wenruo]

Commit 581c1760415c ("btrfs: Validate child tree block's level and first
key") introduced new @first_key parameter for read_tree_block(), however
caller in replace_path() is parasing wrong key to read_tree_block().

It should use parameter @first_key other than @key.

Normally it won't expose problem as @key is normally initialzied to the
same value of @first_key we expect.
However in relocation recovery case, @key can be set to (0, 0, 0), and
since no valid key in relocation tree can be (0, 0, 0), it will cause
read_tree_block() to return -EUCLEAN and interrupt relocation recovery.

Fix it by setting @first_key correctly.

Signed-off-by: Qu Wenruo <wqu@suse.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 00b7d3231821..b041b945a7ae 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -1841,7 +1841,7 @@ int replace_path(struct btrfs_trans_handle *trans,
 		old_bytenr = btrfs_node_blockptr(parent, slot);
 		blocksize = fs_info->nodesize;
 		old_ptr_gen = btrfs_node_ptr_generation(parent, slot);
-		btrfs_node_key_to_cpu(parent, &key, slot);
+		btrfs_node_key_to_cpu(parent, &first_key, slot);
 
 		if (level <= max_level) {
 			eb = path->nodes[level];
-- 
2.17.0

Re: [PATCH] btrfs: Fix wrong first_key parameter in replace_path

[David Sterba]

On Mon, Apr 23, 2018 at 05:32:04PM +0800, Qu Wenruo wrote:
> Commit 581c1760415c ("btrfs: Validate child tree block's level and first
> key") introduced new @first_key parameter for read_tree_block(), however
> caller in replace_path() is parasing wrong key to read_tree_block().
> 
> It should use parameter @first_key other than @key.
> 
> Normally it won't expose problem as @key is normally initialzied to the
> same value of @first_key we expect.
> However in relocation recovery case, @key can be set to (0, 0, 0), and
> since no valid key in relocation tree can be (0, 0, 0), it will cause
> read_tree_block() to return -EUCLEAN and interrupt relocation recovery.
> 
> Fix it by setting @first_key correctly.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Added to next, thanks.

Re: [PATCH] btrfs: Fix wrong first_key parameter in replace_path

[Nikolay Borisov]

On 23.04.2018 17:16, David Sterba wrote:
> On Mon, Apr 23, 2018 at 05:32:04PM +0800, Qu Wenruo wrote:
>> Commit 581c1760415c ("btrfs: Validate child tree block's level and first
>> key") introduced new @first_key parameter for read_tree_block(), however
>> caller in replace_path() is parasing wrong key to read_tree_block().
>>
>> It should use parameter @first_key other than @key.
>>
>> Normally it won't expose problem as @key is normally initialzied to the
>> same value of @first_key we expect.
>> However in relocation recovery case, @key can be set to (0, 0, 0), and
>> since no valid key in relocation tree can be (0, 0, 0), it will cause
>> read_tree_block() to return -EUCLEAN and interrupt relocation recovery.
>>
>> Fix it by setting @first_key correctly.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> Added to next, thanks.

This is actually -rc2 material, right?
> --
> 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: Fix wrong first_key parameter in replace_path

[David Sterba]

On Mon, Apr 23, 2018 at 05:20:06PM +0300, Nikolay Borisov wrote:
> On 23.04.2018 17:16, David Sterba wrote:
> > On Mon, Apr 23, 2018 at 05:32:04PM +0800, Qu Wenruo wrote:
> >> Commit 581c1760415c ("btrfs: Validate child tree block's level and first
> >> key") introduced new @first_key parameter for read_tree_block(), however
> >> caller in replace_path() is parasing wrong key to read_tree_block().
> >>
> >> It should use parameter @first_key other than @key.
> >>
> >> Normally it won't expose problem as @key is normally initialzied to the
> >> same value of @first_key we expect.
> >> However in relocation recovery case, @key can be set to (0, 0, 0), and
> >> since no valid key in relocation tree can be (0, 0, 0), it will cause
> >> read_tree_block() to return -EUCLEAN and interrupt relocation recovery.
> >>
> >> Fix it by setting @first_key correctly.
> >>
> >> Signed-off-by: Qu Wenruo <wqu@suse.com>
> > 
> > Added to next, thanks.
> 
> This is actually -rc2 material, right?

Yes, adding to next is the 1st stage as we want to start testing. What
will end up in the next rc pull is determined later if everything is
fine. For some patches it's obvious that they're fixing a regression,
the others are clear fixes that still qualify for rc. I can be more
specific about the target branch, but this should be transparent to
developers anyway once the patch is in the pool.