[PATCH] ntfs3: fix info-leak in ntfs_rename()

[PATCH] ntfs3: fix info-leak in ntfs_rename()

[Dmitry Antipov]

In 'ntfs_rename()', buffer passed to 'fill_name_de()' should
be allocated with 'kzalloc()' to avoid exposing contents of
an uninitialized kernel memory via 'copy_to_user_iter()'.

Reported-by: syzbot+905d785c4923bea2c1db@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=905d785c4923bea2c1db
Fixes: ca2a04e84af7 ("ntfs: ->d_compare() must not block")
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 fs/ntfs3/namei.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
index b2af8f695e60..74fe002214f3 100644
--- a/fs/ntfs3/namei.c
+++ b/fs/ntfs3/namei.c
@@ -303,7 +303,7 @@ static int ntfs_rename(struct mnt_idmap *idmap, struct inode *dir,
 			return err;
 	}
 
-	de = kmalloc(PATH_MAX, GFP_KERNEL);
+	de = kzalloc(PATH_MAX, GFP_KERNEL);
 	if (!de)
 		return -ENOMEM;
 
-- 
2.54.0

Re: [PATCH] ntfs3: fix info-leak in ntfs_rename()

[Al Viro]

On Tue, Jun 09, 2026 at 03:36:18PM +0300, Dmitry Antipov wrote:
> In 'ntfs_rename()', buffer passed to 'fill_name_de()' should
> be allocated with 'kzalloc()' to avoid exposing contents of
> an uninitialized kernel memory via 'copy_to_user_iter()'.
> 
> Reported-by: syzbot+905d785c4923bea2c1db@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=905d785c4923bea2c1db
> Fixes: ca2a04e84af7 ("ntfs: ->d_compare() must not block")
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>  fs/ntfs3/namei.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
> index b2af8f695e60..74fe002214f3 100644
> --- a/fs/ntfs3/namei.c
> +++ b/fs/ntfs3/namei.c
> @@ -303,7 +303,7 @@ static int ntfs_rename(struct mnt_idmap *idmap, struct inode *dir,
>  			return err;
>  	}
>  
> -	de = kmalloc(PATH_MAX, GFP_KERNEL);
> +	de = kzalloc(PATH_MAX, GFP_KERNEL);
>  	if (!de)
>  		return -ENOMEM;

Could you please elaborate the way by which the contents of that
object would have managed to reach copy_to_user_iter()?
While we are at it, which userland addresses would rename()
want to write into?

Re: [PATCH] ntfs3: fix info-leak in ntfs_rename()

[Al Viro]

On Tue, Jun 09, 2026 at 09:19:48PM +0100, Al Viro wrote:
> On Tue, Jun 09, 2026 at 03:36:18PM +0300, Dmitry Antipov wrote:
> > In 'ntfs_rename()', buffer passed to 'fill_name_de()' should
> > be allocated with 'kzalloc()' to avoid exposing contents of
> > an uninitialized kernel memory via 'copy_to_user_iter()'.
> > 
> > Reported-by: syzbot+905d785c4923bea2c1db@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=905d785c4923bea2c1db
> > Fixes: ca2a04e84af7 ("ntfs: ->d_compare() must not block")
> > Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> > ---
> >  fs/ntfs3/namei.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
> > index b2af8f695e60..74fe002214f3 100644
> > --- a/fs/ntfs3/namei.c
> > +++ b/fs/ntfs3/namei.c
> > @@ -303,7 +303,7 @@ static int ntfs_rename(struct mnt_idmap *idmap, struct inode *dir,
> >  			return err;
> >  	}
> >  
> > -	de = kmalloc(PATH_MAX, GFP_KERNEL);
> > +	de = kzalloc(PATH_MAX, GFP_KERNEL);
> >  	if (!de)
> >  		return -ENOMEM;
> 
> Could you please elaborate the way by which the contents of that
> object would have managed to reach copy_to_user_iter()?
> While we are at it, which userland addresses would rename()
> want to write into?

From the syzkaller spew it would appear that data (filename converted
to unicode, AFAICS) somehow gets returned by read(2).  If that is
accurate, I would suggest that the things are already FUBAR and zeroing
is not going to fix whatever underlying bug you've got there (metadata
bh left around after the corresponding on-disk block got freed and
reused for regular file, perhaps?)

Re: [PATCH] ntfs3: fix info-leak in ntfs_rename()

[Dmitry Antipov]

On Wed, 2026-06-10 at 00:08 +0100, Al Viro wrote:

> From the syzkaller spew it would appear that data (filename converted
> to unicode, AFAICS) somehow gets returned by read(2).  If that is
> accurate, I would suggest that the things are already FUBAR and zeroing
> is not going to fix whatever underlying bug you've got there (metadata
> bh left around after the corresponding on-disk block got freed and
> reused for regular file, perhaps?)

Hard to say about copy_to_user_iter(), but at least the first splat
looks correct. At the end of fill_name_de(), data layout is:

struct NTFS_DE *e = buf;
...

    |<- data_size + sizeof(struct NTFS_DE) ->|<- XXX ->|
buf |-----------------------------------------------------------
    |<- ALIGN(data_size, 8) + sizeof(struct NTFS_DE) ->|   ;; e->size

If 'buf' was allocated with kmalloc(), XXX remains uninitialized and
passed as such to memcpy() called from hdr_insert_de().

So using kzalloc() for all buffers passed to fill_name_de() looks
the simplest and most safe solution. OTOH if someone would have
said that an overhead of PAGE_SIZE'd memset() is too large, more
fine-granted solution is to memset() XXX only, i.e.:

diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
index b2af8f695e60..21e4a2a34389 100644
--- a/fs/ntfs3/namei.c
+++ b/fs/ntfs3/namei.c
@@ -22,7 +22,7 @@ int fill_name_de(struct ntfs_sb_info *sbi, void *buf, const struct qstr *name,
 {
        int err;
        struct NTFS_DE *e = buf;
-       u16 data_size;
+       u16 data_size, real_size, aligned_size;
        struct ATTR_FILE_NAME *fname = (struct ATTR_FILE_NAME *)(e + 1);
 
 #ifndef CONFIG_NTFS3_64BIT_CLUSTER
@@ -53,7 +53,12 @@ int fill_name_de(struct ntfs_sb_info *sbi, void *buf, const struct qstr *name,
        fname->type = FILE_NAME_POSIX;
        data_size = fname_full_size(fname);
 
-       e->size = cpu_to_le16(ALIGN(data_size, 8) + sizeof(struct NTFS_DE));
+       real_size = data_size + sizeof(struct NTFS_DE);
+       aligned_size = ALIGN(data_size, 8) + sizeof(struct NTFS_DE);
+       if (aligned_size > real_size)
+               memset((char *)buf + real_size, 0, aligned_size - real_size);
+
+       e->size = cpu_to_le16(aligned_size);
        e->key_size = cpu_to_le16(data_size);
        e->flags = 0;
        e->res = 0;

Dmitry

Re: [PATCH] ntfs3: fix info-leak in ntfs_rename()

[Konstantin Komarov]

On 6/10/26 13:12, Dmitry Antipov wrote:

> On Wed, 2026-06-10 at 00:08 +0100, Al Viro wrote:
>
>>  From the syzkaller spew it would appear that data (filename converted
>> to unicode, AFAICS) somehow gets returned by read(2).  If that is
>> accurate, I would suggest that the things are already FUBAR and zeroing
>> is not going to fix whatever underlying bug you've got there (metadata
>> bh left around after the corresponding on-disk block got freed and
>> reused for regular file, perhaps?)
> Hard to say about copy_to_user_iter(), but at least the first splat
> looks correct. At the end of fill_name_de(), data layout is:
>
> struct NTFS_DE *e = buf;
> ...
>
>      |<- data_size + sizeof(struct NTFS_DE) ->|<- XXX ->|
> buf |-----------------------------------------------------------
>      |<- ALIGN(data_size, 8) + sizeof(struct NTFS_DE) ->|   ;; e->size
>
> If 'buf' was allocated with kmalloc(), XXX remains uninitialized and
> passed as such to memcpy() called from hdr_insert_de().
>
> So using kzalloc() for all buffers passed to fill_name_de() looks
> the simplest and most safe solution. OTOH if someone would have
> said that an overhead of PAGE_SIZE'd memset() is too large, more
> fine-granted solution is to memset() XXX only, i.e.:
>
> diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
> index b2af8f695e60..21e4a2a34389 100644
> --- a/fs/ntfs3/namei.c
> +++ b/fs/ntfs3/namei.c
> @@ -22,7 +22,7 @@ int fill_name_de(struct ntfs_sb_info *sbi, void *buf, const struct qstr *name,
>   {
>          int err;
>          struct NTFS_DE *e = buf;
> -       u16 data_size;
> +       u16 data_size, real_size, aligned_size;
>          struct ATTR_FILE_NAME *fname = (struct ATTR_FILE_NAME *)(e + 1);
>   
>   #ifndef CONFIG_NTFS3_64BIT_CLUSTER
> @@ -53,7 +53,12 @@ int fill_name_de(struct ntfs_sb_info *sbi, void *buf, const struct qstr *name,
>          fname->type = FILE_NAME_POSIX;
>          data_size = fname_full_size(fname);
>   
> -       e->size = cpu_to_le16(ALIGN(data_size, 8) + sizeof(struct NTFS_DE));
> +       real_size = data_size + sizeof(struct NTFS_DE);
> +       aligned_size = ALIGN(data_size, 8) + sizeof(struct NTFS_DE);
> +       if (aligned_size > real_size)
> +               memset((char *)buf + real_size, 0, aligned_size - real_size);
> +
> +       e->size = cpu_to_le16(aligned_size);
>          e->key_size = cpu_to_le16(data_size);
>          e->flags = 0;
>          e->res = 0;
>
> Dmitry

Hello,

Very sorry for the delay.
The patch is applied, thank you.

Regards,
Konstantin