ocfs2: handle OCFS2_SUPER_BLOCK_FL flag in system dinode

When ocfs2_populate_inode() is called during mount process, if the flag OCFS2_SUPER_BLOCK_FL is set in on-disk system dinode, then BUG() is triggered, causing kernel to panic. This is indicative of metadata corruption.

This is fixed by calling ocfs2_error() to print the error log and the corresponding inode is marked as 'bad', so that it is not used further during the mount process. It is ensured that the fact of that inode being bad is propagated to caller ocfs2_populate_inode() i.e. ocfs2_read_locked_inode() using is_bad_inode() and further behind along the call trace as well.

Reported-by [email protected]

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

Tested-by [email protected]

Cc [email protected]

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

---
 fs/ocfs2/inode.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

fs/ocfs2/inode.c +12 -2

index 12e5d1f73325..f439dc801845 100644
@@ -347,7 +347,12 @@ void ocfs2_populate_inode(struct inode *inode, struct ocfs2_dinode *fe,
 	} else if (fe->i_flags & cpu_to_le32(OCFS2_SUPER_BLOCK_FL)) {
 		/* we can't actually hit this as read_inode can't
 		 * handle superblocks today ;-) */
-		BUG();
+		ocfs2_error(sb,
+			    "System Inode %llu has "
+			    "OCFS2_SUPER_BLOCK_FL set",
+			    (unsigned long long)le64_to_cpu(fe->i_blkno));
+		make_bad_inode(inode);
+		return;
 	}
 
 	switch (inode->i_mode & S_IFMT) {

@@ -555,6 +560,11 @@ static int ocfs2_read_locked_inode(struct inode *inode,
 
 	ocfs2_populate_inode(inode, fe, 0);
 
+	if (is_bad_inode(inode)) {
+		status = -EIO;
+		goto bail;
+	}
+
 	BUG_ON(args->fi_blkno != le64_to_cpu(fe->i_blkno));
 
 	if (buffer_dirty(bh) && !buffer_jbd(bh)) {

@@ -576,7 +586,7 @@ static int ocfs2_read_locked_inode(struct inode *inode,
 	if (can_lock)
 		ocfs2_inode_unlock(inode, lock_level);
 
-	if (status < 0)
+	if (status < 0 && !is_bad_inode(inode))
 		make_bad_inode(inode);
 
 	brelse(bh);

base-commit: d76bb1ebb5587f66b0f8b8099bfbb44722bc08b3

Signature · 1 line

2.43.0

Quoted 61 lines · Prithvi Tambewagh

When ocfs2_populate_inode() is called during mount process, if the flag OCFS2_SUPER_BLOCK_FL is set in on-disk system dinode, then BUG() is triggered, causing kernel to panic. This is indicative of metadata corruption.

This is fixed by calling ocfs2_error() to print the error log and the corresponding inode is marked as 'bad', so that it is not used further during the mount process. It is ensured that the fact of that inode being bad is propagated to caller ocfs2_populate_inode() i.e. ocfs2_read_locked_inode() using is_bad_inode() and further behind along the call trace as well.

Reported-by [email protected]

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

Tested-by [email protected]

Cc [email protected]

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

---
 fs/ocfs2/inode.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

fs/ocfs2/inode.c +12 -2

index 12e5d1f73325..f439dc801845 100644
@@ -347,7 +347,12 @@ void ocfs2_populate_inode(struct inode *inode, struct ocfs2_dinode *fe,
 	} else if (fe->i_flags & cpu_to_le32(OCFS2_SUPER_BLOCK_FL)) {
 		/* we can't actually hit this as read_inode can't
 		 * handle superblocks today ;-) */
-		BUG();
+		ocfs2_error(sb,
+			    "System Inode %llu has "
+			    "OCFS2_SUPER_BLOCK_FL set",
+			    (unsigned long long)le64_to_cpu(fe->i_blkno));
+		make_bad_inode(inode);
+		return;
 	}
 
 	switch (inode->i_mode & S_IFMT) {

@@ -555,6 +560,11 @@ static int ocfs2_read_locked_inode(struct inode *inode,
 
 	ocfs2_populate_inode(inode, fe, 0);
 
+	if (is_bad_inode(inode)) {
+		status = -EIO;
+		goto bail;
+	}
+
 	BUG_ON(args->fi_blkno != le64_to_cpu(fe->i_blkno));
 
 	if (buffer_dirty(bh) && !buffer_jbd(bh)) {

@@ -576,7 +586,7 @@ static int ocfs2_read_locked_inode(struct inode *inode,
 	if (can_lock)
 		ocfs2_inode_unlock(inode, lock_level);
 
-	if (status < 0)
+	if (status < 0 && !is_bad_inode(inode))
 		make_bad_inode(inode);
 
 	brelse(bh);

Quoted 5 lines

base-commit: d76bb1ebb5587f66b0f8b8099bfbb44722bc08b3

Signature · 1 line

2.43.0

ocfs2_populate_inode has two callers: __ocfs2_mknod_locked() and ocfs2_read_locked_inode()

Your code only works for the ocfs2_read_locked_inode() path, but not for the __ocfs2_mknod_locked() path.
In __ocfs2_mknod_locked(), there are two tasks after ocfs2_populate_inode:
"creating locks" and "updating the transaction". If you use a 'goto' to bypass these two tasks, ocfs2 will crash in the near future. Conversely, if you choose to execute the two jobs, the logic is flawed because we perform on a bad inode.

In my view, the existing code (using BUG()) is acceptable. We don't need to worry about this syzbot report.

Thanks,
Heming