[PATCH v2] ocfs2: validate dx entry list counts on read

ZhengYuan Huang <[email protected]>
Newsgroups dev.linux.lists.ocfs2-devel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
[BUG]
A corrupt indexed directory dx leaf can carry de_num_used larger than
de_count. When unlinkat() removes an indexed entry, that corrupt count
reaches ocfs2_dx_list_remove_entry() and drives an oversized memmove():

BUG: KASAN: use-after-free in ocfs2_dx_list_remove_entry fs/ocfs2/dir.c:1246 [inline]
BUG: KASAN: use-after-free in ocfs2_delete_entry_dx fs/ocfs2/dir.c:1363 [inline]
BUG: KASAN: use-after-free in ocfs2_delete_entry+0xb47/0x1070 fs/ocfs2/dir.c:1417
Read of size 401648 at addr ffff888020b8bd20 by task syz.0.1/347
Call Trace:
 __dump_stack lib/dump_stack.c:94 [inline]
 dump_stack_lvl+0xbe/0x130 lib/dump_stack.c:120
 print_address_description mm/kasan/report.c:378 [inline]
 print_report+0xd1/0x650 mm/kasan/report.c:482
 kasan_report+0xfb/0x140 mm/kasan/report.c:595
 check_region_inline mm/kasan/generic.c:186 [inline]
 kasan_check_range+0x11c/0x200 mm/kasan/generic.c:200
 __asan_memmove+0x23/0x80 mm/kasan/shadow.c:94
 ocfs2_dx_list_remove_entry fs/ocfs2/dir.c:1246 [inline]
 ocfs2_delete_entry_dx fs/ocfs2/dir.c:1363 [inline]
 ocfs2_delete_entry+0xb47/0x1070 fs/ocfs2/dir.c:1417
 ocfs2_unlink+0xb79/0x1900 fs/ocfs2/namei.c:991
 vfs_unlink+0x2c2/0x900 fs/namei.c:4673
 do_unlinkat+0x474/0x680 fs/namei.c:4737
 __do_sys_unlinkat fs/namei.c:4778 [inline]
 __se_sys_unlinkat fs/namei.c:4771 [inline]
 __x64_sys_unlinkat+0xae/0x130 fs/namei.c:4771
 ...

[CAUSE]
DX leaf and inline dx root blocks validate signatures and ECC, but they
do not validate the geometry of struct ocfs2_dx_entry_list. Corrupt
de_count/de_num_used values therefore survive block read and are later
used as traversal bounds and memmove/memset lengths in lookup, delete
and rebalance paths. In the unlink case, lookup can still find the
target entry before the bad bound matters, and the same corrupt
de_num_used then reaches ocfs2_dx_list_remove_entry().

[FIX]
Validate dx entry-list geometry in the metadata read callbacks. de_count
must match the block layout for the current superblock, and de_num_used
must not exceed de_count. Do this for both dx leaves and inline dx
roots so corrupted metadata is rejected before any caller walks or
rewrites de_entries[].

Signed-off-by: ZhengYuan Huang <[email protected]>
---
No Fixes tag is included because this is a missing metadata validation
check, not a regression attributable to a single introducing commit.

v2:
- Reframe the bug report around the unlinkat-triggered UAF
- Keep the code change unchanged because it fixes the same root cause
---
fs/ocfs2/dir.c | 40 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
index 8e6b03238327..a89634e20b32 100644
--- a/fs/ocfs2/dir.c
+++ b/fs/ocfs2/dir.c
@@ -578,6 +578,29 @@ static int ocfs2_read_dir_block_direct(struct inode *dir, u64 phys,
 	return ret;
 }
 
+static int ocfs2_validate_dx_entry_list(struct super_block *sb, u64 blkno,
+					struct ocfs2_dx_entry_list *entry_list,
+					unsigned int expected,
+					const char *owner)
+{
+	unsigned int count = le16_to_cpu(entry_list->de_count);
+	unsigned int num_used = le16_to_cpu(entry_list->de_num_used);
+
+	if (count != expected)
+		return ocfs2_error(sb,
+				   "%s # %llu has invalid de_count %u (expected %u)\n",
+				   owner, (unsigned long long)blkno, count,
+				   expected);
+
+	if (num_used > count)
+		return ocfs2_error(sb,
+				   "%s # %llu has invalid de_num_used %u (de_count %u)\n",
+				   owner, (unsigned long long)blkno, num_used,
+				   count);
+
+	return 0;
+}
+
 static int ocfs2_validate_dx_root(struct super_block *sb,
 				  struct buffer_head *bh)
 {
@@ -604,7 +625,14 @@ static int ocfs2_validate_dx_root(struct super_block *sb,
 		goto bail;
 	}
 
-	if (!(dx_root->dr_flags & OCFS2_DX_FLAG_INLINE)) {
+	if (dx_root->dr_flags & OCFS2_DX_FLAG_INLINE) {
+		ret = ocfs2_validate_dx_entry_list(sb, bh->b_blocknr,
+						   &dx_root->dr_entries,
+						   ocfs2_dx_entries_per_root(sb),
+						   "Dir Index Root");
+		if (ret)
+			goto bail;
+	} else {
 		struct ocfs2_extent_list *el = &dx_root->dr_list;
 
 		if (le16_to_cpu(el->l_count) != ocfs2_extent_recs_per_dx_root(sb)) {
@@ -660,13 +688,21 @@ static int ocfs2_validate_dx_leaf(struct super_block *sb,
 		mlog(ML_ERROR,
 		     "Checksum failed for dir index leaf block %llu\n",
 		     (unsigned long long)bh->b_blocknr);
-		return ret;
+		goto bail;
 	}
 
 	if (!OCFS2_IS_VALID_DX_LEAF(dx_leaf)) {
 		ret = ocfs2_error(sb, "Dir Index Leaf has bad signature %.*s\n",
 				  7, dx_leaf->dl_signature);
+		goto bail;
 	}
 
+	ret = ocfs2_validate_dx_entry_list(sb, bh->b_blocknr,
+					   &dx_leaf->dl_list,
+					   ocfs2_dx_entries_per_leaf(sb),
+					   "Dir Index Leaf");
+
+bail:
+
 	return ret;
 }
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.