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

[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.

   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
>>
>