Re: [PATCH v2 6/6] ntfs: reject non-resident attributes whose sizes exceed their allocation

[Hyunchul Lee]

On Sun, Sep 27, 2026 at 01:08:25PM +0800, Matthias Goergens wrote:
> ntfs_read_locked_inode(), ntfs_read_locked_attr_inode() and
> ntfs_read_locked_index_inode() take a non-resident attribute's data_size
> and initialized_size from disk without checking them against its
> allocated_size.  The runlist ends at allocated_size and the read path
> maps what lies beyond it as a hole, so a data_size larger than the
> allocation makes reads return zeros that are not on disk, with no error.
> 
> On a crafted volume with 4 KiB clusters where a 6144000-byte file claims
> a data_size and initialized_size 64 KiB beyond its allocation, reading
> the file returns 16 clusters of such zeros.  A sparse file behaves the
> same.  A compressed file instead fails with -EIO after a burst of "Still
> have pages left!" errors from ntfs_read_compressed_block().
> 
> Require 0 <= initialized_size <= data_size <= allocated_size, with
> allocated_size a multiple of the cluster size, when the inode is read,
> as fs/ntfs3 does in mi_enum_attr(), and fail with -EIO otherwise, which
> marks the inode bad and the volume as having errors.  initialized_size
> above data_size is rejected too because an extending write zeroes the
> gap it opens only from initialized_size onwards.  An unaligned
> allocated_size ends inside a cluster that the read path treats as past
> the end: a crafted 66440-byte file with 4 KiB clusters reads its last
> 904 bytes as zeros.
> 
> Sparse and compressed attributes need no exception.  Every non-resident
> attribute has a cluster-aligned allocated_size >= data_size, holes
> included, on eight public test volumes (seven written by Windows, with
> LZNT1-compressed files, a sparse $UsnJrnl:$J and a OneDrive placeholder
> whose unnamed $DATA is one hole, and one by mkntfs), on a volume written
> by ntfs-3g and on one written by this driver, and the patched driver
> reads every file on them as before.  fs/ntfs3 has enforced the same
> checks since v6.6, also without exceptions.  The layout.h comment saying
> that data_size can exceed allocated_size for compressed and sparse
> attributes is corrected.
> 
> This also covers a non-resident attribute list, which
> load_attribute_list() reads through ntfs_attr_iget(), and $MFT, whose
> inode goes through ntfs_read_locked_inode() after the previous patch's
> check.  The check was already missing in the classic driver; the Fixes
> tag names the commit that brought that code back.
> 
> Fixes: 1e9ea7e04472 ("Revert "fs: Remove NTFS classic"")
> Signed-off-by: Matthias Goergens <matthias.goergens@xxxxxxxxx>
> ---
>  fs/ntfs/inode.c  | 38 ++++++++++++++++++++++++++++++++++++++
>  fs/ntfs/layout.h | 13 ++++++++-----
>  2 files changed, 46 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
> index 9c97fc3f9e5ff..126c50b5a349e 100644
> --- a/fs/ntfs/inode.c
> +++ b/fs/ntfs/inode.c
> @@ -651,6 +651,38 @@ void ntfs_set_vfs_operations(struct inode *inode, mode_t mode, dev_t dev)
>  	}
>  }
>  
> +/*
> + * The clusters of a non-resident attribute end at its allocated size.  Past
> + * that there is nothing on disk to read, and past the initialized size reads
> + * return zeros and writes zero the gap they open, so the sizes must satisfy
> + * 0 <= initialized_size <= data_size <= allocated_size, with allocated_size
> + * a multiple of the cluster size.
> + *
> + * Sparse and compressed attributes get no exception.  Windows, ntfs-3g and
> + * this driver all count holes in allocated_size (the clusters actually in use
> + * are in compressed_size), including for streams that are entirely a hole,
> + * and fs/ntfs3 has applied the same check to every non-resident attribute in
> + * mi_enum_attr() since v6.6.
> + */

Could you remove the comment above. The comment in layout.h already
document these size relationship sufficiently, so I think that the
duplicated explanation is unnecessary.

> +static bool ntfs_non_resident_sizes_inconsistent(struct inode *vi,
> +						 const struct attr_record *a)
> +{
> +	s64 allocated_size = le64_to_cpu(a->data.non_resident.allocated_size);
> +	s64 data_size = le64_to_cpu(a->data.non_resident.data_size);
> +	s64 initialized_size = le64_to_cpu(a->data.non_resident.initialized_size);
> +
> +	if (initialized_size >= 0 && initialized_size <= data_size &&
> +	    data_size <= allocated_size &&
> +	    !ntfs_bytes_to_cluster_off(NTFS_I(vi)->vol, allocated_size))
> +		return false;
> +
> +	ntfs_error(vi->i_sb,
> +		   "Attribute 0x%x of inode 0x%llx is corrupt (initialized size %lld, data size %lld, allocated size %lld).",
> +		   le32_to_cpu(a->type), NTFS_I(vi)->mft_no, initialized_size,
> +		   data_size, allocated_size);
> +	return true;
> +}
> +
>  /*
>   * ntfs_read_locked_inode - read an inode from its device
>   * @vi:		inode to read
> @@ -1184,6 +1216,8 @@ static int ntfs_read_locked_inode(struct inode *vi)
>  					"First extent of $DATA attribute has non zero lowest_vcn.");
>  				goto unm_err_out;
>  			}
> +			if (ntfs_non_resident_sizes_inconsistent(vi, a))
> +				goto unm_err_out;
>  			vi->i_size = ni->data_size = le64_to_cpu(a->data.non_resident.data_size);
>  			ni->initialized_size = le64_to_cpu(a->data.non_resident.initialized_size);
>  			ni->allocated_size = le64_to_cpu(a->data.non_resident.allocated_size);
> @@ -1446,6 +1480,8 @@ static int ntfs_read_locked_attr_inode(struct inode *base_vi, struct inode *vi)
>  			ntfs_error(vi->i_sb, "First extent of attribute has non-zero lowest_vcn.");
>  			goto unm_err_out;
>  		}
> +		if (ntfs_non_resident_sizes_inconsistent(vi, a))
> +			goto unm_err_out;
>  		vi->i_size = ni->data_size = le64_to_cpu(a->data.non_resident.data_size);
>  		ni->initialized_size = le64_to_cpu(a->data.non_resident.initialized_size);
>  		ni->allocated_size = le64_to_cpu(a->data.non_resident.allocated_size);
> @@ -1675,6 +1711,8 @@ static int ntfs_read_locked_index_inode(struct inode *base_vi, struct inode *vi)
>  			"First extent of $INDEX_ALLOCATION attribute has non zero lowest_vcn.");
>  		goto unm_err_out;
>  	}
> +	if (ntfs_non_resident_sizes_inconsistent(vi, a))
> +		goto unm_err_out;
>  	vi->i_size = ni->data_size = le64_to_cpu(a->data.non_resident.data_size);
>  	ni->initialized_size = le64_to_cpu(a->data.non_resident.initialized_size);
>  	ni->allocated_size = le64_to_cpu(a->data.non_resident.allocated_size);
> diff --git a/fs/ntfs/layout.h b/fs/ntfs/layout.h
> index 8f5792139d719..2de83d4ca40b9 100644
> --- a/fs/ntfs/layout.h
> +++ b/fs/ntfs/layout.h
> @@ -811,14 +811,17 @@ enum {
>   *                                  on XP SP2+.
>   * @data.non_resident.reserved:     5 bytes for 8-byte alignment.
>   * @data.non_resident.allocated_size:
> - *                                  Allocated disk space in bytes.
> - *                                  For compressed: logical allocated size.
> + *                                  Allocated size in bytes, a multiple of
> + *                                  the cluster size.  For compressed and
> + *                                  sparse attributes holes count as
> + *                                  allocated; the clusters actually in use
> + *                                  are in compressed_size.
>   * @data.non_resident.data_size:    Logical attribute value size in bytes.
> - *                                  Can be larger than allocated_size if
> - *                                  compressed/sparse.
> + *                                  Never larger than allocated_size, also
> + *                                  when compressed/sparse.
>   * @data.non_resident.initialized_size:
>   *                                  Initialized portion size in bytes.
> - *                                  Usually equals data_size.
> + *                                  Usually equals data_size, never larger.
>   * @data.non_resident.compressed_size:
>   *                                  Compressed on-disk size in bytes.
>   *                                  Only present when compressed or sparse.
> -- 
> 2.55.0
> 

-- 
Thanks,
Hyunchul