]> git.ipfire.org Git - thirdparty/xfsprogs-dev.git/commit - libfrog/crc32cselftest.h
misc: test the dir/attr hash before formatting or repairing fs
authorDarrick J. Wong <djwong@kernel.org>
Thu, 6 Apr 2023 00:08:02 +0000 (17:08 -0700)
committerCarlos Maiolino <cem@kernel.org>
Fri, 21 Apr 2023 09:15:45 +0000 (11:15 +0200)
commitb9d29568e40dbead8484a4dba2089ff54df7caaf
tree6519a2ae9419379c36a48be22cda7316dd147a9b
parentd8a19f2986c0f65c5ed02110192f3fd5f86e2b32
misc: test the dir/attr hash before formatting or repairing fs

Back in the 6.2-rc1 days, Eric Whitney reported a fstests regression in
ext4 against generic/454.  The cause of this test failure was the
unfortunate combination of setting an xattr name containing UTF8 encoded
emoji, an xattr hash function that accepted a char pointer with no
explicit signedness, signed type extension of those chars to an int, and
the 6.2 build tools maintainers deciding to mandate -funsigned-char
across the board.  As a result, the ondisk extended attribute structure
written out by 6.1 and 6.2 were not the same.

This discrepancy, in fact, had been noticeable if a filesystem with such
an xattr were moved between any two architectures that don't employ the
same signedness of a raw "char" declaration.  The only reason anyone
noticed is that x86 gcc defaults to signed, and no such -funsigned-char
update was made to e2fsprogs, so e2fsck immediately started reporting
data corruption.

After a day and a half of discussing how to handle this use case (xattrs
with bit 7 set anywhere in the name) without breaking existing users,
Linus merged his own patch and didn't tell the mailing list.  None of
the developers noticed until AUTOSEL made an announcement.

In the end, this problem could have been detected much earlier if there
had been any useful tests of hash function(s) in use inside ext4 to make
sure that they always produce the same outputs given the same inputs.

The XFS dirent/xattr name hash takes a uint8_t*, so I don't think it's
vulnerable to this problem.  However, let's avoid all this drama by
adding our own self test to check that the da hash produces the same
outputs for a static pile of inputs on various platforms.  This
corresponds to the similar patch for the kernel.

Link: https://lore.kernel.org/linux-ext4/Y8bpkm3jA3bDm3eL@debian-BULLSEYE-live-builder-AMD64/
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
libfrog/Makefile
libfrog/crc32cselftest.h
libfrog/dahashselftest.h [new file with mode: 0644]
mkfs/xfs_mkfs.c
repair/init.c