Re: [PATCH] ntfs: avoid leaking uninitialised bytes in new security descriptors

[Namjae Jeon]

On Fri, May 8, 2026 at 12:49 AM DaeMyung Kang <charsyam@xxxxxxxxx> wrote:
>
> ntfs_sd_add_everyone() builds the on-disk security descriptor for a
> newly created file by kmalloc()'ing a buffer and then partially
> filling it in:
>
>         sd = kmalloc(sd_len, GFP_NOFS);
>         ...
>         sd->revision = 1;
>         sd->control = SE_DACL_PRESENT | SE_SELF_RELATIVE;
>         ...
>
> The buffer is then handed to ntfs_attr_add() and persisted as the
> SECURITY_DESCRIPTOR attribute of the new MFT record.  The descriptor
> covers a relative security descriptor header, two SIDs (owner and
> group), an ACL header, and a single ACE, but several fields inside
> those structures are never written before the buffer is committed
> to disk:
>
>   - struct security_descriptor_relative
>         @alignment              (1 byte)
>         @sacl                   (4 bytes; SE_SACL_PRESENT is not set
>                                  but the offset still reaches disk)
>
>   - struct ntfs_sid (3 instances: owner, group, ACE.sid)
>         identifier_authority.value[0..4] (5 bytes per SID, 15 total
>                                           - only value[5] is set)
>
>   - struct ntfs_acl
>         @alignment1             (1 byte)
>         @alignment2             (2 bytes)
>
> That is 23 bytes of uninitialised slab memory persisted to disk for
> every new file or directory the legacy ntfs driver creates.  The
> "+ 4" trailing accounting in sd_len holds ace->sid.sub_authority[0],
> which the existing code does explicitly write to zero, so it is
> not part of the leak.
>
> Anything later able to read the SECURITY_DESCRIPTOR attribute - the
> same NTFS volume mounted on Windows or by another NTFS reader, an
> offline forensics tool, an unprivileged user that ends up with read
> access to the volume - can recover those bytes.  The leak persists
> for the lifetime of the file on disk, not just the lifetime of the
> kernel that wrote it.
>
> Switch the allocation to kzalloc() so every byte the on-disk
> descriptor covers is zero before the explicit initialisations run.
> While there, replace the bare "return -1" allocation-failure path
> with a proper -ENOMEM so the error reaches userspace as a meaningful
> errno instead of an unrelated -EPERM.
>
> Found by inspection while auditing fs/ntfs new-inode paths.
>
> Fixes: af0db57d4293 ("ntfs: update inode operations")
> Signed-off-by: DaeMyung Kang <charsyam@xxxxxxxxx>
Applied it to #ntfs-next.
Thanks!