Re: [PATCH v2] fs/ntfs3: fix slab-out-of-bounds write in ni_create_attr_list()

[HE WEI（ギカク）]

Thanks for clarifying. Since the upstream commit did not carry the
stable tag, could the stable team please consider backporting commit
7c4841e2a627 to the applicable supported stable trees? This fixes an
attacker-controlled on-disk slab-out-of-bounds write. My v2 carried
Cc: stable@xxxxxxxxxxxxxxx, but the functionally identical v1 was the
version merged upstream.

Thanks,
He Wei（ギカク）

Konstantin Komarov <almaz.alexandrovich@xxxxxxxxxxxxxxxxxxxx>
于2026年9月8日周二 17:41写道：
>
> On 7/6/26 06:40, HE WEI（ギカク） wrote:
>
> > Hi Konstantin,
> >
> > Gentle ping on this v2. It's an attacker-controlled on-disk image heap
> > out-of-bounds write in ni_create_attr_list(),
> > reachable via setxattr on a crafted, loop-mounted NTFS image (KASAN
> > trace is in the commit message), which is why it's Cc'd to stable.
> >
> > For the record, the fix was first posted as v1 on 2026-06-10:
> > https://lore.kernel.org/ntfs3/20260610002929.51765-1-skyexpoc@xxxxxxxxx/
> > This v2 (2026-06-25) only adds Cc: stable and widens review to
> > linux-fsdevel; the fix itself is unchanged from v1.
> >
> > Could you let me know if you'd like any changes, or whether it can be
> > queued for a future bugfix pull? I'm happy to rebase or adjust as
> > needed.
> >
> > Thanks,
> > HE WEI (ギカク)
> >
> > hewei-gikaku <skyexpoc@xxxxxxxxx> 于2026年6月25日周四 12:19写道：
> >> From: HE WEI (ギカク) <skyexpoc@xxxxxxxxx>
> >>
> >> ni_create_attr_list() allocates a fixed buffer of al_aligned(record_size)
> >> (== record_size) bytes and then walks every attribute of the primary MFT
> >> record, writing one ATTR_LIST_ENTRY per attribute and advancing the cursor
> >> by le_size(name_len), with no check against the end of the buffer; the
> >> total size is only computed after the loop.
> >>
> >> A minimum-size resident attribute occupies SIZEOF_RESIDENT (0x18 = 24)
> >> bytes on disk, but an unnamed attribute expands to le_size(0) (0x20 = 32)
> >> bytes in the list.  Because the number of attributes in a record is not
> >> bounded (mi_enum_attr() accepts arbitrarily many equal-type, nameless
> >> minimum-size attributes), a crafted record packed with such attributes
> >> produces a list larger than record_size and overflows the heap buffer.
> >>
> >> This is reachable from a crafted, loop-mounted NTFS image: opening the file
> >> and adding an attribute (e.g. via setxattr) drives ntfs_set_ea() ->
> >> ni_insert_resident() -> ni_insert_attr() -> ni_ins_attr_ext() ->
> >> ni_create_attr_list().
> >>
> >>    BUG: KASAN: slab-out-of-bounds in ni_create_attr_list+0xc48/0x1058
> >>    Write of size 4 at addr ffff000008984c00 by task setfattr/345
> >>     ni_create_attr_list+0xc48/0x1058
> >>     ni_ins_attr_ext+0x510/0x7c0
> >>     ni_insert_attr+0x3f8/0x70c
> >>     ni_insert_resident+0xc8/0x3b0
> >>     ntfs_set_ea+0x66c/0xd28
> >>     ntfs_setxattr+0x4d8/0x5b0
> >>     __arm64_sys_setxattr+0xa4/0x124
> >>    Allocated by task 345:
> >>     ni_create_attr_list+0x188/0x1058
> >>    The buggy address belongs to the cache kmalloc-1k of size 1024
> >>    (the write lands at object+1024).
> >>
> >> Size the buffer from the actual attributes instead of assuming a single
> >> record_size is always enough.
> >>
> >> Fixes: 4342306f0f0d ("fs/ntfs3: Add file operations and implementation")
> >> Cc: stable@xxxxxxxxxxxxxxx
> >> Signed-off-by: HE WEI (ギカク) <skyexpoc@xxxxxxxxx>
> >> ---
> >> v2:
> >>   - Add Cc: stable@xxxxxxxxxxxxxxx: this is an attacker-controlled on-disk
> >>     image heap out-of-bounds write and should be backported.
> >>   - No functional change from v1; widening Cc (linux-fsdevel, VFS) for
> >>     review, as the v1 posting received no response.
> >>   - Drop a redundant self Reported-by.
> >>
> >> v1: https://lore.kernel.org/all/20260610002929.51765-1-skyexpoc@xxxxxxxxx/
> >> ---
> >>   fs/ntfs3/frecord.c | 20 ++++++++++++++++----
> >>   1 file changed, 16 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
> >> index 2e901d073fe9..6488d7a415c0 100644
> >> --- a/fs/ntfs3/frecord.c
> >> +++ b/fs/ntfs3/frecord.c
> >> @@ -768,10 +768,23 @@ int ni_create_attr_list(struct ntfs_inode *ni)
> >>          rs = sbi->record_size;
> >>
> >>          /*
> >> -        * Skip estimating exact memory requirement.
> >> -        * Looks like one record_size is always enough.
> >> +        * Compute the exact size of the attribute list.  Each attribute in the
> >> +        * record yields one ATTR_LIST_ENTRY of le_size(name_len) bytes.  The
> >> +        * minimum on-disk attribute is SIZEOF_RESIDENT (0x18) bytes, but an
> >> +        * unnamed one expands to le_size(0) (0x20) here, so a record crafted
> >> +        * with many such attributes needs more than a single record_size; the
> >> +        * previous fixed kzalloc(record_size) could therefore be overflowed by
> >> +        * an attacker-controlled record.
> >>           */
> >> -       le = kzalloc(al_aligned(rs), GFP_NOFS);
> >> +       lsize = 0;
> >> +       attr = NULL;
> >> +       while ((attr = mi_enum_attr(ni, &ni->mi, attr)))
> >> +               lsize += le_size(attr->name_len);
> >> +
> >> +       if (!lsize)
> >> +               return -EINVAL;
> >> +
> >> +       le = kzalloc(al_aligned(lsize), GFP_NOFS);
> >>          if (!le)
> >>                  return -ENOMEM;
> >>
> >> @@ -781,7 +794,6 @@ int ni_create_attr_list(struct ntfs_inode *ni)
> >>          attr = NULL;
> >>          nb = 0;
> >>          free_b = 0;
> >> -       attr = NULL;
> >>
> >>          for (; (attr = mi_enum_attr(ni, &ni->mi, attr)); le = Add2Ptr(le, sz)) {
> >>                  sz = le_size(attr->name_len);
> >> --
> >> 2.43.0
>
> Hello,
>
> Sorry for the confusion. The fix is already upstream: v1 was applied as
> commit 7c4841e2a627 ("fs/ntfs3: fix slab-out-of-bounds write in
> ni_create_attr_list()"), in v7.3-rc1. v2 is functionally identical, so
> there is nothing further for me to apply.
>
> The applied commit does not carry Cc: stable, though, so it has not
> been backported.
>
> Thanks for the fix, and for following up on it.
>
> Regards,
> Konstantin
>