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

On 5/19/26 7:04 PM, Michael Bommarito wrote:
> ocfs2_validate_inode_block() currently accepts any non-zero i_mode value.
> 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; an unrecognised type falls through to ocfs2_special_file_iops and
> init_special_inode().
> 
> Reject dinodes whose type bits do not name one of the seven canonical
> POSIX file types.  Use fs_umode_to_ftype(), the same generic file-type
> conversion helper OCFS2 already uses for directory entries, so the
> accepted inode type set matches the kernel file-type vocabulary instead
> of open-coding a local switch.
> 
> Apply the same structural check to the online filecheck read path.
> filecheck keeps its own error namespace, so it reports malformed i_mode
> through the filecheck logger and OCFS2_FILECHECK_ERR_INVALIDINO instead
> of calling ocfs2_error(), but it must not allow a malformed dinode to
> proceed into ocfs2_populate_inode().
> 
> Fixes: b657c95c1108 ("ocfs2: Wrap inode block reads in a dedicated function.")
> Cc: [email protected]
> Link: https://sashiko.dev/#/patchset/20260517111015.3187935-1-michael.bommarito%40gmail.com
> Signed-off-by: Michael Bommarito <[email protected]>
> Assisted-by: Claude:claude-opus-4-7

Reviewed-by: Joseph Qi <[email protected]>
> ---
>  fs/ocfs2/inode.c | 36 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
> index a510a0eb1adcc..e149ccbdc03ce 100644
> --- a/fs/ocfs2/inode.c
> +++ b/fs/ocfs2/inode.c
> @@ -13,6 +13,7 @@
>  #include <linux/pagemap.h>
>  #include <linux/quotaops.h>
>  #include <linux/iversion.h>
> +#include <linux/fs_dirent.h>
>  
>  #include <asm/byteorder.h>
>  
> @@ -64,7 +65,12 @@ static int ocfs2_filecheck_read_inode_block_full(struct inode *inode,
>  static int ocfs2_filecheck_validate_inode_block(struct super_block *sb,
>  						struct buffer_head *bh);
>  static int ocfs2_filecheck_repair_inode_block(struct super_block *sb,
> -					      struct buffer_head *bh);
> +						      struct buffer_head *bh);
> +
> +static bool ocfs2_valid_inode_mode(umode_t mode)
> +{
> +	return fs_umode_to_ftype(mode) != FT_UNKNOWN;
> +}
>  
>  void ocfs2_set_inode_flags(struct inode *inode)
>  {
> @@ -1494,6 +1500,24 @@ 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.  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 here so the dispatch only ever sees a value mkfs.ocfs2 /
> +	 * VFS can produce.
> +	 */
> +	if (!ocfs2_valid_inode_mode(le16_to_cpu(di->i_mode))) {
> +		rc = ocfs2_error(sb,
> +				 "Invalid dinode #%llu: mode 0%o has unknown file type\n",
> +				 (unsigned long long)bh->b_blocknr,
> +				 le16_to_cpu(di->i_mode));
> +		goto bail;
> +	}
> +
>  	if (le16_to_cpu(di->i_dyn_features) & OCFS2_INLINE_DATA_FL) {
>  		struct ocfs2_inline_data *data = &di->id2.i_data;
>  
> @@ -1624,6 +1648,15 @@ static int ocfs2_filecheck_validate_inode_block(struct super_block *sb,
>  		     (unsigned long long)bh->b_blocknr,
>  		     le32_to_cpu(di->i_fs_generation));
>  		rc = -OCFS2_FILECHECK_ERR_GENERATION;
> +		goto bail;
> +	}
> +
> +	if (!ocfs2_valid_inode_mode(le16_to_cpu(di->i_mode))) {
> +		mlog(ML_ERROR,
> +		     "Filecheck: invalid dinode #%llu: mode 0%o has unknown file type\n",
> +		     (unsigned long long)bh->b_blocknr,
> +		     le16_to_cpu(di->i_mode));
> +		rc = -OCFS2_FILECHECK_ERR_INVALIDINO;
>  	}
>  
>  bail:
> @@ -1812,4 +1845,3 @@ const struct ocfs2_caching_operations ocfs2_inode_caching_ops = {
>  	.co_io_lock		= ocfs2_inode_cache_io_lock,
>  	.co_io_unlock		= ocfs2_inode_cache_io_unlock,
>  };
> -