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

Brian Foster
2015-02-16T13:41:32Z

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