linux-kernel - Re: [PATCH] ocfs2: handle OCFS2_SUPER_BLOCK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <njikmwfpnsdzalxump7dj7wnlvkwfmvuqwxhpwzly45v7ioj5l@yckvd3veaifi>
Date: Fri, 19 Dec 2025 00:07:56 +0800
From: Heming Zhao <heming.zhao@...e.com>
To: Prithvi Tambewagh <activprithvi@...il.com>
Cc: mark@...heh.com, jlbec@...lplan.org, joseph.qi@...ux.alibaba.com, 
	ocfs2-devel@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	linux-kernel-mentees@...ts.linux.dev, skhan@...uxfoundation.org, david.hunter.linux@...il.com, 
	khalid@...nel.org, syzbot+779d072a1067a8b1a917@...kaller.appspotmail.com, 
	stable@...r.kernel.org
Subject: Re: [PATCH] ocfs2: handle OCFS2_SUPER_BLOCK_FL flag in system dinode

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@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=779d072a1067a8b1a917
> Tested-by: syzbot+779d072a1067a8b1a917@...kaller.appspotmail.com
> Cc: stable@...r.kernel.org
> Signed-off-by: Prithvi Tambewagh <activprithvi@...il.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