summaryrefslogtreecommitdiff
path: root/fs/btrfs/inode.c
diff options
context:
space:
mode:
authorQu Wenruo <wqu@suse.com>2021-07-26 09:34:51 +0300
committerDavid Sterba <dsterba@suse.com>2021-08-23 14:19:03 +0300
commit3670e6451bc9c39ab3a46f1da19360219e4319f3 (patch)
tree91a62880bf0e06cc32f99ca9e9b38f225f5bb8fd /fs/btrfs/inode.c
parent4c37a7938496756649096a7ec26320eb8b0d90fb (diff)
downloadlinux-3670e6451bc9c39ab3a46f1da19360219e4319f3.tar.xz
btrfs: subpage: check if there are compressed extents inside one page
[BUG] When testing experimental subpage compressed write support, it hits a NULL pointer dereference inside read path: Unable to handle kernel NULL pointer dereference at virtual address 0000000000000018 pc : __pi_memcmp+0x28/0x1ec lr : check_data_csum+0xd0/0x274 [btrfs] Call trace: __pi_memcmp+0x28/0x1ec btrfs_verify_data_csum+0xf4/0x244 [btrfs] end_bio_extent_readpage+0x1d0/0x6b0 [btrfs] bio_endio+0x15c/0x1dc end_workqueue_fn+0x44/0x64 [btrfs] btrfs_work_helper+0x74/0x250 [btrfs] process_one_work+0x1d4/0x47c worker_thread+0x180/0x400 kthread+0x11c/0x120 ret_from_fork+0x10/0x30 Code: 54000261 d100044c d343fd8c f8408403 (f8408424) ---[ end trace 9e2c59f33ea40866 ]--- [CAUSE] When reading two compressed extents inside the same page, like the following layout, we trigger above crash: 0 32K 64K |-------|\\\\\\\| | \- Compressed extent (A) \--------- Compressed extent (B) For compressed read, we don't need to populate its io_bio->csum, as we rely on compressed_bio->csum to verify the compressed data, and then copy the decompressed to inode pages. Normally btrfs_verify_data_csum() skip such page by checking and clearing its PageChecked flag But since that flag is still for the full page, when endio for inode page range [0, 32K) gets executed, it clears PageChecked flag for the full page. Then when endio for inode page range [32K, 64K) gets executed, since the page no longer has PageChecked flag, it just continues checking, even though io_bio->csum is NULL. [FIX] Thankfully there are only two users of PageChecked bit: - Cow fixup Since subpage has its own way to trace page dirty (dirty_bitmap) and ordered bit (ordered_bitmap), it should never trigger cow fixup. - Compressed read We can distinguish such read by just checking io_bio->csum. So just check io_bio->csum before doing the verification to avoid such NULL pointer dereference. Signed-off-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
Diffstat (limited to 'fs/btrfs/inode.c')
-rw-r--r--fs/btrfs/inode.c14
1 files changed, 14 insertions, 0 deletions
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 83caf69295f7..ae9e4ad1949a 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3256,6 +3256,20 @@ unsigned int btrfs_verify_data_csum(struct btrfs_io_bio *io_bio, u32 bio_offset,
return 0;
}
+ /*
+ * For subpage case, above PageChecked is not safe as it's not subpage
+ * compatible.
+ * But for now only cow fixup and compressed read utilize PageChecked
+ * flag, while in this context we can easily use io_bio->csum to
+ * determine if we really need to do csum verification.
+ *
+ * So for now, just exit if io_bio->csum is NULL, as it means it's
+ * compressed read, and its compressed data csum has already been
+ * verified.
+ */
+ if (io_bio->csum == NULL)
+ return 0;
+
if (BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)
return 0;