Commit 837514f7 authored by Dave Chinner's avatar Dave Chinner Committed by Darrick J. Wong

xfs: fix overflow in xfs_attr3_leaf_verify

generic/070 on 64k block size filesystems is failing with a verifier
corruption on writeback or an attribute leaf block:

[   94.973083] XFS (pmem0): Metadata corruption detected at xfs_attr3_leaf_verify+0x246/0x260, xfs_attr3_leaf block 0x811480
[   94.975623] XFS (pmem0): Unmount and run xfs_repair
[   94.976720] XFS (pmem0): First 128 bytes of corrupted metadata buffer:
[   94.978270] 000000004b2e7b45: 00 00 00 00 00 00 00 00 3b ee 00 00 00 00 00 00  ........;.......
[   94.980268] 000000006b1db90b: 00 00 00 00 00 81 14 80 00 00 00 00 00 00 00 00  ................
[   94.982251] 00000000433f2407: 22 7b 5c 82 2d 5c 47 4c bb 31 1c 37 fa a9 ce d6  "{\.-\GL.1.7....
[   94.984157] 0000000010dc7dfb: 00 00 00 00 00 81 04 8a 00 0a 18 e8 dd 94 01 00  ................
[   94.986215] 00000000d5a19229: 00 a0 dc f4 fe 98 01 68 f0 d8 07 e0 00 00 00 00  .......h........
[   94.988171] 00000000521df36c: 0c 2d 32 e2 fe 20 01 00 0c 2d 58 65 fe 0c 01 00  .-2.. ...-Xe....
[   94.990162] 000000008477ae06: 0c 2d 5b 66 fe 8c 01 00 0c 2d 71 35 fe 7c 01 00  .-[f.....-q5.|..
[   94.992139] 00000000a4a6bca6: 0c 2d 72 37 fc d4 01 00 0c 2d d8 b8 f0 90 01 00  .-r7.....-......
[   94.994789] XFS (pmem0): xfs_do_force_shutdown(0x8) called from line 1453 of file fs/xfs/xfs_buf.c. Return address = ffffffff815365f3

This is failing this check:

                end = ichdr.freemap[i].base + ichdr.freemap[i].size;
                if (end < ichdr.freemap[i].base)
>>>>>                   return __this_address;
                if (end > mp->m_attr_geo->blksize)
                        return __this_address;

And from the buffer output above, the freemap array is:

	freemap[0].base = 0x00a0
	freemap[0].size = 0xdcf4	end = 0xdd94
	freemap[1].base = 0xfe98
	freemap[1].size = 0x0168	end = 0x10000
	freemap[2].base = 0xf0d8
	freemap[2].size = 0x07e0	end = 0xf8b8

These all look valid - the block size is 0x10000 and so from the
last check in the above verifier fragment we know that the end
of freemap[1] is valid. The problem is that end is declared as:

	uint16_t	end;

And (uint16_t)0x10000 = 0. So we have a verifier bug here, not a
corruption. Fix the verifier to use uint32_t types for the check and
hence avoid the overflow.

Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=201577Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
parent bdec055b
...@@ -243,7 +243,7 @@ xfs_attr3_leaf_verify( ...@@ -243,7 +243,7 @@ xfs_attr3_leaf_verify(
struct xfs_mount *mp = bp->b_target->bt_mount; struct xfs_mount *mp = bp->b_target->bt_mount;
struct xfs_attr_leafblock *leaf = bp->b_addr; struct xfs_attr_leafblock *leaf = bp->b_addr;
struct xfs_attr_leaf_entry *entries; struct xfs_attr_leaf_entry *entries;
uint16_t end; uint32_t end; /* must be 32bit - see below */
int i; int i;
xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &ichdr, leaf); xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &ichdr, leaf);
...@@ -293,6 +293,11 @@ xfs_attr3_leaf_verify( ...@@ -293,6 +293,11 @@ xfs_attr3_leaf_verify(
/* /*
* Quickly check the freemap information. Attribute data has to be * Quickly check the freemap information. Attribute data has to be
* aligned to 4-byte boundaries, and likewise for the free space. * aligned to 4-byte boundaries, and likewise for the free space.
*
* Note that for 64k block size filesystems, the freemap entries cannot
* overflow as they are only be16 fields. However, when checking end
* pointer of the freemap, we have to be careful to detect overflows and
* so use uint32_t for those checks.
*/ */
for (i = 0; i < XFS_ATTR_LEAF_MAPSIZE; i++) { for (i = 0; i < XFS_ATTR_LEAF_MAPSIZE; i++) {
if (ichdr.freemap[i].base > mp->m_attr_geo->blksize) if (ichdr.freemap[i].base > mp->m_attr_geo->blksize)
...@@ -303,7 +308,9 @@ xfs_attr3_leaf_verify( ...@@ -303,7 +308,9 @@ xfs_attr3_leaf_verify(
return __this_address; return __this_address;
if (ichdr.freemap[i].size & 0x3) if (ichdr.freemap[i].size & 0x3)
return __this_address; return __this_address;
end = ichdr.freemap[i].base + ichdr.freemap[i].size;
/* be care of 16 bit overflows here */
end = (uint32_t)ichdr.freemap[i].base + ichdr.freemap[i].size;
if (end < ichdr.freemap[i].base) if (end < ichdr.freemap[i].base)
return __this_address; return __this_address;
if (end > mp->m_attr_geo->blksize) if (end > mp->m_attr_geo->blksize)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment