[PATCH] xfs: validate dquot buffer geometry in dquot log recovery

[PATCH] xfs: validate dquot buffer geometry in dquot log recovery

[Xingyuan Mo]

xlog_recover_dquot_commit_pass2() has never really validated the dquot
log item. The only check on qlf_len is a DEBUG-only ASSERT, and
qlf_boffset goes straight into xfs_buf_offset() to pick the dquot slot
inside the chunk buffer:

        dqb = xfs_buf_offset(bp, dq_f->qlf_boffset);
        ...
        memcpy(ddq, recddq, item->ri_buf[1].iov_len);

Both the offset and the length of the memcpy come from the journal of
the filesystem being mounted, so a crafted log can make mount time
replay write past the end of the chunk buffer. Sub-page block sizes
get an exact-size kmalloc for the buffer, so an offset of one blocksize
lands right on the KASAN redzone; larger offsets are wild writes. On
CRC filesystems the dd_lsn load in between is out of bounds too. The
recovery regions are allocated to the logged length, so a short region
0 also turns the qlf_* dereferences into out-of-bounds reads.

Make this code validate the item like the rest of log recovery does:
both regions must be exactly the expected size, qlf_len must be 1, and
qlf_boffset must point inside the buffer that is about to be read.
The readahead path gets the region length and qlf_len checks too.

I reproduced this on a v4 filesystem with 512-byte blocks and user
quota enabled: force a few dquot transactions into the log, crash the
box so they stay dirty, rewrite qlf_boffset of the on-disk XFS_LI_DQUOT
items to 512 (one block past bp->b_addr) and mount. Without the fix,
replay trips KASAN:

BUG: KASAN: slab-out-of-bounds in xlog_recover_dquot_commit_pass2+0x32a/0x500
Write of size 104 at addr ffff88801c8b0200 by task mount/430
CPU: 0 UID: 0 PID: 430 Comm: mount Not tainted 7.4.0-rc2-00021-gb3dfbde62cf1 #44 PREEMPT(lazy)
Call Trace:
 <TASK>
 dump_stack_lvl+0x73/0xb0
 print_report+0xd1/0x610
 kasan_report+0xff/0x130
 kasan_check_range+0x10c/0x1c0
 __asan_memcpy+0x3f/0x70
 xlog_recover_dquot_commit_pass2+0x32a/0x500
 xlog_recover_items_pass2+0x7d/0x1b0
 xlog_recover_commit_trans+0x585/0x630
 xlog_recovery_process_trans+0xe2/0xf0
 xlog_recover_process_ophdr+0xf8/0x1d0
 xlog_recover_process_data+0x126/0x270
 xlog_recover_process+0x132/0x210
 xlog_do_recovery_pass+0x6be/0x8f0
 xlog_do_log_recovery+0x65/0x90
 xlog_do_recover+0x7d/0x2b0
 xlog_recover+0x1bc/0x2d0
 xfs_log_mount+0x17b/0x2c0
 xfs_mountfs+0x70a/0xe70
 xfs_fs_fill_super+0xaca/0xf60
 get_tree_bdev+0x14/0x20
 xfs_fs_get_tree+0x19/0x20
 vfs_get_tree+0x54/0x170
 path_mount+0x4ad/0x13a0
 __x64_sys_mount+0x1dd/0x210
 do_syscall_64+0x10b/0x5f0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
 </TASK>

With the fix the same image is rejected without any out-of-bounds
access, and a pristine dirty log still recovers fine:

XFS (loop2): log mount/recovery failed: error -117

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Xingyuan Mo <hdthky0@gmail.com>
---
 fs/xfs/xfs_dquot_item_recover.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_dquot_item_recover.c b/fs/xfs/xfs_dquot_item_recover.c
index b8854316ac756..3172efb94c277 100644
--- a/fs/xfs/xfs_dquot_item_recover.c
+++ b/fs/xfs/xfs_dquot_item_recover.c
@@ -47,7 +47,9 @@ xlog_recover_dquot_ra_pass2(
 
 	dq_f = item->ri_buf[0].iov_base;
 	ASSERT(dq_f);
-	ASSERT(dq_f->qlf_len == 1);
+	if (item->ri_buf[0].iov_len != sizeof(struct xfs_dq_logformat) ||
+	    dq_f->qlf_len != 1)
+		return;
 
 	xlog_buf_readahead(log, dq_f->qlf_blkno,
 			XFS_FSB_TO_BB(mp, dq_f->qlf_len),
@@ -84,7 +86,7 @@ xlog_recover_dquot_commit_pass2(
 		xfs_alert(log->l_mp, "NULL dquot in %s.", __func__);
 		return -EFSCORRUPTED;
 	}
-	if (item->ri_buf[1].iov_len < sizeof(struct xfs_disk_dquot)) {
+	if (item->ri_buf[1].iov_len != sizeof(struct xfs_disk_dquot)) {
 		xfs_alert(log->l_mp, "dquot too small (%zd) in %s.",
 			item->ri_buf[1].iov_len, __func__);
 		return -EFSCORRUPTED;
@@ -110,13 +112,22 @@ xlog_recover_dquot_commit_pass2(
 	 */
 	dq_f = item->ri_buf[0].iov_base;
 	ASSERT(dq_f);
+
+	if (item->ri_buf[0].iov_len != sizeof(struct xfs_dq_logformat))
+		return -EFSCORRUPTED;
+
 	fa = xfs_dquot_verify(mp, recddq, dq_f->qlf_id);
 	if (fa) {
 		xfs_alert(mp, "corrupt dquot ID 0x%x in log at %pS",
 				dq_f->qlf_id, fa);
 		return -EFSCORRUPTED;
 	}
-	ASSERT(dq_f->qlf_len == 1);
+	if (dq_f->qlf_len != 1)
+		return -EFSCORRUPTED;
+
+	if (dq_f->qlf_boffset > BBTOB(XFS_FSB_TO_BB(mp, dq_f->qlf_len)) -
+				  sizeof(struct xfs_dqblk))
+		return -EFSCORRUPTED;
 
 	/*
 	 * At this point we are assuming that the dquots have been allocated
-- 
2.43.0