ocfs2: Add check for total number of chains in chain list

The functions ocfs2_reserve_suballoc_bits(), ocfs2_block_group_alloc(), ocfs2_block_group_alloc_contig() and ocfs2_find_smallest_chain() trust the on-disk values related to the allocation chain. However, KASAN bug was triggered in these functions, and the kernel panicked when accessing redzoned memory. This occurred due to the corrupted value of cl_count field of struct ocfs2_chain_list. Upon analysis, the value of cl_count was observed to be overwhemingly large, due to which the code accessed redzoned memory.

The fix introduces an if statement which validates value of cl_count (both lower and upper bounds). Lower bound check ensures the value of cl_count is not zero and upper bound check ensures that the value of cl_count is in the range such that it has a value less than the total size of struct ocfs2_chain_list and maximum number of chains that can be present, so as to fill one block.

Reported-by [email protected]

Closes https://syzkaller.appspot.com/bug?extid=af14efe17dfa46173239

Tested-by [email protected]

Cc [email protected]

Signed-off-by Prithvi Tambewagh <[email protected]>

---
 fs/ocfs2/suballoc.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

fs/ocfs2/suballoc.c +15 -0

index f7b483f0de2a..7ea63e9cc4f8 100644
@@ -671,6 +671,21 @@ static int ocfs2_block_group_alloc(struct ocfs2_super *osb,
 	BUG_ON(ocfs2_is_cluster_bitmap(alloc_inode));
 
 	cl = &fe->id2.i_chain;
+	unsigned int block_size = osb->sb->s_blocksize;
+	unsigned int max_cl_count =
+	(block_size - offsetof(struct ocfs2_chain_list, cl_recs)) /
+	sizeof(struct ocfs2_chain_rec);
+
+	if (!le16_to_cpu(cl->cl_count) ||
+	    le16_to_cpu(cl->cl_count) > max_cl_count) {
+		ocfs2_error(osb->sb,
+			    "Invalid chain list: cl_count %u "
+			    "exceeds max %u",
+			    le16_to_cpu(cl->cl_count), max_cl_count);
+		status = -EIO;
+		goto bail;
+	}
+
 	status = ocfs2_reserve_clusters_with_limit(osb,
 						   le16_to_cpu(cl->cl_cpg),
 						   max_block, flags, &ac);

base-commit: 36c254515dc6592c44db77b84908358979dd6b50

Signature · 1 line

2.34.1

Quoted 52 lines · Prithvi Tambewagh

The functions ocfs2_reserve_suballoc_bits(), ocfs2_block_group_alloc(), ocfs2_block_group_alloc_contig() and ocfs2_find_smallest_chain() trust the on-disk values related to the allocation chain. However, KASAN bug was triggered in these functions, and the kernel panicked when accessing redzoned memory. This occurred due to the corrupted value of cl_count field of struct ocfs2_chain_list. Upon analysis, the value of cl_count was observed to be overwhemingly large, due to which the code accessed redzoned memory.

The fix introduces an if statement which validates value of cl_count (both lower and upper bounds). Lower bound check ensures the value of cl_count is not zero and upper bound check ensures that the value of cl_count is in the range such that it has a value less than the total size of struct ocfs2_chain_list and maximum number of chains that can be present, so as to fill one block.

Reported-by [email protected]

Closes https://syzkaller.appspot.com/bug?extid=af14efe17dfa46173239

Tested-by [email protected]

Cc [email protected]

Signed-off-by Prithvi Tambewagh <[email protected]>

---
 fs/ocfs2/suballoc.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

fs/ocfs2/suballoc.c +15 -0

index f7b483f0de2a..7ea63e9cc4f8 100644
@@ -671,6 +671,21 @@ static int ocfs2_block_group_alloc(struct ocfs2_super *osb,
 	BUG_ON(ocfs2_is_cluster_bitmap(alloc_inode));
 
 	cl = &fe->id2.i_chain;
+	unsigned int block_size = osb->sb->s_blocksize;
+	unsigned int max_cl_count =
+	(block_size - offsetof(struct ocfs2_chain_list, cl_recs)) /
+	sizeof(struct ocfs2_chain_rec);
+
+	if (!le16_to_cpu(cl->cl_count) ||
+	    le16_to_cpu(cl->cl_count) > max_cl_count) {
+		ocfs2_error(osb->sb,
+			    "Invalid chain list: cl_count %u "
+			    "exceeds max %u",
+			    le16_to_cpu(cl->cl_count), max_cl_count);
+		status = -EIO;
+		goto bail;
+	}
+
 	status = ocfs2_reserve_clusters_with_limit(osb,
 						   le16_to_cpu(cl->cl_cpg),
 						   max_block, flags, &ac);

Quoted 4 lines

base-commit: 36c254515dc6592c44db77b84908358979dd6b50

Signature · 1 line

2.34.1

Since 'fe' is read by ocfs2_read_inode_block(), the validation function ocfs2_validate_inode_block() is the appropriate place to perform this sanity check.

Please follow the pattern in commit e1c70505ee81 ("ocfs2: add extra consistency checks for chain allocator dinodes") when adding your code.

btw, I am a little bit confused, it seems commit e1c70505ee81 is enough to fix this syzbot issue.

Thanks,
Heming