Re: [PATCH 1/3] ocfs2: reject dinodes with non-canonical i_mode type or stray bits

On 5/17/26 7:10 PM, Michael Bommarito wrote:
> ocfs2_validate_inode_block() currently accepts any 16-bit i_mode
> value as long as i_mode is non-zero.  ocfs2_populate_inode() then
> copies that mode verbatim into inode->i_mode and dispatches on
> i_mode & S_IFMT to the file/dir/symlink/special_file iops; any
> unrecognised type falls through to ocfs2_special_file_iops and
> init_special_inode(), which interprets id1.dev1.i_rdev as a
> device number.
> 
> The result is that anything able to forge or corrupt an inode
> block (a hostile cluster peer with raw write access to the
> shared LUN, a privileged user mounting an attacker-supplied
> image, on-disk corruption) can publish an in-core inode whose
> type bits do not name a POSIX file type, or whose permission
> bits carry bytes outside S_IFMT|07777.  Both shapes propagate
> into VFS-visible state that downstream code paths assume is
> well-formed.
> 
> Reject early in the validator:
> 
>   - mode bits outside S_IFMT|07777
>   - S_IFMT values that are not one of S_IFREG, S_IFDIR, S_IFLNK,
>     S_IFCHR, S_IFBLK, S_IFIFO, S_IFSOCK
> 
> mkfs.ocfs2 and the kernel only ever produce these seven types
> plus the standard permission, setuid/setgid/sticky bits; an
> on-disk i_mode outside this envelope is structurally malformed
> regardless of how it got there.
> 
> Validated against the existing inline_data, refcount, and
> chain-list checks: this hardening fires before any of them and
> does not perturb their behaviour for well-formed inodes.
> 
> 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

Looks fine.
Reviewed-by: Joseph Qi <[email protected]>

> ---
>  fs/ocfs2/inode.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
> index a510a0eb1adcc..fb592bf3e5f31 100644
> --- a/fs/ocfs2/inode.c
> +++ b/fs/ocfs2/inode.c
> @@ -1494,6 +1494,45 @@ int ocfs2_validate_inode_block(struct super_block *sb,
>  		goto bail;
>  	}
>  
> +	/*
> +	 * Reject dinodes whose i_mode does not name one of the seven
> +	 * canonical POSIX file types, or whose mode carries bits outside
> +	 * S_IFMT | 07777.  ocfs2_populate_inode() copies i_mode verbatim
> +	 * into inode->i_mode and then dispatches via switch (mode & S_IFMT)
> +	 * to file/dir/symlink/special_file iops; an unrecognised type
> +	 * falls into ocfs2_special_file_iops with init_special_inode(),
> +	 * which interprets i_rdev.  Constrain the type byte here so the
> +	 * dispatch only ever sees a value mkfs.ocfs2 / VFS can produce.
> +	 */
> +	{
> +		u16 mode = le16_to_cpu(di->i_mode);
> +
> +		if (mode & ~(S_IFMT | 07777)) {
> +			rc = ocfs2_error(sb,
> +					 "Invalid dinode #%llu: mode 0%o has bits outside S_IFMT|07777\n",
> +					 (unsigned long long)bh->b_blocknr,
> +					 mode);
> +			goto bail;
> +		}
> +
> +		switch (mode & S_IFMT) {
> +		case S_IFREG:
> +		case S_IFDIR:
> +		case S_IFLNK:
> +		case S_IFCHR:
> +		case S_IFBLK:
> +		case S_IFIFO:
> +		case S_IFSOCK:
> +			break;
> +		default:
> +			rc = ocfs2_error(sb,
> +					 "Invalid dinode #%llu: mode 0%o has unknown file type\n",
> +					 (unsigned long long)bh->b_blocknr,
> +					 mode);
> +			goto bail;
> +		}
> +	}
> +
>  	if (le16_to_cpu(di->i_dyn_features) & OCFS2_INLINE_DATA_FL) {
>  		struct ocfs2_inline_data *data = &di->id2.i_data;
>