Re: [PATCH] ubifs: Don't leak kernel memory to the MTD

[Boris Brezillon]

On Fri, 16 Jun 2017 16:21:44 +0200
Richard Weinberger <richard@xxxxxx> wrote:

> When UBIFS prepares data structures which will be written to the MTD it
> ensues that their lengths are multiple of 8. Since it uses kmalloc() the
> padded bytes are left uninitialized and we leak a few bytes of kernel
> memory to the MTD.
> To make sure that all bytes are initialized, let's switch to kzalloc().
> Kzalloc() is fine in this case because the buffers are not huge and in
> the IO path the performance bottleneck is anyway the MTD.
> 
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 1e51764a3c2a ("UBIFS: add new flash file system")
> Signed-off-by: Richard Weinberger <richard@xxxxxx>

Reviewed-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx>

> ---
>  fs/ubifs/journal.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
> index 294519b98874..981a7ea86674 100644
> --- a/fs/ubifs/journal.c
> +++ b/fs/ubifs/journal.c
> @@ -574,7 +574,7 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
>  	/* Make sure to also account for extended attributes */
>  	len += host_ui->data_len;
>  
> -	dent = kmalloc(len, GFP_NOFS);
> +	dent = kzalloc(len, GFP_NOFS);
>  	if (!dent)
>  		return -ENOMEM;
>  
> @@ -967,7 +967,7 @@ int ubifs_jnl_xrename(struct ubifs_info *c, const struct inode *fst_dir,
>  	if (twoparents)
>  		len += plen;
>  
> -	dent1 = kmalloc(len, GFP_NOFS);
> +	dent1 = kzalloc(len, GFP_NOFS);
>  	if (!dent1)
>  		return -ENOMEM;
>  
> @@ -1117,7 +1117,7 @@ int ubifs_jnl_rename(struct ubifs_info *c, const struct inode *old_dir,
>  	len = aligned_dlen1 + aligned_dlen2 + ALIGN(ilen, 8) + ALIGN(plen, 8);
>  	if (move)
>  		len += plen;
> -	dent = kmalloc(len, GFP_NOFS);
> +	dent = kzalloc(len, GFP_NOFS);
>  	if (!dent)
>  		return -ENOMEM;
>  
> @@ -1500,7 +1500,7 @@ int ubifs_jnl_delete_xattr(struct ubifs_info *c, const struct inode *host,
>  	hlen = host_ui->data_len + UBIFS_INO_NODE_SZ;
>  	len = aligned_xlen + UBIFS_INO_NODE_SZ + ALIGN(hlen, 8);
>  
> -	xent = kmalloc(len, GFP_NOFS);
> +	xent = kzalloc(len, GFP_NOFS);
>  	if (!xent)
>  		return -ENOMEM;
>  
> @@ -1607,7 +1607,7 @@ int ubifs_jnl_change_xattr(struct ubifs_info *c, const struct inode *inode,
>  	aligned_len1 = ALIGN(len1, 8);
>  	aligned_len = aligned_len1 + ALIGN(len2, 8);
>  
> -	ino = kmalloc(aligned_len, GFP_NOFS);
> +	ino = kzalloc(aligned_len, GFP_NOFS);
>  	if (!ino)
>  		return -ENOMEM;
>