[PATCH v2] ntfs: fix name offset validation in ntfs_non_resident_attr_value_is_valid

[PATCH v2] ntfs: fix name offset validation in ntfs_non_resident_attr_value_is_valid

[Hongling Zeng]

ntfs_attr_update_meta() performs memmove operations on attribute names
when converting between sparse and non-sparse attributes:

- Converting to sparse shifts the name forward by 8 bytes
  (name_offset + 8)
- Converting from sparse shifts the name backward by 8 bytes
  (name_offset - 8)

However, the validator does not check that name_offset is within safe
boundaries for these operations. A malicious MFT record could set
name_offset such that:

1. The name is positioned at the very end of a non-sparse attribute.
   Converting to sparse would shift the name forward by 8 bytes,
   writing beyond the attribute boundary.

2. The name overlaps with the mapping pairs, causing corruption during
   conversion.

Add validation to ensure:
- For named attributes, name_offset is within valid bounds
- Name does not extend beyond the attribute or overlap with mapping pairs
- For non-sparse attributes, name_end + 8 fits within attr_len to allow
  room for the forward shift when becoming sparse

Note: name_offset validation only applies when name_length != 0, as
unnamed attributes use name_offset = 0 which is valid.

Fixes: 7e2a1c554bc4 ("ntfs: Fix min_len for compressed/sparse attributes in ntfs_non_resident_attr_value_is_valid()")
Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
Changes in v2:
- Read name_length directly as an 8-bit field instead of using
  le16_to_cpu().
- Restrict the name forward-shift bounds check to the same attribute
  types that can actually be converted by ntfs_attr_update_meta().
---
 fs/ntfs/attrib.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index d354c3b0fae1..ffd08633970a 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -693,6 +693,9 @@ static bool ntfs_non_resident_attr_value_is_valid(const struct attr_record *a)
 	u32 attr_len;
 	u32 min_len;
 	u16 mp_offset;
+	u16 name_offset;
+	u8 name_len;
+	u32 name_end;
 
 	attr_len = le32_to_cpu(a->length);
 	min_len = offsetof(struct attr_record, data.non_resident.initialized_size) +
@@ -706,7 +709,35 @@ static bool ntfs_non_resident_attr_value_is_valid(const struct attr_record *a)
 		return false;
 
 	mp_offset = le16_to_cpu(a->data.non_resident.mapping_pairs_offset);
-	return mp_offset >= min_len && mp_offset <= attr_len;
+	if (mp_offset < min_len || mp_offset > attr_len)
+		return false;
+
+	/*
+	 * Validate name_offset for named attributes.
+	 * may be zero for unnamed attributes.
+	 */
+	name_len = a->name_length;
+	if (name_len) {
+		name_offset = le16_to_cpu(a->name_offset);
+
+		if (name_offset < min_len || name_offset >= attr_len)
+			return false;
+
+		name_end = name_offset + name_len * sizeof(__le16);
+		if (name_end > attr_len || name_end > mp_offset)
+			return false;
+
+		/*
+		 * A normal non-resident attribute moves its name forward when
+		 * it is converted to a sparse attribute.
+		 */
+		if (!(a->flags & (ATTR_IS_SPARSE | ATTR_COMPRESSION_MASK)) &&
+		     name_end + 8 > attr_len)
+			return false;
+	}
+
+	return true;
+
 }
 
 static bool ntfs_attr_value_is_valid(struct ntfs_volume *vol,
-- 
2.25.1

Re: [PATCH v2] ntfs: fix name offset validation in ntfs_non_resident_attr_value_is_valid

[Hongling Zeng]

Hi:

  Thank you for the review. You're absolutely right on both points:

   1. name_length is u8, not __le16 - my mistake.

   2. Moving the check outside the if (name_length) block is better as
      it covers both named and unnamed attributes.

   However, I have a concern about using ATTR_COMPRESSION_MASK here.

   In ntfs_attr_update_meta() line 3610, the actual conversion check is:

       if (sparse && !(a->flags & (ATTR_IS_SPARSE | ATTR_IS_COMPRESSED)))

   This uses ATTR_IS_COMPRESSED (0x0001), not ATTR_COMPRESSION_MASK (0x00ff).

   If a malicious MFT record sets flags = 0x0002:
   - Your check would skip (0x0002 & 0x00ff is true)
   - But ntfs_attr_update_meta() would still do the forward shift
     (0x0002 & 0x0001 is false)

   Should we use ATTR_IS_COMPRESSED instead to match the actual conversion
   predicate? Or is there another reason for the mask that I'm missing?

   Otherwise, your suggested approach of checking:
       attr_len - mp_offset < sizeof(a->data.non_resident.compressed_size)
   is much cleaner than checking name_end + 8.

在 2026年08月06日 09:39, Hongling Zeng 写道:
> ntfs_attr_update_meta() performs memmove operations on attribute names
> when converting between sparse and non-sparse attributes:
>
> - Converting to sparse shifts the name forward by 8 bytes
>    (name_offset + 8)
> - Converting from sparse shifts the name backward by 8 bytes
>    (name_offset - 8)
>
> However, the validator does not check that name_offset is within safe
> boundaries for these operations. A malicious MFT record could set
> name_offset such that:
>
> 1. The name is positioned at the very end of a non-sparse attribute.
>     Converting to sparse would shift the name forward by 8 bytes,
>     writing beyond the attribute boundary.
>
> 2. The name overlaps with the mapping pairs, causing corruption during
>     conversion.
>
> Add validation to ensure:
> - For named attributes, name_offset is within valid bounds
> - Name does not extend beyond the attribute or overlap with mapping pairs
> - For non-sparse attributes, name_end + 8 fits within attr_len to allow
>    room for the forward shift when becoming sparse
>
> Note: name_offset validation only applies when name_length != 0, as
> unnamed attributes use name_offset = 0 which is valid.
>
> Fixes: 7e2a1c554bc4 ("ntfs: Fix min_len for compressed/sparse attributes in ntfs_non_resident_attr_value_is_valid()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
> Changes in v2:
> - Read name_length directly as an 8-bit field instead of using
>    le16_to_cpu().
> - Restrict the name forward-shift bounds check to the same attribute
>    types that can actually be converted by ntfs_attr_update_meta().
> ---
>   fs/ntfs/attrib.c | 33 ++++++++++++++++++++++++++++++++-
>   1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
> index d354c3b0fae1..ffd08633970a 100644
> --- a/fs/ntfs/attrib.c
> +++ b/fs/ntfs/attrib.c
> @@ -693,6 +693,9 @@ static bool ntfs_non_resident_attr_value_is_valid(const struct attr_record *a)
>   	u32 attr_len;
>   	u32 min_len;
>   	u16 mp_offset;
> +	u16 name_offset;
> +	u8 name_len;
> +	u32 name_end;
>   
>   	attr_len = le32_to_cpu(a->length);
>   	min_len = offsetof(struct attr_record, data.non_resident.initialized_size) +
> @@ -706,7 +709,35 @@ static bool ntfs_non_resident_attr_value_is_valid(const struct attr_record *a)
>   		return false;
>   
>   	mp_offset = le16_to_cpu(a->data.non_resident.mapping_pairs_offset);
> -	return mp_offset >= min_len && mp_offset <= attr_len;
> +	if (mp_offset < min_len || mp_offset > attr_len)
> +		return false;
> +
> +	/*
> +	 * Validate name_offset for named attributes.
> +	 * may be zero for unnamed attributes.
> +	 */
> +	name_len = a->name_length;
> +	if (name_len) {
> +		name_offset = le16_to_cpu(a->name_offset);
> +
> +		if (name_offset < min_len || name_offset >= attr_len)
> +			return false;
> +
> +		name_end = name_offset + name_len * sizeof(__le16);
> +		if (name_end > attr_len || name_end > mp_offset)
> +			return false;
> +
> +		/*
> +		 * A normal non-resident attribute moves its name forward when
> +		 * it is converted to a sparse attribute.
> +		 */
> +		if (!(a->flags & (ATTR_IS_SPARSE | ATTR_COMPRESSION_MASK)) &&
> +		     name_end + 8 > attr_len)
> +			return false;
> +	}
> +
> +	return true;
> +
>   }
>   
>   static bool ntfs_attr_value_is_valid(struct ntfs_volume *vol,

Re: [PATCH v2] ntfs: fix name offset validation in ntfs_non_resident_attr_value_is_valid

[Namjae Jeon]

On Thu, Aug 6, 2026 at 11:39 AM Hongling Zeng <zhongling0719@126.com> wrote:
>
> Hi:
>
>   Thank you for the review. You're absolutely right on both points:
>
>    1. name_length is u8, not __le16 - my mistake.
>
>    2. Moving the check outside the if (name_length) block is better as
>       it covers both named and unnamed attributes.
>
>    However, I have a concern about using ATTR_COMPRESSION_MASK here.
>
>    In ntfs_attr_update_meta() line 3610, the actual conversion check is:
>
>        if (sparse && !(a->flags & (ATTR_IS_SPARSE | ATTR_IS_COMPRESSED)))
>
>    This uses ATTR_IS_COMPRESSED (0x0001), not ATTR_COMPRESSION_MASK (0x00ff).
>
>    If a malicious MFT record sets flags = 0x0002:
>    - Your check would skip (0x0002 & 0x00ff is true)
>    - But ntfs_attr_update_meta() would still do the forward shift
>      (0x0002 & 0x0001 is false)
>
>    Should we use ATTR_IS_COMPRESSED instead to match the actual conversion
>    predicate? Or is there another reason for the mask that I'm missing?
You're right. Can you send v3 patch after updating it ?
Thanks!