[PATCH] ocfs2: handle OCFS2_SUPER_BLOCK_FL flag in system dinode

Prithvi Tambewagh posted 1 patch 1 month, 3 weeks ago
fs/ocfs2/inode.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
[PATCH] ocfs2: handle OCFS2_SUPER_BLOCK_FL flag in system dinode
Posted by Prithvi Tambewagh 1 month, 3 weeks ago
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: syzbot+779d072a1067a8b1a917@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=779d072a1067a8b1a917
Tested-by: syzbot+779d072a1067a8b1a917@syzkaller.appspotmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Prithvi Tambewagh <activprithvi@gmail.com>
---
 fs/ocfs2/inode.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
index 12e5d1f73325..f439dc801845 100644
--- a/fs/ocfs2/inode.c
+++ b/fs/ocfs2/inode.c
@@ -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
-- 
2.43.0
Re: [PATCH] ocfs2: handle OCFS2_SUPER_BLOCK_FL flag in system dinode
Posted by Heming Zhao 1 month, 2 weeks ago
On Wed, Dec 17, 2025 at 01:35:44AM +0530, Prithvi Tambewagh wrote:
> 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: syzbot+779d072a1067a8b1a917@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=779d072a1067a8b1a917
> Tested-by: syzbot+779d072a1067a8b1a917@syzkaller.appspotmail.com
> Cc: stable@vger.kernel.org
> Signed-off-by: Prithvi Tambewagh <activprithvi@gmail.com>
> ---
>  fs/ocfs2/inode.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
> index 12e5d1f73325..f439dc801845 100644
> --- a/fs/ocfs2/inode.c
> +++ b/fs/ocfs2/inode.c
> @@ -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
> -- 
> 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
Re: [PATCH] ocfs2: handle OCFS2_SUPER_BLOCK_FL flag in system dinode
Posted by Prithvi 1 month, 2 weeks ago
On Fri, Dec 19, 2025 at 12:07:56AM +0800, Heming Zhao wrote:
> On Wed, Dec 17, 2025 at 01:35:44AM +0530, Prithvi Tambewagh wrote:
> > 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: syzbot+779d072a1067a8b1a917@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=779d072a1067a8b1a917
> > Tested-by: syzbot+779d072a1067a8b1a917@syzkaller.appspotmail.com
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Prithvi Tambewagh <activprithvi@gmail.com>
> > ---
> >  fs/ocfs2/inode.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
> > index 12e5d1f73325..f439dc801845 100644
> > --- a/fs/ocfs2/inode.c
> > +++ b/fs/ocfs2/inode.c
> > @@ -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
> > -- 
> > 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

Hello Heming,

Thanks for the detailed explanation . I now understand more clearly how
removing BUG() from ocfs2_populate_inode() for inode having 
OCFS2_SUPER_BLOCK_FL set would cause problems in OCFS2 for the path
including __ocfs2_mknod_locked(). Since OCFS2_SUPER_BLOCK_FL flag on a
system dinode is indicative of metadata corruption and considering our
discussion, keeping BUG() for the same is appropriate.

Thanks,
Prithvi