Re: [PATCH 11/11] ntfs: move attribute payload before shrinking its record

2026=EB=85=84 7=EC=9B=94 21=EC=9D=BC (=ED=99=94) =EC=98=A4=ED=9B=84 6:55, N=
amjae Jeon <[email protected]>=EB=8B=98=EC=9D=B4 =EC=9E=91=EC=84=B1:
>
> ntfs_new_attr_flags() resizes the non-resident attribute record before
> moving its name and mapping pairs to their shorter-header offsets when
> compression or sparse state is cleared.
>
> Shrinking the record first moves the following attribute over the tail of
> the old record. The subsequent memmove() therefore copies bytes from that
> following attribute instead of the old mapping pairs. Re-enabling
> compression on an empty file persists those bytes as a malformed mapping
> pairs array, which ntfsck reports as a missing or invalid run length.
>
> Move the payload before shrinking the record, while retaining the existin=
g
> resize-before-move ordering when growing it.
>
> Signed-off-by: Namjae Jeon <[email protected]>

Looks good to me.

Reviewed-by: Hyunchul Lee <[email protected]>

> ---
>  fs/ntfs/ea.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/fs/ntfs/ea.c b/fs/ntfs/ea.c
> index 95587b9a7129..4fb10d51211c 100644
> --- a/fs/ntfs/ea.c
> +++ b/fs/ntfs/ea.c
> @@ -631,7 +631,7 @@ static int ntfs_new_attr_flags(struct ntfs_inode *ni,=
 __le32 fattr)
>         struct attr_record *a;
>         __le16 new_aflags;
>         u16 old_name_ofs, old_mp_ofs;
> -       int mp_size, mp_ofs, name_ofs, arec_size, err;
> +       int mp_size, mp_ofs, name_ofs, old_arec_size, arec_size, err;
>
>         m =3D map_mft_record(ni);
>         if (IS_ERR(m))
> @@ -726,6 +726,19 @@ static int ntfs_new_attr_flags(struct ntfs_inode *ni=
, __le32 fattr)
>
>         mp_ofs =3D (name_ofs + a->name_length * sizeof(__le16) + 7) & ~7;
>         arec_size =3D (mp_ofs + mp_size + 7) & ~7;
> +       old_arec_size =3D le32_to_cpu(a->length);
> +
> +       /*
> +        * Move payloads before shrinking the record.  Otherwise resizing=
 moves
> +        * the following attribute over the old payload before it can be =
copied.
> +        */
> +       if (arec_size < old_arec_size) {
> +               if (a->name_length && name_ofs !=3D old_name_ofs)
> +                       memmove((u8 *)a + name_ofs, (u8 *)a + old_name_of=
s,
> +                               a->name_length * sizeof(__le16));
> +               if (mp_ofs !=3D old_mp_ofs)
> +                       memmove((u8 *)a + mp_ofs, (u8 *)a + old_mp_ofs, m=
p_size);
> +       }
>
>         err =3D ntfs_attr_record_resize(m, a, arec_size);
>         if (unlikely(err))
> @@ -736,18 +749,12 @@ static int ntfs_new_attr_flags(struct ntfs_inode *n=
i, __le32 fattr)
>          * shrinks by the compressed_size field. Update the in-record pay=
load layout
>          * to match the new offsets before exposing the new mapping_pairs=
_offset.
>          */
> -       if (name_ofs > old_name_ofs) {
> +       if (arec_size > old_arec_size) {
>                 if (mp_ofs !=3D old_mp_ofs)
>                         memmove((u8 *)a + mp_ofs, (u8 *)a + old_mp_ofs, m=
p_size);
>                 if (a->name_length)
>                         memmove((u8 *)a + name_ofs, (u8 *)a + old_name_of=
s,
>                                 a->name_length * sizeof(__le16));
> -       } else {
> -               if (a->name_length && name_ofs !=3D old_name_ofs)
> -                       memmove((u8 *)a + name_ofs, (u8 *)a + old_name_of=
s,
> -                               a->name_length * sizeof(__le16));
> -               if (mp_ofs !=3D old_mp_ofs)
> -                       memmove((u8 *)a + mp_ofs, (u8 *)a + old_mp_ofs, m=
p_size);
>         }
>
>         if (new_aflags & (ATTR_IS_SPARSE | ATTR_IS_COMPRESSED)) {
> --
> 2.34.1
>


--=20
Thanks,
Hyunchul