[PATCH] btrfs: initialize offset early

Muhammad Usama Anjum posted 1 patch 4 years, 5 months ago
fs/btrfs/scrub.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH] btrfs: initialize offset early
Posted by Muhammad Usama Anjum 4 years, 5 months ago
Jump to out label can happen before offset is initialized. offset is
being used in code after out label. initialize offset early to cater
this case.

Fixes: 585f784357d8 ("btrfs: use scrub_simple_mirror() to handle RAID56 data stripe scrub")
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 fs/btrfs/scrub.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 26bbe93c3aa3c..3ace9766527ba 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -3530,7 +3530,7 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
 	u64 logic_end;
 	u64 physical_end;
 	u64 increment;	/* The logical increment after finishing one stripe */
-	u64 offset;	/* Offset inside the chunk */
+	u64 offset = 0;	/* Offset inside the chunk */
 	u64 stripe_logical;
 	u64 stripe_end;
 
@@ -3602,7 +3602,6 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
 	ASSERT(map->type & BTRFS_BLOCK_GROUP_RAID56_MASK);
 
 	physical = map->stripes[stripe_index].physical;
-	offset = 0;
 	nstripes = div64_u64(dev_extent_len, map->stripe_len);
 	get_raid56_logic_offset(physical, stripe_index, map, &offset, NULL);
 	increment = map->stripe_len * nr_data_stripes(map);
-- 
2.30.2

Re: [PATCH] btrfs: initialize offset early
Posted by Qu Wenruo 4 years, 5 months ago

On 2022/1/28 20:35, Muhammad Usama Anjum wrote:
> Jump to out label can happen before offset is initialized. offset is
> being used in code after out label. initialize offset early to cater
> this case.
>
> Fixes: 585f784357d8 ("btrfs: use scrub_simple_mirror() to handle RAID56 data stripe scrub")
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

Thanks for the fix.

Although the patch is only in misc-next, and due to another triggered
ASSERT(), the series will be reworked soon.

Thanks,
Qu

> ---
>   fs/btrfs/scrub.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 26bbe93c3aa3c..3ace9766527ba 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -3530,7 +3530,7 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
>   	u64 logic_end;
>   	u64 physical_end;
>   	u64 increment;	/* The logical increment after finishing one stripe */
> -	u64 offset;	/* Offset inside the chunk */
> +	u64 offset = 0;	/* Offset inside the chunk */
>   	u64 stripe_logical;
>   	u64 stripe_end;
>
> @@ -3602,7 +3602,6 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
>   	ASSERT(map->type & BTRFS_BLOCK_GROUP_RAID56_MASK);
>
>   	physical = map->stripes[stripe_index].physical;
> -	offset = 0;
>   	nstripes = div64_u64(dev_extent_len, map->stripe_len);
>   	get_raid56_logic_offset(physical, stripe_index, map, &offset, NULL);
>   	increment = map->stripe_len * nr_data_stripes(map);
Re: [PATCH] btrfs: initialize offset early
Posted by Dan Carpenter 4 years, 4 months ago
On Fri, Jan 28, 2022 at 05:35:58PM +0500, Muhammad Usama Anjum wrote:
> Jump to out label can happen before offset is initialized. offset is
> being used in code after out label. initialize offset early to cater
> this case.
> 
> Fixes: 585f784357d8 ("btrfs: use scrub_simple_mirror() to handle RAID56 data stripe scrub")
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>  fs/btrfs/scrub.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 26bbe93c3aa3c..3ace9766527ba 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -3530,7 +3530,7 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
>  	u64 logic_end;
>  	u64 physical_end;
            ^^^^^^^^^^^^

>  	u64 increment;	/* The logical increment after finishing one stripe */
> -	u64 offset;	/* Offset inside the chunk */
> +	u64 offset = 0;	/* Offset inside the chunk */
>  	u64 stripe_logical;
>  	u64 stripe_end;
>  

The same checker which complained about offset also complained about
physical_end.  Please don't post random half patches which aren't going
to fix the crash.

If you don't know how to fix it, then just report it to the original
author.  This one was already reported.  (I think.  Anyway, consider it
reported now.)

regards,
dan carpenter