Re: [PATCH] fs/ntfs3: bound attr_off in UpdateResidentValue against data_off

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

> In do_action()'s UpdateResidentValue case (fslog.c:3307),
> lrh->attr_off and lrh->redo_len come from the on-disk LRH.
> When they satisfy aoff + dlen < attr->res.data_off, the
> assignment
>
> 	attr->res.data_size = cpu_to_le32(aoff + dlen - data_off);
>
> underflows to ~4 GiB (e.g. 0xFFFFFFF9 when aoff=0x10, dlen=1,
> data_off=0x18).  Subsequent code that reads attr->res.data_size
> to walk the resident attribute payload would then read up to
> 4 GiB past the 1024-byte MFT record allocation.
>
> The existing mi_enum_attr() defense in fs/ntfs3/record.c:287
> catches the corrupted data_size on the next attribute walk
> and fails the mount, but only on the path that walks all
> attributes.  A read site that picks an attribute by name and
> reads its data_size without re-validating is not covered.
> Validate aoff against data_off and asize at the source.
>
> Reproduced under UML+KASAN on mainline 8d90b09e6741 via
> pr_warn-only probe: with aoff=0x10 and data_off=0x18, the
> post-assignment data_size is 0xfffffff9 (mount then fails
> at -22 from mi_enum_attr).
>
> 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 | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
> index acfa18b84401e..4977ac1f62027 100644
> --- a/fs/ntfs3/fslog.c
> +++ b/fs/ntfs3/fslog.c
> @@ -3325,6 +3325,17 @@ static int do_action(struct ntfs_log *log, struct OPEN_ATTR_ENRTY *oe,
>   		nsize = ALIGN(nsize, 8);
>   		data_off = le16_to_cpu(attr->res.data_off);
>   
> +		/*
> +		 * aoff comes from the on-disk lrh->attr_off.  Forbid
> +		 * writes that begin below the resident attribute's
> +		 * data_off (which would overwrite the resident header),
> +		 * and forbid aoff + dlen < data_off, which would make
> +		 * the data_size assignment below underflow to ~4 GiB.
> +		 */
> +		if (aoff < data_off || aoff + dlen < data_off ||
> +		    aoff + dlen > asize)
> +			goto dirty_vol;
> +
>   		if (nsize < asize) {
>   			memmove(Add2Ptr(attr, aoff), data, dlen);
>   			data = NULL; // To skip below memmove().

Hello,

The patch was applied, thank you.

Regards,
Konstantin