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

[Hyunchul Lee]

Hi Hongling,

2026년 8월 24일 (월) 오후 4:59, Hongling Zeng <zenghongling@xxxxxxxxxx>님이 작성:
>
> 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@xxxxxxxxxxxxxxx
> Signed-off-by: Hongling Zeng <zenghongling@xxxxxxxxxx>
> ---
>  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