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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 22 Jun 2017 21:21:07 +0200
From:   Boris Brezillon <boris.brezillon@...e-electrons.com>
To:     Richard Weinberger <richard@....at>
Cc:     linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Adrian Hunter <adrian.hunter@...el.com>,
        stable@...r.kernel.org, Artem Bityutskiy <dedekind1@...il.com>
Subject: Re: [PATCH] ubifs: Don't leak kernel memory to the MTD

On Fri, 16 Jun 2017 16:21:44 +0200
Richard Weinberger <richard@....at> 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@...r.kernel.org
> Fixes: 1e51764a3c2a ("UBIFS: add new flash file system")
> Signed-off-by: Richard Weinberger <richard@....at>

Reviewed-by: Boris Brezillon <boris.brezillon@...e-electrons.com>

> ---
>  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;
>