public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ocfs2: handle OCFS2_SUPER_BLOCK_FL flag in system dinode
@ 2025-12-16 20:05 Prithvi Tambewagh
  2025-12-18 16:07 ` Heming Zhao
  0 siblings, 1 reply; 3+ messages in thread
From: Prithvi Tambewagh @ 2025-12-16 20:05 UTC (permalink / raw)
  To: mark, jlbec, joseph.qi
  Cc: ocfs2-devel, linux-kernel, linux-kernel-mentees, skhan,
	david.hunter.linux, khalid, Prithvi Tambewagh,
	syzbot+779d072a1067a8b1a917, stable

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] ocfs2: handle OCFS2_SUPER_BLOCK_FL flag in system dinode
  2025-12-16 20:05 [PATCH] ocfs2: handle OCFS2_SUPER_BLOCK_FL flag in system dinode Prithvi Tambewagh
@ 2025-12-18 16:07 ` Heming Zhao
  2025-12-19  6:16   ` Prithvi
  0 siblings, 1 reply; 3+ messages in thread
From: Heming Zhao @ 2025-12-18 16:07 UTC (permalink / raw)
  To: Prithvi Tambewagh
  Cc: mark, jlbec, joseph.qi, ocfs2-devel, linux-kernel,
	linux-kernel-mentees, skhan, david.hunter.linux, khalid,
	syzbot+779d072a1067a8b1a917, stable

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ocfs2: handle OCFS2_SUPER_BLOCK_FL flag in system dinode
  2025-12-18 16:07 ` Heming Zhao
@ 2025-12-19  6:16   ` Prithvi
  0 siblings, 0 replies; 3+ messages in thread
From: Prithvi @ 2025-12-19  6:16 UTC (permalink / raw)
  To: Heming Zhao
  Cc: mark, jlbec, joseph.qi, ocfs2-devel, linux-kernel,
	linux-kernel-mentees, skhan, david.hunter.linux, khalid,
	syzbot+779d072a1067a8b1a917, stable

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-12-19  6:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-16 20:05 [PATCH] ocfs2: handle OCFS2_SUPER_BLOCK_FL flag in system dinode Prithvi Tambewagh
2025-12-18 16:07 ` Heming Zhao
2025-12-19  6:16   ` Prithvi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox