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

[Brian Foster <bfoster@redhat.com>]

On Fri, Feb 13, 2015 at 10:14:17AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> 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@gmail.com>
> Tested-by: Michael L. Semon <mlsemon35@gmail.com>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

Looks fine to me:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  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@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs