[PATCH] ocfs2: validate root extent tree depth during inode read

ZhengYuan Huang <[email protected]>
Newsgroups dev.linux.lists.ocfs2-devel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
[BUG]
A corrupt dinode can carry an extent root whose l_tree_depth exceeds
OCFS2_MAX_PATH_DEPTH. When truncate builds a path from that root, OCFS2
hits the internal BUG_ON() in ocfs2_new_path():

kernel BUG at fs/ocfs2/alloc.c:686!
Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
RIP: 0010:ocfs2_new_path+0x1d5/0x240 fs/ocfs2/alloc.c:686
Call Trace:
 ocfs2_commit_truncate+0x29f/0x1aa0 fs/ocfs2/alloc.c:7253
 ocfs2_truncate_file+0xcdd/0x13c0 fs/ocfs2/file.c:509
 ocfs2_setattr+0xa6d/0x1fd0 fs/ocfs2/file.c:1212
 notify_change+0x4b5/0x1030 fs/attr.c:546
 do_truncate+0x1d2/0x230 fs/open.c:68
 handle_truncate fs/namei.c:3596 [inline]
 do_open fs/namei.c:3979 [inline]
 path_openat+0x260f/0x2ce0 fs/namei.c:4134
 do_filp_open+0x1f6/0x430 fs/namei.c:4161
 do_sys_openat2+0x117/0x1c0 fs/open.c:1437
 do_sys_open fs/open.c:1452 [inline]
 __do_sys_openat fs/open.c:1468 [inline]
 __se_sys_openat fs/open.c:1463 [inline]
 __x64_sys_openat+0x15b/0x220 fs/open.c:1463
 ...

[CAUSE]
Extent blocks and __ocfs2_find_path() already treat oversized
l_tree_depth values as on-disk corruption, but the dinode root extent
list is never checked when the inode is read.

That lets a corrupt root extent list reach ocfs2_new_path(), which
stores the tree depth in a fixed-size struct ocfs2_path and therefore
expects the root metadata to have been validated already.

[FIX]
Validate the dinode root extent tree depth in ocfs2_validate_inode_block(),
which is the same cold metadata-read boundary where OCFS2 already checks
other on-disk geometry fields.

Also report this condition as unsupported from online filecheck so the
checker does not try to invent a replacement tree depth for a corrupt
extent tree root.

Signed-off-by: ZhengYuan Huang <[email protected]>
---
No Fixes tag is included because this is a missing metadata validation
check at inode-read time, not a regression that can be cleanly tied to a
single introducing commit.
---
 fs/ocfs2/inode.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
index a510a0eb1adc..b3456e627689 100644
--- a/fs/ocfs2/inode.c
+++ b/fs/ocfs2/inode.c
@@ -1525,6 +1525,15 @@ int ocfs2_validate_inode_block(struct super_block *sb,
 		}
 	}
 
+	if (ocfs2_dinode_has_extents(di) &&
+	    le16_to_cpu(di->id2.i_list.l_tree_depth) >= OCFS2_MAX_PATH_DEPTH) {
+		rc = ocfs2_error(sb,
+				 "Invalid dinode %llu: extent list tree depth %u\n",
+				 (unsigned long long)bh->b_blocknr,
+				 le16_to_cpu(di->id2.i_list.l_tree_depth));
+		goto bail;
+	}
+
 	if (le32_to_cpu(di->i_flags) & OCFS2_CHAIN_FL) {
 		struct ocfs2_chain_list *cl = &di->id2.i_chain;
 		u16 bpc = 1 << (OCFS2_SB(sb)->s_clustersize_bits -
@@ -1626,6 +1635,15 @@ static int ocfs2_filecheck_validate_inode_block(struct super_block *sb,
 		rc = -OCFS2_FILECHECK_ERR_GENERATION;
 	}
 
+	if (ocfs2_dinode_has_extents(di) &&
+	    le16_to_cpu(di->id2.i_list.l_tree_depth) >= OCFS2_MAX_PATH_DEPTH) {
+		mlog(ML_ERROR,
+		     "Filecheck: invalid dinode #%llu: extent list tree depth %u\n",
+		     (unsigned long long)bh->b_blocknr,
+		     le16_to_cpu(di->id2.i_list.l_tree_depth));
+		rc = -OCFS2_FILECHECK_ERR_UNSUPPORTED;
+	}
+
 bail:
 	return rc;
 }
@@ -1633,11 +1651,14 @@ static int ocfs2_filecheck_validate_inode_block(struct super_block *sb,
 static int ocfs2_filecheck_repair_inode_block(struct super_block *sb,
 					      struct buffer_head *bh)
 {
-	int changed = 0;
+	int changed = 0, rc;
 	struct ocfs2_dinode *di = (struct ocfs2_dinode *)bh->b_data;
 
-	if (!ocfs2_filecheck_validate_inode_block(sb, bh))
+	rc = ocfs2_filecheck_validate_inode_block(sb, bh);
+	if (!rc)
 		return 0;
+	if (rc == -OCFS2_FILECHECK_ERR_UNSUPPORTED)
+		return rc;
 
 	trace_ocfs2_filecheck_repair_inode_block(
 		(unsigned long long)bh->b_blocknr);
-- 
2.49.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.