libext2fs: correctly subtract xattr blocks on bigalloc filesystems

From: Eric Biggers <[email protected]>

ext2fs_inode_data_blocks2() calculates an inode's data block count by subtracting the external xattr block, if any, from the total blocks. But on bigalloc filesystems, the xattr "block" is actually a whole cluster, so ext2fs_inode_data_blocks2() would return a too-large value.

It seems this could have caused several different problems, but the one I encountered was that xfstest generic/399 failed in the "bigalloc" config because e2fsck incorrectly considered a symlink on the filesystem to be corrupted at the end of the test. This happened because e2fsck incorrectly calculated a nonzero data block count for a "fast" symlink with an external xattr block and therefore treated it as a "slow" symlink, which failed validation.

Fix this by updating ext2fs_inode_data_blocks2() to subtract the cluster size rather than the block size.

Signed-off-by Eric Biggers <[email protected]>

---

lib/ext2fs/blknum.c                          |   2 +-
tests/f_bigalloc_symlink_with_xattr/expect.1 |   7 +++++++
tests/f_bigalloc_symlink_with_xattr/image.gz | Bin 0 -> 663 bytes
tests/f_bigalloc_symlink_with_xattr/name     |   1 +
tests/f_bigalloc_symlink_with_xattr/script   |   2 ++
5 files changed, 11 insertions(+), 1 deletion(-)

create mode 100644 tests/f_bigalloc_symlink_with_xattr/expect.1 create mode 100644 tests/f_bigalloc_symlink_with_xattr/image.gz create mode 100644 tests/f_bigalloc_symlink_with_xattr/name create mode 100644 tests/f_bigalloc_symlink_with_xattr/script

lib/ext2fs/blknum.c +1 -1

index ac808490..aa0e9118 100644
@@ -71,7 +71,7 @@ blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs,
 	return (inode->i_blocks |
 		(ext2fs_has_feature_huge_file(fs->super) ?
 		 (__u64) inode->osd2.linux2.l_i_blocks_hi << 32 : 0)) -
-		(inode->i_file_acl ? fs->blocksize >> 9 : 0);
+		(inode->i_file_acl ? EXT2_CLUSTER_SIZE(fs->super) >> 9 : 0);
 }
 
 /*

tests/f_bigalloc_symlink_with_xattr/expect.1 new file +7 -0

new file mode 100644
index 00000000..0e6c199f
@@ -0,0 +1,7 @@
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+test_filesys: 12/16 files (0.0% non-contiguous), 112/128 blocks
+Exit status is 0

tests/f_bigalloc_symlink_with_xattr/image.gz new file binary +0 -0

new file mode 100644
index 0000000000000000000000000000000000000000..cbd125dc25a59e6795363e4228c5c5a24e49eeb2
GIT binary patch

tests/f_bigalloc_symlink_with_xattr/name new file +1 -0

new file mode 100644
index 00000000..f612796f
@@ -0,0 +1 @@
+fast symlink with external xattr block on bigalloc filesystem

tests/f_bigalloc_symlink_with_xattr/script new file +2 -1

new file mode 100644
index 00000000..8ab2b9c6
@@ -0,0 +1,2 @@
+ONE_PASS_ONLY="true"
+. $cmd_dir/run_e2fsck
--

2.13.0

Quoted 8 lines · Eric Biggers

From: Eric Biggers <[email protected]>

ext2fs_inode_data_blocks2() calculates an inode's data block count by subtracting the external xattr block, if any, from the total blocks. But on bigalloc filesystems, the xattr "block" is actually a whole cluster, so ext2fs_inode_data_blocks2() would return a too-large value.

Quoted 7 lines

It seems this could have caused several different problems, but the one I encountered was that xfstest generic/399 failed in the "bigalloc" config because e2fsck incorrectly considered a symlink on the filesystem to be corrupted at the end of the test. This happened because e2fsck incorrectly calculated a nonzero data block count for a "fast" symlink with an external xattr block and therefore treated it as a "slow" symlink, which failed validation.

I thought we changed this to detect "fast" inodes by i_size < 60 rather than using the blocks count, because the blocks count was (and apparently continues to be) unreliable for determining fast vs. slow symlinks.

However, ext4_inode_is_fast_symlink() still checks blocks count. In "[PATCH] ext4: fix reading new encrypted symlinks on no-journal filesystems" we discussed whether this was safe, and it appears to be OK from my analysis.

We just continue to hit problems when extrapolating various blocks counts to detect fast symlinks rather than just using the same mechanism we use at creation time, which is "len > EXT4_N_BLOCKS * 4".

Cheers, Andreas

From: Eric Biggers <[email protected]>

ext2fs_inode_data_blocks2() calculates an inode's data block count by subtracting the external xattr block, if any, from the total blocks. But on bigalloc filesystems, the xattr "block" is actually a whole cluster, so ext2fs_inode_data_blocks2() would return a too-large value.

It seems this could have caused several different problems, but the one I encountered was that xfstest generic/399 failed in the "bigalloc" config because e2fsck incorrectly considered a symlink on the filesystem to be corrupted at the end of the test. This happened because e2fsck incorrectly calculated a nonzero data block count for a "fast" symlink with an external xattr block and therefore treated it as a "slow" symlink, which failed validation.

Fix this by updating ext2fs_inode_data_blocks2() to subtract the cluster size rather than the block size.

Signed-off-by Eric Biggers <[email protected]>