xfs
[Top] [All Lists]

Re: [PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Mon, 16 Feb 2015 08:41:21 -0500
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1423782857-11800-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1423782857-11800-1-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
On Fri, Feb 13, 2015 at 10:14:17AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> Test generic/224 is failing with a corruption being detected on one
> of Michael's test boxes.  Debug that Michael added is indicating
> that the minleft trimming is resulting in an underflow:
> 
> .....
>  before fixup:              rlen          1  args->len          0
>  after xfs_alloc_fix_len  : rlen          1  args->len          1
>  before goto out_nominleft: rlen          1  args->len          0
>  before fixup:              rlen          1  args->len          0
>  after xfs_alloc_fix_len  : rlen          1  args->len          1
>  after fixup:               rlen          1  args->len          1
>  before fixup:              rlen          1  args->len          0
>  after xfs_alloc_fix_len  : rlen          1  args->len          1
>  after fixup:               rlen 4294967295  args->len 4294967295
>  XFS: Assertion failed: fs_is_ok, file: fs/xfs/libxfs/xfs_alloc.c, line: 1424
> 
> The "goto out_nominleft:" indicates that we are getting close to
> ENOSPC in the AG, and a couple of allocations later we underflow
> and the corruption check fires in xfs_alloc_ag_vextent_size().
> 
> The issue is that the extent length fixups comaprisons are done
> with variables of xfs_extlen_t types. These are unsigned so an
> underflow looks like a really big value and hence is not detected
> as being smaller than the minimum length allowed for the extent.
> Hence the corruption check fires as it is noticing that the returned
> length is longer than the original extent length passed in.
> 
> This can be easily fixed by ensuring we do the underflow test on
> signed values, the same way xfs_alloc_fix_len() prevents underflow.
> So we realise in future that these casts prevent underflows from
> going undetected, add comments to the code indicating this.
> 
> Reported-by: Michael L. Semon <mlsemon35@xxxxxxxxx>
> Tested-by: Michael L. Semon <mlsemon35@xxxxxxxxx>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---

Looks fine to me:

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

>  fs/xfs/libxfs/xfs_alloc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 710554c..41e3630 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -260,6 +260,7 @@ xfs_alloc_fix_len(
>               rlen = rlen - (k - args->mod);
>       else
>               rlen = rlen - args->prod + (args->mod - k);
> +     /* casts to (int) catch length underflows */
>       if ((int)rlen < (int)args->minlen)
>               return;
>       ASSERT(rlen >= args->minlen && rlen <= args->maxlen);
> @@ -286,7 +287,8 @@ xfs_alloc_fix_minleft(
>       if (diff >= 0)
>               return 1;
>       args->len += diff;              /* shrink the allocated space */
> -     if (args->len >= args->minlen)
> +     /* casts to (int) catch length underflows */
> +     if ((int)args->len >= (int)args->minlen)
>               return 1;
>       args->agbno = NULLAGBLOCK;
>       return 0;
> -- 
> 2.0.0
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

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