Re: [PATCH] ntfs3: zero-initialize trailing VBN slot in index entry

Re: [PATCH] ntfs3: zero-initialize trailing VBN slot in index entry

[syzbot]

> From: Tristan Madani <tristan@talencesecurity.com>
>
> #syz test: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

This crash does not have a reproducer. I cannot test it.

>
>  allocations
>
> indx_insert_into_buffer() and indx_delete_entry() each allocate an
> index entry buffer with room for a trailing VBN (virtual block number)
> sub-node pointer:
>
>     up_e = kmalloc(sp_size + sizeof(u64), GFP_NOFS);
>     memcpy(up_e, sp, sp_size);
>
>     re = kmalloc(le16_to_cpu(te->size) + sizeof(u64), GFP_NOFS);
>     memcpy(re, te, le16_to_cpu(te->size));
>
> The extra sizeof(u64) bytes reserved for the VBN are left
> uninitialized.  When the entry is a leaf node (no sub-nodes), the VBN
> slot is never written, and the uninitialized bytes are eventually
> written to the index buffer on disk.  When the index is later read
> back, ntfs_read_hdr() processes these uninitialized bytes, which KMSAN
> flags as a use of uninitialized memory.
>
> Fix this by using kzalloc() instead of kmalloc(), ensuring the VBN
> slot defaults to zero when not explicitly set.
>
> Reported-by: syzbot+2472d48b2d3b2a23a570@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=2472d48b2d3b2a23a570
> Fixes: 522e010b5837 ("fs/ntfs3: Add file operations and implementation")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
>  fs/ntfs3/index.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
> index XXXXXXX..XXXXXXX 100644
> --- a/fs/ntfs3/index.c
> +++ b/fs/ntfs3/index.c
> @@ -1838,7 +1838,7 @@ static int indx_insert_into_buffer(struct ntfs_index *indx,
>
>  	sp_size = le16_to_cpu(sp->size);
> -	up_e = kmalloc(sp_size + sizeof(u64), GFP_NOFS);
> +	up_e = kzalloc(sp_size + sizeof(u64), GFP_NOFS);
>  	if (!up_e)
>  		return -ENOMEM;
>  	memcpy(up_e, sp, sp_size);
> @@ -2228,7 +2228,7 @@ int indx_delete_entry(struct ntfs_index *indx, struct ntfs_inode *ni,
>  	}
>
>  	/* Copy the candidate entry into the replacement entry buffer. */
> -	re = kmalloc(le16_to_cpu(te->size) + sizeof(u64), GFP_NOFS);
> +	re = kzalloc(le16_to_cpu(te->size) + sizeof(u64), GFP_NOFS);
>  	if (!re) {
>  		err = -ENOMEM;
>  		goto out;
> --
> 2.43.0