Re: [PATCH] fs/ntfs3: bound DeleteIndexEntryAllocation memmove length

On 5/15/26 18:33, Michael Bommarito wrote:

> In do_action()'s DeleteIndexEntryAllocation case, e->size comes
> from an on-disk INDEX_BUFFER entry.  When e->size makes
> e + e->size point past hdr + hdr->used,
> PtrOffset(e1, Add2Ptr(hdr, used)) returns a negative ptrdiff_t
> that is silently cast to a quasi-infinite size_t when passed
> to memmove().  The memmove then walks past the destination
> buffer.
>
> The sibling DeleteIndexEntryRoot case at fslog.c:3540-3543
> already carries the corresponding guard:
>
> 	if (PtrOffset(e1, Add2Ptr(hdr, used)) < esize ||
> 	    Add2Ptr(e, esize) > Add2Ptr(lrh, rec_len) ||
> 	    used + esize > le32_to_cpu(hdr->total)) {
> 		goto dirty_vol;
> 	}
>
> Apply the same shape to the allocation-path case.  Also reject
> esize == 0: memmove(e, e, ...) is a no-op and leaves
> hdr->used unchanged, hiding a malformed entry from the
> existing check_index_header() walk.
>
> Reproduced under UML+KASAN on mainline 8d90b09e6741 by
> mounting a crafted NTFS image: the unguarded memmove takes a
> length of 0xffffffffffffff00 and the kernel oopses in
> memmove+0x81/0x1a0 on the do_action+0x36a2 frame.
>
> Fixes: b46acd6a6a62 ("fs/ntfs3: Add NTFS journal")
> Cc: [email protected]
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Michael Bommarito <[email protected]>
> ---
>   fs/ntfs3/fslog.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
> index acfa18b84401e..4b32d08441b8d 100644
> --- a/fs/ntfs3/fslog.c
> +++ b/fs/ntfs3/fslog.c
> @@ -3570,9 +3570,24 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
>   		}
>   
>   		e1 = Add2Ptr(e, esize);
> -		nsize = esize;
>   		used = le32_to_cpu(hdr->used);
>   
> +		/*
> +		 * Reject crafted entries whose e->size makes e + esize
> +		 * point past the INDEX_HDR's used boundary.  Without this,
> +		 * PtrOffset(e1, hdr + used) underflows to a quasi-infinite
> +		 * size_t when fed to the memmove() below.
> +		 *
> +		 * Also reject esize == 0: memmove(e, e, ...) is a no-op and
> +		 * leaves hdr->used unchanged, masking the crafted entry.
> +		 */
> +		if (!esize ||
> +		    Add2Ptr(e, esize) > Add2Ptr(hdr, used) ||
> +		    PtrOffset(e1, Add2Ptr(hdr, used)) < esize)
> +			goto dirty_vol;
> +
> +		nsize = esize;
> +
>   		memmove(e, e1, PtrOffset(e1, Add2Ptr(hdr, used)));
>   
>   		hdr->used = cpu_to_le32(used - nsize);

Hello,

The patch was applied, thank you.

Regards,
Konstantin