[PATCH] ntfs: fix memmove overlap in ntfs_new_attr_flags

[PATCH] ntfs: fix memmove overlap in ntfs_new_attr_flags

[Hongling Zeng]

When the record shrinks while the payload offsets increase (e.g., enabling
compression reduces padding, making arec_size < old_arec_size, but the header
grows by 8 bytes), moving the name first can overwrite the old mapping_pairs
before they are copied. Move mapping_pairs first in this case.

Since mp_ofs is derived from name_ofs, they always change in the same
direction. Checking name_ofs alone is sufficient.

Fixes: fc053f05ca28 ("ntfs: add reparse and ea operations")
Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/ntfs/ea.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/fs/ntfs/ea.c b/fs/ntfs/ea.c
index 534f7efaf128..c836d33ab0d3 100644
--- a/fs/ntfs/ea.c
+++ b/fs/ntfs/ea.c
@@ -729,15 +729,36 @@ static int ntfs_new_attr_flags(struct ntfs_inode *ni, __le32 fattr)
 	old_arec_size = le32_to_cpu(a->length);
 
 	/*
-	 * Move payloads before shrinking the record.  Otherwise resizing moves
+	 * Move payloads before shrinking the record. Otherwise resizing moves
 	 * the following attribute over the old payload before it can be copied.
+	 *
+	 * When offsets increase, move mapping_pairs first to avoid name
+	 * overwriting the start of mapping_pairs.
 	 */
 	if (arec_size < old_arec_size) {
-		if (a->name_length && name_ofs != old_name_ofs)
-			memmove((u8 *)a + name_ofs, (u8 *)a + old_name_ofs,
-				a->name_length * sizeof(__le16));
-		if (mp_ofs != old_mp_ofs)
-			memmove((u8 *)a + mp_ofs, (u8 *)a + old_mp_ofs, mp_size);
+		if (name_ofs > old_name_ofs) {
+			/* Payload offsets increased: move mapping pairs first. */
+			if (mp_ofs != old_mp_ofs)
+				memmove((u8 *)a + mp_ofs,
+						(u8 *)a + old_mp_ofs,
+						mp_size);
+			if (a->name_length && name_ofs != old_name_ofs)
+				memmove((u8 *)a + name_ofs,
+						(u8 *)a + old_name_ofs,
+						a->name_length *
+							sizeof(__le16));
+		} else {
+			/* Payload offsets decreased or unchanged: move name first. */
+			if (a->name_length && name_ofs != old_name_ofs)
+				memmove((u8 *)a + name_ofs,
+						(u8 *)a + old_name_ofs,
+						a->name_length *
+							sizeof(__le16));
+			if (mp_ofs != old_mp_ofs)
+				memmove((u8 *)a + mp_ofs,
+						(u8 *)a + old_mp_ofs,
+						mp_size);
+		}
 	}
 
 	err = ntfs_attr_record_resize(ctx->mrec, a, arec_size);
-- 
2.25.1

Re: [PATCH] ntfs: fix memmove overlap in ntfs_new_attr_flags

[Hyunchul Lee]

Hi Hongling,

2026년 8월 24일 (월) 오후 4:59, Hongling Zeng <zenghongling@kylinos.cn>님이 작성:
>
> When the record shrinks while the payload offsets increase (e.g., enabling
> compression reduces padding, making arec_size < old_arec_size, but the header
> grows by 8 bytes), moving the name first can overwrite the old mapping_pairs
> before they are copied. Move mapping_pairs first in this case.

Can this situation occur even when
it is not a crafted image?

>
> Since mp_ofs is derived from name_ofs, they always change in the same
> direction. Checking name_ofs alone is sufficient.
>
> Fixes: fc053f05ca28 ("ntfs: add reparse and ea operations")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>  fs/ntfs/ea.c | 33 +++++++++++++++++++++++++++------
>  1 file changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ntfs/ea.c b/fs/ntfs/ea.c
> index 534f7efaf128..c836d33ab0d3 100644
> --- a/fs/ntfs/ea.c
> +++ b/fs/ntfs/ea.c
> @@ -729,15 +729,36 @@ static int ntfs_new_attr_flags(struct ntfs_inode *ni, __le32 fattr)
>         old_arec_size = le32_to_cpu(a->length);
>
>         /*
> -        * Move payloads before shrinking the record.  Otherwise resizing moves
> +        * Move payloads before shrinking the record. Otherwise resizing moves
>          * the following attribute over the old payload before it can be copied.
> +        *
> +        * When offsets increase, move mapping_pairs first to avoid name
> +        * overwriting the start of mapping_pairs.
>          */
>         if (arec_size < old_arec_size) {
> -               if (a->name_length && name_ofs != old_name_ofs)
> -                       memmove((u8 *)a + name_ofs, (u8 *)a + old_name_ofs,
> -                               a->name_length * sizeof(__le16));
> -               if (mp_ofs != old_mp_ofs)
> -                       memmove((u8 *)a + mp_ofs, (u8 *)a + old_mp_ofs, mp_size);
> +               if (name_ofs > old_name_ofs) {
> +                       /* Payload offsets increased: move mapping pairs first. */
> +                       if (mp_ofs != old_mp_ofs)
> +                               memmove((u8 *)a + mp_ofs,
> +                                               (u8 *)a + old_mp_ofs,
> +                                               mp_size);
> +                       if (a->name_length && name_ofs != old_name_ofs)
> +                               memmove((u8 *)a + name_ofs,
> +                                               (u8 *)a + old_name_ofs,
> +                                               a->name_length *
> +                                                       sizeof(__le16));
> +               } else {
> +                       /* Payload offsets decreased or unchanged: move name first. */
> +                       if (a->name_length && name_ofs != old_name_ofs)
> +                               memmove((u8 *)a + name_ofs,
> +                                               (u8 *)a + old_name_ofs,
> +                                               a->name_length *
> +                                                       sizeof(__le16));
> +                       if (mp_ofs != old_mp_ofs)
> +                               memmove((u8 *)a + mp_ofs,
> +                                               (u8 *)a + old_mp_ofs,
> +                                               mp_size);
> +               }
>         }
>
>         err = ntfs_attr_record_resize(ctx->mrec, a, arec_size);
> --
> 2.25.1
>


-- 
Thanks,
Hyunchul

Re: [PATCH] ntfs: fix memmove overlap in ntfs_new_attr_flags

[Hongling Zeng]

在 2026年08月25日 13:45, Hyunchul Lee 写道:
> Hi Hongling,
>
> 2026년 8월 24일 (월) 오후 4:59, Hongling Zeng <zenghongling@kylinos.cn>님이 작성:
>> When the record shrinks while the payload offsets increase (e.g., enabling
>> compression reduces padding, making arec_size < old_arec_size, but the header
>> grows by 8 bytes), moving the name first can overwrite the old mapping_pairs
>> before they are copied. Move mapping_pairs first in this case.
> Can this situation occur even when
> it is not a crafted image?
Hi Hyunchul

   Yes. This can occur during normal operations when modifying 
system.ntfs_attrib
   on a file with a named non-resident attribute. The header grows 
(adding the
   compressed_size field) while the total record shrinks (reduced padding),
   causing name_ofs and mp_ofs to increase and creating the memmove overlap.

   No crafted image is required - a valid NTFS filesystem with the right
   attribute layout will trigger this path.

   Thanks for the review.

>> Since mp_ofs is derived from name_ofs, they always change in the same
>> direction. Checking name_ofs alone is sufficient.
>>
>> Fixes: fc053f05ca28 ("ntfs: add reparse and ea operations")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>> ---
>>   fs/ntfs/ea.c | 33 +++++++++++++++++++++++++++------
>>   1 file changed, 27 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/ntfs/ea.c b/fs/ntfs/ea.c
>> index 534f7efaf128..c836d33ab0d3 100644
>> --- a/fs/ntfs/ea.c
>> +++ b/fs/ntfs/ea.c
>> @@ -729,15 +729,36 @@ static int ntfs_new_attr_flags(struct ntfs_inode *ni, __le32 fattr)
>>          old_arec_size = le32_to_cpu(a->length);
>>
>>          /*
>> -        * Move payloads before shrinking the record.  Otherwise resizing moves
>> +        * Move payloads before shrinking the record. Otherwise resizing moves
>>           * the following attribute over the old payload before it can be copied.
>> +        *
>> +        * When offsets increase, move mapping_pairs first to avoid name
>> +        * overwriting the start of mapping_pairs.
>>           */
>>          if (arec_size < old_arec_size) {
>> -               if (a->name_length && name_ofs != old_name_ofs)
>> -                       memmove((u8 *)a + name_ofs, (u8 *)a + old_name_ofs,
>> -                               a->name_length * sizeof(__le16));
>> -               if (mp_ofs != old_mp_ofs)
>> -                       memmove((u8 *)a + mp_ofs, (u8 *)a + old_mp_ofs, mp_size);
>> +               if (name_ofs > old_name_ofs) {
>> +                       /* Payload offsets increased: move mapping pairs first. */
>> +                       if (mp_ofs != old_mp_ofs)
>> +                               memmove((u8 *)a + mp_ofs,
>> +                                               (u8 *)a + old_mp_ofs,
>> +                                               mp_size);
>> +                       if (a->name_length && name_ofs != old_name_ofs)
>> +                               memmove((u8 *)a + name_ofs,
>> +                                               (u8 *)a + old_name_ofs,
>> +                                               a->name_length *
>> +                                                       sizeof(__le16));
>> +               } else {
>> +                       /* Payload offsets decreased or unchanged: move name first. */
>> +                       if (a->name_length && name_ofs != old_name_ofs)
>> +                               memmove((u8 *)a + name_ofs,
>> +                                               (u8 *)a + old_name_ofs,
>> +                                               a->name_length *
>> +                                                       sizeof(__le16));
>> +                       if (mp_ofs != old_mp_ofs)
>> +                               memmove((u8 *)a + mp_ofs,
>> +                                               (u8 *)a + old_mp_ofs,
>> +                                               mp_size);
>> +               }
>>          }
>>
>>          err = ntfs_attr_record_resize(ctx->mrec, a, arec_size);
>> --
>> 2.25.1
>>
>

Re: [PATCH] ntfs: fix memmove overlap in ntfs_new_attr_flags

[Hyunchul Lee]

2026년 8월 25일 (화) 오후 3:36, Hongling Zeng <zhongling0719@126.com>님이 작성:
>
>
> 在 2026年08月25日 13:45, Hyunchul Lee 写道:
> > Hi Hongling,
> >
> > 2026년 8월 24일 (월) 오후 4:59, Hongling Zeng <zenghongling@kylinos.cn>님이 작성:
> >> When the record shrinks while the payload offsets increase (e.g., enabling
> >> compression reduces padding, making arec_size < old_arec_size, but the header
> >> grows by 8 bytes), moving the name first can overwrite the old mapping_pairs
> >> before they are copied. Move mapping_pairs first in this case.
> > Can this situation occur even when
> > it is not a crafted image?
> Hi Hyunchul
>
>    Yes. This can occur during normal operations when modifying
> system.ntfs_attrib
>    on a file with a named non-resident attribute. The header grows
> (adding the
>    compressed_size field) while the total record shrinks (reduced padding),
>    causing name_ofs and mp_ofs to increase and creating the memmove overlap.
>
>    No crafted image is required - a valid NTFS filesystem with the right
>    attribute layout will trigger this path.

What I am wondering was whether there had been
cases where such padding existed.
I has been considering where we should guard
against that case.

This patch looks good to me.

Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>

Re: [PATCH] ntfs: fix memmove overlap in ntfs_new_attr_flags

[Namjae Jeon]

On Mon, Aug 24, 2026 at 4:59 PM Hongling Zeng <zenghongling@kylinos.cn> wrote:
>
> When the record shrinks while the payload offsets increase (e.g., enabling
> compression reduces padding, making arec_size < old_arec_size, but the header
> grows by 8 bytes), moving the name first can overwrite the old mapping_pairs
> before they are copied. Move mapping_pairs first in this case.
>
> Since mp_ofs is derived from name_ofs, they always change in the same
> direction. Checking name_ofs alone is sufficient.
>
> Fixes: fc053f05ca28 ("ntfs: add reparse and ea operations")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
Applied it to #ntfs-next.
Thanks!

[PATCH] ntfs: fix memmove overlap in ntfs_new_attr_flags

[Hongling Zeng]

When the record shrinks while the payload offsets increase (e.g., enabling
compression reduces padding, making arec_size < old_arec_size, but the header
grows by 8 bytes), moving the name first can overwrite the old mapping_pairs
before they are copied. Move mapping_pairs first in this case.

Since mp_ofs is derived from name_ofs, they always change in the same
direction. Checking name_ofs alone is sufficient.

Fixes: fc053f05ca28 ("ntfs: add reparse and ea operations")
Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/ntfs/ea.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/fs/ntfs/ea.c b/fs/ntfs/ea.c
index cdd306933d73..61ca945ce5bf 100644
--- a/fs/ntfs/ea.c
+++ b/fs/ntfs/ea.c
@@ -753,15 +753,36 @@ static int ntfs_new_attr_flags(struct ntfs_inode *ni, __le32 fattr)
 	old_arec_size = le32_to_cpu(a->length);
 
 	/*
-	 * Move payloads before shrinking the record.  Otherwise resizing moves
+	 * Move payloads before shrinking the record. Otherwise resizing moves
 	 * the following attribute over the old payload before it can be copied.
+	 *
+	 * When offsets increase, move mapping_pairs first to avoid name
+	 * overwriting the start of mapping_pairs.
 	 */
 	if (arec_size < old_arec_size) {
-		if (a->name_length && name_ofs != old_name_ofs)
-			memmove((u8 *)a + name_ofs, (u8 *)a + old_name_ofs,
-				a->name_length * sizeof(__le16));
-		if (mp_ofs != old_mp_ofs)
-			memmove((u8 *)a + mp_ofs, (u8 *)a + old_mp_ofs, mp_size);
+		if (name_ofs > old_name_ofs) {
+			/* Payload offsets increased: move mapping pairs first. */
+			if (mp_ofs != old_mp_ofs)
+				memmove((u8 *)a + mp_ofs,
+						(u8 *)a + old_mp_ofs,
+						mp_size);
+			if (a->name_length && name_ofs != old_name_ofs)
+				memmove((u8 *)a + name_ofs,
+						(u8 *)a + old_name_ofs,
+						a->name_length *
+							sizeof(__le16));
+		} else {
+			/* Payload offsets decreased or unchanged: move name first. */
+			if (a->name_length && name_ofs != old_name_ofs)
+				memmove((u8 *)a + name_ofs,
+						(u8 *)a + old_name_ofs,
+						a->name_length *
+							sizeof(__le16));
+			if (mp_ofs != old_mp_ofs)
+				memmove((u8 *)a + mp_ofs,
+						(u8 *)a + old_mp_ofs,
+						mp_size);
+		}
 	}
 
 	err = ntfs_attr_record_resize(m, a, arec_size);
-- 
2.25.1