xfs
[Top] [All Lists]

[PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC

To: xfs@xxxxxxxxxxx
Subject: [PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 13 Feb 2015 10:14:17 +1100
Delivered-to: xfs@xxxxxxxxxxx
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>
---
 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

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