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

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

[DaeMyung Kang]

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@gmail.com>
---
 fs/ntfs/namei.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
index dc786270afe5..aad928ebb7bd 100644
--- a/fs/ntfs/namei.c
+++ b/fs/ntfs/namei.c
@@ -355,9 +355,9 @@ static int ntfs_sd_add_everyone(struct ntfs_inode *ni)
 	sd_len = sizeof(struct security_descriptor_relative) + 2 *
 		(sizeof(struct ntfs_sid) + 8) + sizeof(struct ntfs_acl) +
 		sizeof(struct ntfs_ace) + 4;
-	sd = kmalloc(sd_len, GFP_NOFS);
+	sd = kzalloc(sd_len, GFP_NOFS);
 	if (!sd)
-		return -1;
+		return -ENOMEM;
 
 	sd->revision = 1;
 	sd->control = SE_DACL_PRESENT | SE_SELF_RELATIVE;
-- 
2.43.0

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

[Hyunchul Lee]

2026년 5월 8일 (금) 오전 12:48, DaeMyung Kang <charsyam@gmail.com>님이 작성:
>
> 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@gmail.com>

Looks good to me.

Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>

> ---
>  fs/ntfs/namei.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
> index dc786270afe5..aad928ebb7bd 100644
> --- a/fs/ntfs/namei.c
> +++ b/fs/ntfs/namei.c
> @@ -355,9 +355,9 @@ static int ntfs_sd_add_everyone(struct ntfs_inode *ni)
>         sd_len = sizeof(struct security_descriptor_relative) + 2 *
>                 (sizeof(struct ntfs_sid) + 8) + sizeof(struct ntfs_acl) +
>                 sizeof(struct ntfs_ace) + 4;
> -       sd = kmalloc(sd_len, GFP_NOFS);
> +       sd = kzalloc(sd_len, GFP_NOFS);
>         if (!sd)
> -               return -1;
> +               return -ENOMEM;
>
>         sd->revision = 1;
>         sd->control = SE_DACL_PRESENT | SE_SELF_RELATIVE;
> --
> 2.43.0
>


-- 
Thanks,
Hyunchul

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@gmail.com> 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@gmail.com>
Applied it to #ntfs-next.
Thanks!