fs/ntfs/ea.c | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-)
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.1On 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!
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
在 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
>>
>
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>
© 2016 - 2026 Red Hat, Inc.