[PATCH] x86/mce: Check misc_register() return code.

[PATCH] x86/mce: Check misc_register() return code.

[Mathieu Souchaud]

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@free.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);
 
 	return err;
 }
-- 
1.7.9.5

Re: [PATCH] x86/mce: Check misc_register() return code.

[Borislav Petkov]

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@free.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.

Re: [PATCH] x86/mce: Check misc_register() return code.

[mathieu souchaud]

Great,

I will fix that soon.

Thanks.

Le 25/04/2014 12:42, Borislav Petkov a écrit :
> 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@free.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.