xfs
[Top] [All Lists]

Re: [PATCH 1/2] xfsdump: avoid segfault in partial_reg() in error case

To: xfs@xxxxxxxxxxx
Subject: Re: [PATCH 1/2] xfsdump: avoid segfault in partial_reg() in error case
From: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
Date: Thu, 10 Oct 2013 11:17:11 -0300
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <525481C2.6040100@xxxxxxxxxxx>
Mail-followup-to: xfs@xxxxxxxxxxx
References: <525480A6.3000202@xxxxxxxxxx> <525481C2.6040100@xxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
Looks good to me

Reviewed-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>

On Tue, Oct 08, 2013 at 05:05:54PM -0500, Eric Sandeen wrote:
> If we go down the "/* Should never get here. */" path
> in partial_reg(), we issue a warning but then continue
> with the function.  This calls pi_unlock() twice,
> but worse, uses a null isptr:
> 
>         if ( ! isptr ) {
> ... isptr is never set if we get to ...
>                 /* Should never get here. */
>                 pi_unlock();
> ...
>         }
> ...
>         /* Update this drive's entry */
>         bsptr = &isptr->is_bs[d_index];
>         if (bsptr->endoffset == 0) {
> 
> From all appearances, because we unlock on that "never get
> here" path, it should just be returning after printing the
> warning.  So add that, and we avoid the segfault.
> 
> The previous fix to partial_reg() should prevent us from
> hitting this in the first place.
> 
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
> 
> diff --git a/restore/content.c b/restore/content.c
> index 54d933c..cc49336 100644
> --- a/restore/content.c
> +++ b/restore/content.c
> @@ -9007,6 +9007,7 @@ partial_reg( ix_t d_index,
>  #ifdef DEBUGPARTIALS
>               dump_partials();
>  #endif
> +             return;
>       }
>  
>  found:
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

<Prev in Thread] Current Thread [Next in Thread>