mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Joseph Qi <joseph.qi@linux.alibaba.com>
To: Michael Bommarito <michael.bommarito@gmail.com>,
	akpm <akpm@linux-foundation.org>
Cc: ZhengYuan Huang <gality369@gmail.com>,
	ocfs2-devel@lists.linux.dev, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, Mark Fasheh <mark@fasheh.com>,
	Joel Becker <jlbec@evilplan.org>
Subject: Re: [PATCH 1/3] ocfs2: reject dinodes with non-canonical i_mode type or stray bits
Date: Mon, 18 May 2026 09:36:17 +0800	[thread overview]
Message-ID: <318beeb0-0f44-4d39-b2d3-dd0dd570ef0f@linux.alibaba.com> (raw)
In-Reply-To: <20260517111015.3187935-2-michael.bommarito@gmail.com>



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: stable@vger.kernel.org
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> Assisted-by: Claude:claude-opus-4-7

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

> ---
>  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;
>  


  reply	other threads:[~2026-05-18  1:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-17 11:10 [PATCH 0/3] ocfs2: harden inode validators against forged metadata Michael Bommarito
2026-05-17 11:10 ` [PATCH 1/3] ocfs2: reject dinodes with non-canonical i_mode type or stray bits Michael Bommarito
2026-05-18  1:36   ` Joseph Qi [this message]
2026-05-17 11:10 ` [PATCH 2/3] ocfs2: reject dinodes whose i_rdev disagrees with the file type Michael Bommarito
2026-05-18  1:37   ` Joseph Qi
2026-05-17 11:10 ` [PATCH 3/3] ocfs2: reject regular files with non-zero i_size and zero i_clusters Michael Bommarito
2026-05-18  1:38   ` Joseph Qi
2026-05-18 21:40 ` [PATCH 0/3] ocfs2: harden inode validators against forged metadata Andrew Morton
2026-05-19  0:57   ` Michael Bommarito
2026-06-01 17:39 ` Joel Becker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=318beeb0-0f44-4d39-b2d3-dd0dd570ef0f@linux.alibaba.com \
    --to=joseph.qi@linux.alibaba.com \
    --cc=akpm@linux-foundation.org \
    --cc=gality369@gmail.com \
    --cc=jlbec@evilplan.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark@fasheh.com \
    --cc=michael.bommarito@gmail.com \
    --cc=ocfs2-devel@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome