Re: [PATCH v2 2/3] ocfs2: reject dinodes whose i_rdev disagrees with the file type

On Tue, May 19, 2026 at 07:04:03AM -0400, Michael Bommarito wrote:
> id1.dev1.i_rdev is the device-number arm of the ocfs2_dinode id1 union.
> It is only meaningful for character and block device inodes. For any
> other user-visible file type the on-disk value must be zero.
> 
> ocfs2_populate_inode() currently copies id1.dev1.i_rdev into
> inode->i_rdev before the S_IFMT switch decides whether the inode is a
> special file. A non-device inode with a non-zero i_rdev can therefore
> publish stale or attacker-controlled device state into the in-core inode.
> 
> System inodes legitimately use other arms of the same union, so keep
> the cross-check restricted to non-system inodes. Factor that predicate
> into a helper and use it in both the normal validator and online
> filecheck path; filecheck reports the malformed dinode through
> OCFS2_FILECHECK_ERR_INVALIDINO instead of ocfs2_error().
> 
> Fixes: b657c95c1108 ("ocfs2: Wrap inode block reads in a dedicated function.")
> Cc: [email protected]
> Signed-off-by: Michael Bommarito <[email protected]>
> Assisted-by: Claude:claude-opus-4-7

Reviewed-by: Joel Becker <[email protected]>

> ---
>  fs/ocfs2/inode.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
> index e149ccbdc03ce..992980ea98046 100644
> --- a/fs/ocfs2/inode.c
> +++ b/fs/ocfs2/inode.c
> @@ -72,6 +72,16 @@ static bool ocfs2_valid_inode_mode(umode_t mode)
>  	return fs_umode_to_ftype(mode) != FT_UNKNOWN;
>  }
>  
> +static bool ocfs2_dinode_has_unexpected_rdev(struct ocfs2_dinode *di)
> +{
> +	umode_t mode = le16_to_cpu(di->i_mode);
> +
> +	if (le32_to_cpu(di->i_flags) & OCFS2_SYSTEM_FL)
> +		return false;
> +
> +	return !S_ISCHR(mode) && !S_ISBLK(mode) && di->id1.dev1.i_rdev != 0;
> +}
> +
>  void ocfs2_set_inode_flags(struct inode *inode)
>  {
>  	unsigned int flags = OCFS2_I(inode)->ip_attr;
> @@ -1518,6 +1528,41 @@ int ocfs2_validate_inode_block(struct super_block *sb,
>  		goto bail;
>  	}
>  
> +	/*
> +	 * id1.dev1.i_rdev is the device-number arm of the id1 union and
> +	 * is only meaningful for character and block device inodes.  For
> +	 * any other regular user-visible file type the on-disk value
> +	 * must be zero.  ocfs2_populate_inode() currently runs
> +	 *
> +	 *     inode->i_rdev = huge_decode_dev(le64_to_cpu(fe->id1.dev1.i_rdev));
> +	 *
> +	 * unconditionally, before the S_IFMT switch decides whether the
> +	 * inode is a special file.  As a result, an i_rdev value present
> +	 * on a non-device inode is silently published into the in-core
> +	 * inode; a subsequent forced re-read or in-core mode mutation
> +	 * (cluster peer with raw write access to the shared LUN,
> +	 * on-disk corruption, or a separately forged dinode) can then
> +	 * expose the attacker-controlled device number to
> +	 * init_special_inode() without ever showing an unusual i_mode
> +	 * at validation time.
> +	 *
> +	 * System inodes (OCFS2_SYSTEM_FL) legitimately use the bitmap1
> +	 * and journal1 arms of the same union (allocator i_used /
> +	 * i_total counters and the journal ij_flags /
> +	 * ij_recovery_generation pair); those bytes are not an i_rdev
> +	 * and must not be checked here.  Restrict the cross-check to
> +	 * non-system inodes, which is the full attacker-controllable
> +	 * surface.
> +	 */
> +	if (ocfs2_dinode_has_unexpected_rdev(di)) {
> +		rc = ocfs2_error(sb,
> +				 "Invalid dinode #%llu: non-device mode 0%o with i_rdev %llu\n",
> +				 (unsigned long long)bh->b_blocknr,
> +				 le16_to_cpu(di->i_mode),
> +				 (unsigned long long)le64_to_cpu(di->id1.dev1.i_rdev));
> +		goto bail;
> +	}
> +
>  	if (le16_to_cpu(di->i_dyn_features) & OCFS2_INLINE_DATA_FL) {
>  		struct ocfs2_inline_data *data = &di->id2.i_data;
>  
> @@ -1657,6 +1702,16 @@ static int ocfs2_filecheck_validate_inode_block(struct super_block *sb,
>  		     (unsigned long long)bh->b_blocknr,
>  		     le16_to_cpu(di->i_mode));
>  		rc = -OCFS2_FILECHECK_ERR_INVALIDINO;
> +		goto bail;
> +	}
> +
> +	if (ocfs2_dinode_has_unexpected_rdev(di)) {
> +		mlog(ML_ERROR,
> +		     "Filecheck: invalid dinode #%llu: non-device mode 0%o with i_rdev %llu\n",
> +		     (unsigned long long)bh->b_blocknr,
> +		     le16_to_cpu(di->i_mode),
> +		     (unsigned long long)le64_to_cpu(di->id1.dev1.i_rdev));
> +		rc = -OCFS2_FILECHECK_ERR_INVALIDINO;
>  	}
>  
>  bail:
> -- 
> 2.53.0

-- 

"Nearly all men can stand adversity, but if you really want to
 test a man's character, give him power."
	- Abraham Lincoln

			http://www.jlbec.org/
			[email protected]