linux-kernel - Re: [PATCH] x86/mce: Check misc

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 for Android: free password hash cracker in your pocket

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

Date:	Fri, 25 Apr 2014 12:42:56 +0200
From:	Borislav Petkov <bp@...en8.de>
To:	Mathieu Souchaud <mattieu.souchaud@...e.fr>
Cc:	tony.luck@...el.com, tglx@...utronix.de, mingo@...hat.com,
	hpa@...or.com, x86@...nel.org, linux-edac@...r.kernel.org,
	linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] x86/mce: Check misc_register() return code.

On Fri, Apr 25, 2014 at 11:48:15AM +0200, Mathieu Souchaud wrote:
> The current code does not check the return from misc_register() so set
> the return variable with the return code of misc_register.
> 
> Signed-off-by: Mathieu Souchaud <mattieu.souchaud@...e.fr>
> ---
>  arch/x86/kernel/cpu/mcheck/mce.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
> index 68317c8..45fb7aa 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce.c
> @@ -2462,7 +2462,7 @@ static __init int mcheck_init_device(void)
>  	cpu_notifier_register_done();
>  
>  	/* register character device /dev/mcelog */
> -	misc_register(&mce_chrdev_device);
> +	err = misc_register(&mce_chrdev_device);

This function mcheck_init_device() is a one big sloppy mess
of not handling the error cases responsibly. It doesn't check
zalloc_cpumask_var() retval, it doesn't free the mce_device_initialized
when subsys_system_register() fails, the same when mce_device_create
fails, the same with __register_hotcpu_notifier and finally with
misc_register.

Would you be willing to fix this properly with goto labels and proper
unwinding, like it is done in other, saner places in the kernel, and
at the end maybe add a printk for the error case warning us that
/dev/mcelog didn't get registered?

That would be a very nice cleanup! :-)

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/