mbox series

[0/3] xfs: fix serious bugs in rmap key comparison

Message ID 160494582942.772693.12774142799511044233.stgit@magnolia (mailing list archive)
Headers show
Series xfs: fix serious bugs in rmap key comparison | expand

Message

Darrick J. Wong Nov. 9, 2020, 6:17 p.m. UTC
Hi all,

Last week I found some time to spend auditing the effectiveness of the
online repair prototype code, and discovered a serious bug in the rmap
code.  Each btree type provides four comparison predicates: one for
comparing a key against the current record; one for comparing two
arbitrary record keys, and one each for checking if a btree block's
records or keys are in the correct order.

Unfortunately, I encoded a major thinko into those last three functions.
The XFS_RMAP_OFF macro masks off the three namespace bits before we
perform a comparison, which means that key comparisons do not notice
differences between the unwritten, bmbt, or attr fork status.  On a
consistent filesystem this is not an issue because there can only ever
be overlapping rmap records for written inode data fork extents, which
is why we've not yet seen any problems in the field.

Fortunately, the last two functions are used by debugging asserts and
online scrub to check the contents of a btree block, so the severity of
the flaw there is not high.

Unfortunately, the flaw in _diff_two_keys is more severe, because it is
used for query_range requests.  Ranged queries are used by the regular
rmap handling code when reflink is enabled; and it is used in the rmap
btree validation routines of both xfs_scrub and xfs_repair.  As I
mentioned above, the flaw should not manifest on a *consistent*
filesystem, but for fuzzed (or corrupt) filesystems, this seriously
impacts our ability to detect problems.

The first two patches in this series fix two places where we pass the
wrong flags arguments to the rmap query functions (which didn't
previously cause lookup failures due to the broken code) and the third
patch fixes the comparison functions.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=rmap-fixes-5.10
---
 fs/xfs/libxfs/xfs_rmap.c       |    2 +-
 fs/xfs/libxfs/xfs_rmap_btree.c |   16 ++++++++--------
 fs/xfs/scrub/bmap.c            |    2 ++
 3 files changed, 11 insertions(+), 9 deletions(-)