Skip to content

Commit 6f13c1d

Browse files
author
Darrick J. Wong
committed
xfs: delete attr leaf freemap entries when empty
Back in commit 2a2b593 ("xfs: fix attr leaf header freemap.size underflow"), Brian Foster observed that it's possible for a small freemap at the end of the end of the xattr entries array to experience a size underflow when subtracting the space consumed by an expansion of the entries array. There are only three freemap entries, which means that it is not a complete index of all free space in the leaf block. This code can leave behind a zero-length freemap entry with a nonzero base. Subsequent setxattr operations can increase the base up to the point that it overlaps with another freemap entry. This isn't in and of itself a problem because the code in _leaf_add that finds free space ignores any freemap entry with zero size. However, there's another bug in the freemap update code in _leaf_add, which is that it fails to update a freemap entry that begins midway through the xattr entry that was just appended to the array. That can result in the freemap containing two entries with the same base but different sizes (0 for the "pushed-up" entry, nonzero for the entry that's actually tracking free space). A subsequent _leaf_add can then allocate xattr namevalue entries on top of the entries array, leading to data loss. But fixing that is for later. For now, eliminate the possibility of confusion by zeroing out the base of any freemap entry that has zero size. Because the freemap is not intended to be a complete index of free space, a subsequent failure to find any free space for a new xattr will trigger block compaction, which regenerates the freemap. It looks like this bug has been in the codebase for quite a long time. Cc: <stable@vger.kernel.org> # v2.6.12 Fixes: 1da177e ("Linux-2.6.12-rc2") Signed-off-by: "Darrick J. Wong" <djwong@kernel.org> Reviewed-by: Christoph Hellwig <hch@lst.de>
1 parent a1ca658 commit 6f13c1d

1 file changed

Lines changed: 13 additions & 0 deletions

File tree

fs/xfs/libxfs/xfs_attr_leaf.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1580,6 +1580,19 @@ xfs_attr3_leaf_add_work(
15801580
min_t(uint16_t, ichdr->freemap[i].size,
15811581
sizeof(xfs_attr_leaf_entry_t));
15821582
}
1583+
1584+
/*
1585+
* Don't leave zero-length freemaps with nonzero base lying
1586+
* around, because we don't want the code in _remove that
1587+
* matches on base address to get confused and create
1588+
* overlapping freemaps. If we end up with no freemap entries
1589+
* then the next _add will compact the leaf block and
1590+
* regenerate the freemaps.
1591+
*/
1592+
if (ichdr->freemap[i].size == 0 && ichdr->freemap[i].base > 0) {
1593+
ichdr->freemap[i].base = 0;
1594+
ichdr->holes = 1;
1595+
}
15831596
}
15841597
ichdr->usedbytes += xfs_attr_leaf_entsize(leaf, args->index);
15851598
}

0 commit comments

Comments
 (0)