All of lore.kernel.org
 help / color / mirror / Atom feed
From: mathieu souchaud <mattieu.souchaud@free.fr>
To: Borislav Petkov <bp@alien8.de>
Cc: tony.luck@intel.com, tglx@linutronix.de, mingo@redhat.com,
	hpa@zytor.com, x86@kernel.org, linux-edac@vger.kernel.org,
	linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] x86/mce: Check misc_register() return code.
Date: Fri, 25 Apr 2014 11:53:35 +0000	[thread overview]
Message-ID: <535A4CBF.1090301@free.fr> (raw)
In-Reply-To: <20140425104256.GB18540@nazgul.tnic>

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.

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: mathieu souchaud <mattieu.souchaud@free.fr>
To: Borislav Petkov <bp@alien8.de>
Cc: tony.luck@intel.com, tglx@linutronix.de, mingo@redhat.com,
	hpa@zytor.com, x86@kernel.org, linux-edac@vger.kernel.org,
	linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] x86/mce: Check misc_register() return code.
Date: Fri, 25 Apr 2014 13:53:35 +0200	[thread overview]
Message-ID: <535A4CBF.1090301@free.fr> (raw)
In-Reply-To: <20140425104256.GB18540@nazgul.tnic>

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.


  reply	other threads:[~2014-04-25 11:53 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-25  9:48 [PATCH] x86/mce: Check misc_register() return code Mathieu Souchaud
2014-04-25  9:48 ` Mathieu Souchaud
2014-04-25 10:42 ` Borislav Petkov
2014-04-25 10:42   ` Borislav Petkov
2014-04-25 11:53   ` mathieu souchaud [this message]
2014-04-25 11:53     ` mathieu souchaud

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=535A4CBF.1090301@free.fr \
    --to=mattieu.souchaud@free.fr \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.