linux-kernel - Re: [PATCH] tty: vt: Check for NULL pointer after calling kzalloc

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <716113c0-13d6-6fbc-f1a0-9fea6091bb90@kernel.org>
Date:   Fri, 21 Jan 2022 08:01:28 +0100
From:   Jiri Slaby <jirislaby@...nel.org>
To:     Jiasheng Jiang <jiasheng@...as.ac.cn>, gregkh@...uxfoundation.org,
        jcmvbkbc@...il.com, dsterba@...e.com, johan@...nel.org,
        dankamongmen@...il.com, penguin-kernel@...ove.SAKURA.ne.jp,
        igormtorrente@...il.com
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH] tty: vt: Check for NULL pointer after calling kzalloc

On 21. 01. 22, 7:53, Jiasheng Jiang wrote:
> As the potential failure of the allocation, the kzalloc() will return
> NULL pointer.
> Therefore, it should be better to check it in order to avoid the
> dereference of the NULL pointer.
> When it fails, we should free all the allocated memory and return error
> number.
> To make the code more clear, I use the 'err_free', like how
> vc_allocate() deals with the allocation failure.

https://lore.kernel.org/all/X+CRTqDQwVYSZQBi@kroah.com/

> Fixes: a5f4f52e8211 ("vt: use kzalloc() instead of the bootmem allocator")
> Signed-off-by: Jiasheng Jiang <jiasheng@...as.ac.cn>
> ---
>   drivers/tty/vt/vt.c | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index f8c87c4d7399..343fa6fffc18 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -3519,11 +3519,17 @@ static int __init con_init(void)
>   
>   	for (currcons = 0; currcons < MIN_NR_CONSOLES; currcons++) {
>   		vc_cons[currcons].d = vc = kzalloc(sizeof(struct vc_data), GFP_NOWAIT);
> +		if (!vc)
> +			goto err_free;
> +
>   		INIT_WORK(&vc_cons[currcons].SAK_work, vc_SAK);
>   		tty_port_init(&vc->port);
>   		visual_init(vc, currcons, 1);
>   		/* Assuming vc->vc_{cols,rows,screenbuf_size} are sane here. */
>   		vc->vc_screenbuf = kzalloc(vc->vc_screenbuf_size, GFP_NOWAIT);
> +		if (!vc->vc_screenbuf)
> +			goto err_free_vc;
> +
>   		vc_init(vc, vc->vc_rows, vc->vc_cols,
>   			currcons || !vc->vc_sw->con_save_screen);
>   	}
> @@ -3545,6 +3551,19 @@ static int __init con_init(void)
>   	register_console(&vt_console_driver);
>   #endif
>   	return 0;
> +err_free_vc:
> +	visual_deinit(vc);
> +	kfree(vc);
> +	vc_cons[currcons].d = NULL;
> +err_free:
> +	for (i = 0; i < currcons; i++) {
> +		kfree(vc_cons[currcons].d->vc_screenbuf);
> +		visual_deinit(vc_cons[currcons].d);
> +		kfree(vc_cons[currcons].d);
> +		vc_cons[currcons].d = NULL;
> +	}
> +	console_unlock();
> +	return -ENOMEM;
>   }
>   console_initcall(con_init);
>   


-- 
js