[PATCH v2] ocfs2: reject inconsistent inode size before truncate

[PATCH v2] ocfs2: reject inconsistent inode size before truncate

[ZhengYuan Huang]

[BUG]
openat(..., O_WRONLY|O_CREAT|O_TRUNC) can hit:

kernel BUG at fs/ocfs2/file.c:454!
Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
RIP: 0010:ocfs2_truncate_file+0x1204/0x13c0 fs/ocfs2/file.c:454
Call Trace:
 ocfs2_setattr+0xa6d/0x1fd0 fs/ocfs2/file.c:1212
 notify_change+0x4b5/0x1030 fs/attr.c:546
 do_truncate+0x1d2/0x230 fs/open.c:68
 handle_truncate fs/namei.c:3596 [inline]
 do_open fs/namei.c:3979 [inline]
 path_openat+0x260f/0x2ce0 fs/namei.c:4134
 do_filp_open+0x1f6/0x430 fs/namei.c:4161
 do_sys_openat2+0x117/0x1c0 fs/open.c:1437
 do_sys_open fs/open.c:1452 [inline]
 __do_sys_openat fs/open.c:1468 [inline]
 __se_sys_openat fs/open.c:1463 [inline]
 __x64_sys_openat+0x15b/0x220 fs/open.c:1463
 ...

[CAUSE]
ocfs2_truncate_file() treats di_bh->i_size matching inode->i_size as an
internal code invariant and BUGs if it is broken.

That assumption is too strong for corrupted metadata. The dinode block can
still be structurally valid enough to pass ocfs2_read_inode_block() while
no longer matching an already-instantiated VFS inode. On local mounts,
ocfs2_inode_lock_update() skips refresh entirely, so truncate can
observe the mismatch directly and crash instead of rejecting the
corruption.

[FIX]
Turn the BUG_ON into normal OCFS2 corruption handling. If truncate sees
di_bh->i_size disagree with inode->i_size, report it with ocfs2_error() and
abort before touching truncate state.

This keeps the fix at the first boundary that actually requires the
sizes to match and avoids widening checks into hotter generic
inode-lock paths

Signed-off-by: ZhengYuan Huang <gality369@gmail.com>
---
v2:
- Tie the new truncate comment to the local mount case where inode refresh
  is skipped.

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -444,15 +444,17 @@ int ocfs2_truncate_file(struct inode *inode,
 	int status = 0;
 	struct ocfs2_dinode *fe = NULL;
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 
-	/* We trust di_bh because it comes from ocfs2_inode_lock(), which
-	 * already validated it */
+	/*
+	 * On local mounts ocfs2_inode_lock_update() skips the inode
+	 * refresh path, so truncation still needs to reject an inode
+	 * state that no longer matches di_bh.
+	 */
 	fe = (struct ocfs2_dinode *) di_bh->b_data;
 
 	trace_ocfs2_truncate_file((unsigned long long)OCFS2_I(inode)->ip_blkno,
 				  (unsigned long long)le64_to_cpu(fe->i_size),
 				  (unsigned long long)new_i_size);
 
-	mlog_bug_on_msg(le64_to_cpu(fe->i_size) != i_size_read(inode),
-			"Inode %llu, inode i_size = %lld != di "
-			"i_size = %llu, i_flags = 0x%x\n",
-			(unsigned long long)OCFS2_I(inode)->ip_blkno,
-			i_size_read(inode),
-			(unsigned long long)le64_to_cpu(fe->i_size),
-			le32_to_cpu(fe->i_flags));
+	if (unlikely(le64_to_cpu(fe->i_size) != i_size_read(inode))) {
+		status = ocfs2_error(inode->i_sb,
+				     "Inode %llu has inconsistent i_size: inode = %lld, dinode = %llu, i_flags = 0x%x\n",
+				     (unsigned long long)OCFS2_I(inode)->ip_blkno,
+				     i_size_read(inode),
+				     (unsigned long long)le64_to_cpu(fe->i_size),
+				     le32_to_cpu(fe->i_flags));
+		goto bail;
+	}
 
 	if (new_i_size > le64_to_cpu(fe->i_size)) {
 		trace_ocfs2_truncate_file_error(
 			(unsigned long long)le64_to_cpu(fe->i_size),
-- 
2.43.0

Re: [PATCH v2] ocfs2: reject inconsistent inode size before truncate

[Joseph Qi]

On 5/12/26 10:16 AM, ZhengYuan Huang wrote:
> [BUG]
> openat(..., O_WRONLY|O_CREAT|O_TRUNC) can hit:
> 
> kernel BUG at fs/ocfs2/file.c:454!
> Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
> RIP: 0010:ocfs2_truncate_file+0x1204/0x13c0 fs/ocfs2/file.c:454
> Call Trace:
>  ocfs2_setattr+0xa6d/0x1fd0 fs/ocfs2/file.c:1212
>  notify_change+0x4b5/0x1030 fs/attr.c:546
>  do_truncate+0x1d2/0x230 fs/open.c:68
>  handle_truncate fs/namei.c:3596 [inline]
>  do_open fs/namei.c:3979 [inline]
>  path_openat+0x260f/0x2ce0 fs/namei.c:4134
>  do_filp_open+0x1f6/0x430 fs/namei.c:4161
>  do_sys_openat2+0x117/0x1c0 fs/open.c:1437
>  do_sys_open fs/open.c:1452 [inline]
>  __do_sys_openat fs/open.c:1468 [inline]
>  __se_sys_openat fs/open.c:1463 [inline]
>  __x64_sys_openat+0x15b/0x220 fs/open.c:1463
>  ...
> 
> [CAUSE]
> ocfs2_truncate_file() treats di_bh->i_size matching inode->i_size as an
> internal code invariant and BUGs if it is broken.
> 
> That assumption is too strong for corrupted metadata. The dinode block can
> still be structurally valid enough to pass ocfs2_read_inode_block() while
> no longer matching an already-instantiated VFS inode. On local mounts,
> ocfs2_inode_lock_update() skips refresh entirely, so truncate can
> observe the mismatch directly and crash instead of rejecting the
> corruption.
> 
> [FIX]
> Turn the BUG_ON into normal OCFS2 corruption handling. If truncate sees
> di_bh->i_size disagree with inode->i_size, report it with ocfs2_error() and
> abort before touching truncate state.
> 
> This keeps the fix at the first boundary that actually requires the
> sizes to match and avoids widening checks into hotter generic
> inode-lock paths
> 
> Signed-off-by: ZhengYuan Huang <gality369@gmail.com>

Looks good.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

> ---
> v2:
> - Tie the new truncate comment to the local mount case where inode refresh
>   is skipped.
> 
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -444,15 +444,17 @@ int ocfs2_truncate_file(struct inode *inode,
>  	int status = 0;
>  	struct ocfs2_dinode *fe = NULL;
>  	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>  
> -	/* We trust di_bh because it comes from ocfs2_inode_lock(), which
> -	 * already validated it */
> +	/*
> +	 * On local mounts ocfs2_inode_lock_update() skips the inode
> +	 * refresh path, so truncation still needs to reject an inode
> +	 * state that no longer matches di_bh.
> +	 */
>  	fe = (struct ocfs2_dinode *) di_bh->b_data;
>  
>  	trace_ocfs2_truncate_file((unsigned long long)OCFS2_I(inode)->ip_blkno,
>  				  (unsigned long long)le64_to_cpu(fe->i_size),
>  				  (unsigned long long)new_i_size);
>  
> -	mlog_bug_on_msg(le64_to_cpu(fe->i_size) != i_size_read(inode),
> -			"Inode %llu, inode i_size = %lld != di "
> -			"i_size = %llu, i_flags = 0x%x\n",
> -			(unsigned long long)OCFS2_I(inode)->ip_blkno,
> -			i_size_read(inode),
> -			(unsigned long long)le64_to_cpu(fe->i_size),
> -			le32_to_cpu(fe->i_flags));
> +	if (unlikely(le64_to_cpu(fe->i_size) != i_size_read(inode))) {
> +		status = ocfs2_error(inode->i_sb,
> +				     "Inode %llu has inconsistent i_size: inode = %lld, dinode = %llu, i_flags = 0x%x\n",
> +				     (unsigned long long)OCFS2_I(inode)->ip_blkno,
> +				     i_size_read(inode),
> +				     (unsigned long long)le64_to_cpu(fe->i_size),
> +				     le32_to_cpu(fe->i_flags));
> +		goto bail;
> +	}
>  
>  	if (new_i_size > le64_to_cpu(fe->i_size)) {
>  		trace_ocfs2_truncate_file_error(
>  			(unsigned long long)le64_to_cpu(fe->i_size),