public inbox for ntfs3@lists.linux.dev
 help / color / mirror / Atom feed
From: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
To: Michael Bommarito <michael.bommarito@gmail.com>
Cc: <ntfs3@lists.linux.dev>, <linux-fsdevel@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <stable@vger.kernel.org>
Subject: Re: [PATCH] ntfs3: bound to_move in indx_insert_into_root before hdr_insert_head
Date: Thu, 30 Apr 2026 14:00:01 +0200	[thread overview]
Message-ID: <4435a979-e5cc-4b1d-9a6f-764bc48c2106@paragon-software.com> (raw)
In-Reply-To: <20260417233305.1787096-1-michael.bommarito@gmail.com>

On 4/18/26 01:33, Michael Bommarito wrote:

> indx_insert_into_root() promotes a full resident $INDEX_ROOT into
> $INDEX_ALLOCATION and copies all non-last resident root entries into
> a newly allocated INDEX_BUFFER via hdr_insert_head(). The source
> byte count 'to_move' is summed from the on-disk resident entry sizes
> and is independent of the destination buffer size, which comes from
> root->index_block_size (via indx->index_bits).
>
> A crafted NTFS image that keeps a valid, full resident root but
> shrinks root->index_block_size down to 512 after the root has been
> populated makes hdr_insert_head() memcpy attacker-controlled resident
> entry bytes past the end of the kmalloc(1u << indx->index_bits)
> allocation returned by indx_new(). For a 512-byte destination and a
> resident root whose non-last entries total 560 bytes, the memcpy
> overruns by 120 bytes and a following memmove extends the highest
> written offset to 136 bytes past the allocation. The overflow bytes
> are a direct copy of on-disk entries (via kmemdup), so they are
> fully attacker-controlled.
>
> The write is reachable from unprivileged open(O_CREAT) on a mounted
> crafted NTFS image: a single sufficiently long create in a directory
> whose resident root is already full forces root promotion and
> triggers the copy.
>
> This is a controlled out-of-bounds write of 120-136 bytes past a
> kmalloc(index_block_size) allocation, with attacker-controlled
> content. It is a bounded adjacent-heap corruption primitive; it is
> not an arbitrary-address write. Successful exploitation into a named
> victim object depends on the surrounding slab layout.
>
> Reject the copy at the sink. The destination's INDEX_HDR already
> reports hdr_total (the payload capacity of the new buffer) and
> hdr_used (the bytes already consumed by the terminal END entry
> installed by indx_new()); require that to_move fits in the remaining
> payload before calling hdr_insert_head(). On mismatch, fail with
> -EINVAL and mark the filesystem as having a detected on-disk
> inconsistency, which is the same behaviour as the surrounding
> validation in this function.
>
> Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
>
>   - FYI, like the sp_size patch, I have a larger refactor that might
>     make this easier to avoid long term.  It's a mount-time variant
>     that adds the cross-check between root->index_block_size and
>     the resident root attribute size to indx_init() instead of the
>     sink, closing the whole "root entries do not fit declared
>     index_block_size" class for any future caller that reaches
>     hdr_insert_head from elsewhere.  Happy to send it as v2 if
>     you prefer the wider change;  otherwise, this minimal guard is
>     scoped to the minimal memcpy overrun site and is easier to
>     backport.
>
>   fs/ntfs3/index.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
>
> diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
> index 2c43e7c27861..b7633b721d19 100644
> --- a/fs/ntfs3/index.c
> +++ b/fs/ntfs3/index.c
> @@ -1740,6 +1740,22 @@ static int indx_insert_into_root(struct ntfs_index *indx, struct ntfs_inode *ni,
>   	hdr_used = le32_to_cpu(hdr->used);
>   	hdr_total = le32_to_cpu(hdr->total);
>   
> +	/*
> +	 * The destination INDEX_BUFFER has 'hdr_total' bytes of payload
> +	 * available after the header, of which 'hdr_used' are already
> +	 * consumed by the single terminal END entry installed by
> +	 * indx_new(). A crafted image can present a resident root whose
> +	 * non-last entries (summing to 'to_move') exceed what fits in
> +	 * this buffer; copying them unchecked would overrun the
> +	 * kmalloc(1u << indx->index_bits) allocation backing the new
> +	 * buffer. Reject the copy in that case.
> +	 */
> +	if (to_move > hdr_total - hdr_used) {
> +		err = -EINVAL;
> +		ntfs_set_state(sbi, NTFS_DIRTY_ERROR);
> +		goto out_put_n;
> +	}
> +
>   	/* Copy root entries into new buffer. */
>   	hdr_insert_head(hdr, re, to_move);
>   

Hello,

Sorry for the delay.
Your patch is queued for the next merge window, thanks.

Regards,
Konstantin


      reply	other threads:[~2026-04-30 12:00 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-17 23:33 [PATCH] ntfs3: bound to_move in indx_insert_into_root before hdr_insert_head Michael Bommarito
2026-04-30 12:00 ` Konstantin Komarov [this message]

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=4435a979-e5cc-4b1d-9a6f-764bc48c2106@paragon-software.com \
    --to=almaz.alexandrovich@paragon-software.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael.bommarito@gmail.com \
    --cc=ntfs3@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    /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