[PATCH] x86 Microcode: Fix the failure path of microcode update driver init code

[PATCH] x86 Microcode: Fix the failure path of microcode update driver init code

[Srivatsa S. Bhat]

The microcode update driver's initialization code does not handle failures
correctly. This patch fixes this issue.

Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 arch/x86/kernel/microcode_core.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/microcode_core.c b/arch/x86/kernel/microcode_core.c
index f2d2a66..c7bdbfa 100644
--- a/arch/x86/kernel/microcode_core.c
+++ b/arch/x86/kernel/microcode_core.c
@@ -519,10 +519,8 @@ static int __init microcode_init(void)
 
 	microcode_pdev = platform_device_register_simple("microcode", -1,
 							 NULL, 0);
-	if (IS_ERR(microcode_pdev)) {
-		microcode_dev_exit();
+	if (IS_ERR(microcode_pdev))
 		return PTR_ERR(microcode_pdev);
-	}
 
 	get_online_cpus();
 	mutex_lock(&microcode_mutex);
@@ -532,14 +530,12 @@ static int __init microcode_init(void)
 	mutex_unlock(&microcode_mutex);
 	put_online_cpus();
 
-	if (error) {
-		platform_device_unregister(microcode_pdev);
-		return error;
-	}
+	if (error)
+		goto out_pdev;
 
 	error = microcode_dev_init();
 	if (error)
-		return error;
+		goto out_sysdev_driver;
 
 	register_syscore_ops(&mc_syscore_ops);
 	register_hotcpu_notifier(&mc_cpu_notifier);
@@ -548,6 +544,13 @@ static int __init microcode_init(void)
 		" <tigran@aivazian.fsnet.co.uk>, Peter Oruba\n");
 
 	return 0;
+
+out_sysdev_driver:
+	sysdev_driver_unregister(&cpu_sysdev_class, &mc_sysdev_driver);
+out_pdev:
+	platform_device_unregister(microcode_pdev);
+	return error;
+
 }
 module_init(microcode_init);
 

Re: [PATCH] x86 Microcode: Fix the failure path of microcode update driver init code

[Borislav Petkov]

On Mon, Nov 07, 2011 at 06:05:32PM +0530, Srivatsa S. Bhat wrote:
> The microcode update driver's initialization code does not handle failures
> correctly. This patch fixes this issue.
> 
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>

Looks ok, applied.

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

[PATCH] x86: fix error paths in microcode_init()

[Jan Beulich]

After failure of platform_device_register_simple(), microcode_dev_exit()
got called without the call to microcode_dev_init() already having taken
place.

After failure of microcode_dev_init(), no cleanup of previously carried
out setup was done at all.

As a result, microcode_dev_exit() can now get __exit tagged on it.

(Noticed while looking at the code, not because of having experienced
an actual problem.)

Signed-off-by: Jan Beulich <jbeulich@suse.com>

---
 arch/x86/kernel/microcode_core.c |   18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

--- 3.2-rc4/arch/x86/kernel/microcode_core.c
+++ 3.2-rc4-x86-ucode-init-eh/arch/x86/kernel/microcode_core.c
@@ -256,7 +256,7 @@ static int __init microcode_dev_init(voi
 	return 0;
 }
 
-static void microcode_dev_exit(void)
+static void __exit microcode_dev_exit(void)
 {
 	misc_deregister(&microcode_dev);
 }
@@ -519,10 +519,8 @@ static int __init microcode_init(void)
 
 	microcode_pdev = platform_device_register_simple("microcode", -1,
 							 NULL, 0);
-	if (IS_ERR(microcode_pdev)) {
-		microcode_dev_exit();
+	if (IS_ERR(microcode_pdev))
 		return PTR_ERR(microcode_pdev);
-	}
 
 	get_online_cpus();
 	mutex_lock(&microcode_mutex);
@@ -538,8 +536,18 @@ static int __init microcode_init(void)
 	}
 
 	error = microcode_dev_init();
-	if (error)
+	if (error) {
+		get_online_cpus();
+		mutex_lock(&microcode_mutex);
+
+		sysdev_driver_unregister(&cpu_sysdev_class, &mc_sysdev_driver);
+
+		mutex_unlock(&microcode_mutex);
+		put_online_cpus();
+
+		platform_device_unregister(microcode_pdev);
 		return error;
+	}
 
 	register_syscore_ops(&mc_syscore_ops);
 	register_hotcpu_notifier(&mc_cpu_notifier);

Re: [PATCH] x86: fix error paths in microcode_init()

[Borislav Petkov]

Hi,

On Fri, Dec 02, 2011 at 01:35:19PM +0000, Jan Beulich wrote:
> After failure of platform_device_register_simple(), microcode_dev_exit()
> got called without the call to microcode_dev_init() already having taken
> place.
> 
> After failure of microcode_dev_init(), no cleanup of previously carried
> out setup was done at all.
> 
> As a result, microcode_dev_exit() can now get __exit tagged on it.
> 
> (Noticed while looking at the code, not because of having experienced
> an actual problem.)
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> ---
>  arch/x86/kernel/microcode_core.c |   18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> --- 3.2-rc4/arch/x86/kernel/microcode_core.c
> +++ 3.2-rc4-x86-ucode-init-eh/arch/x86/kernel/microcode_core.c
> @@ -256,7 +256,7 @@ static int __init microcode_dev_init(voi
>  	return 0;
>  }
>  
> -static void microcode_dev_exit(void)
> +static void __exit microcode_dev_exit(void)
>  {
>  	misc_deregister(&microcode_dev);
>  }
> @@ -519,10 +519,8 @@ static int __init microcode_init(void)
>  
>  	microcode_pdev = platform_device_register_simple("microcode", -1,
>  							 NULL, 0);
> -	if (IS_ERR(microcode_pdev)) {
> -		microcode_dev_exit();
> +	if (IS_ERR(microcode_pdev))
>  		return PTR_ERR(microcode_pdev);
> -	}
>  
>  	get_online_cpus();
>  	mutex_lock(&microcode_mutex);
> @@ -538,8 +536,18 @@ static int __init microcode_init(void)
>  	}
>  
>  	error = microcode_dev_init();
> -	if (error)
> +	if (error) {
> +		get_online_cpus();
> +		mutex_lock(&microcode_mutex);
> +
> +		sysdev_driver_unregister(&cpu_sysdev_class, &mc_sysdev_driver);
> +
> +		mutex_unlock(&microcode_mutex);
> +		put_online_cpus();
> +
> +		platform_device_unregister(microcode_pdev);
>  		return error;
> +	}

Actually, Srivatsa made a similar patch already which I sent to x86
guys (I don't think they've pulled yet) but yours is additionally more
careful to do proper locking before doing sysdev_driver_unregister().

Would you like to add that part ontop of Srivatsa's patch at the
out_sysdev_driver label and resend?

Patch is at git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git ucode

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [PATCH] x86: fix error paths in microcode_init()

[Jan Beulich]

>>> On 02.12.11 at 15:35, Borislav Petkov <bp@amd64.org> wrote:
> Hi,
> 
> On Fri, Dec 02, 2011 at 01:35:19PM +0000, Jan Beulich wrote:
>> After failure of platform_device_register_simple(), microcode_dev_exit()
>> got called without the call to microcode_dev_init() already having taken
>> place.
>> 
>> After failure of microcode_dev_init(), no cleanup of previously carried
>> out setup was done at all.
>> 
>> As a result, microcode_dev_exit() can now get __exit tagged on it.
>> 
>> (Noticed while looking at the code, not because of having experienced
>> an actual problem.)
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> 
>> ---
>>  arch/x86/kernel/microcode_core.c |   18 +++++++++++++-----
>>  1 file changed, 13 insertions(+), 5 deletions(-)
>> 
>> --- 3.2-rc4/arch/x86/kernel/microcode_core.c
>> +++ 3.2-rc4-x86-ucode-init-eh/arch/x86/kernel/microcode_core.c
>> @@ -256,7 +256,7 @@ static int __init microcode_dev_init(voi
>>  	return 0;
>>  }
>>  
>> -static void microcode_dev_exit(void)
>> +static void __exit microcode_dev_exit(void)
>>  {
>>  	misc_deregister(&microcode_dev);
>>  }
>> @@ -519,10 +519,8 @@ static int __init microcode_init(void)
>>  
>>  	microcode_pdev = platform_device_register_simple("microcode", -1,
>>  							 NULL, 0);
>> -	if (IS_ERR(microcode_pdev)) {
>> -		microcode_dev_exit();
>> +	if (IS_ERR(microcode_pdev))
>>  		return PTR_ERR(microcode_pdev);
>> -	}
>>  
>>  	get_online_cpus();
>>  	mutex_lock(&microcode_mutex);
>> @@ -538,8 +536,18 @@ static int __init microcode_init(void)
>>  	}
>>  
>>  	error = microcode_dev_init();
>> -	if (error)
>> +	if (error) {
>> +		get_online_cpus();
>> +		mutex_lock(&microcode_mutex);
>> +
>> +		sysdev_driver_unregister(&cpu_sysdev_class, &mc_sysdev_driver);
>> +
>> +		mutex_unlock(&microcode_mutex);
>> +		put_online_cpus();
>> +
>> +		platform_device_unregister(microcode_pdev);
>>  		return error;
>> +	}
> 
> Actually, Srivatsa made a similar patch already which I sent to x86
> guys (I don't think they've pulled yet) but yours is additionally more
> careful to do proper locking before doing sysdev_driver_unregister().
> 
> Would you like to add that part ontop of Srivatsa's patch at the
> out_sysdev_driver label and resend?

Why shouldn't this one be taken in its entirety instead?

Jan

Re: [PATCH] x86: fix error paths in microcode_init()

[Srivatsa S. Bhat]

On 12/02/2011 08:23 PM, Jan Beulich wrote:

>>>> On 02.12.11 at 15:35, Borislav Petkov <bp@amd64.org> wrote:
>> Hi,
>>
>> On Fri, Dec 02, 2011 at 01:35:19PM +0000, Jan Beulich wrote:
>>> After failure of platform_device_register_simple(), microcode_dev_exit()
>>> got called without the call to microcode_dev_init() already having taken
>>> place.
>>>
>>> After failure of microcode_dev_init(), no cleanup of previously carried
>>> out setup was done at all.
>>>
>>> As a result, microcode_dev_exit() can now get __exit tagged on it.
>>>
>>> (Noticed while looking at the code, not because of having experienced
>>> an actual problem.)
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> ---
>>>  arch/x86/kernel/microcode_core.c |   18 +++++++++++++-----
>>>  1 file changed, 13 insertions(+), 5 deletions(-)
>>>
>>> --- 3.2-rc4/arch/x86/kernel/microcode_core.c
>>> +++ 3.2-rc4-x86-ucode-init-eh/arch/x86/kernel/microcode_core.c
>>> @@ -256,7 +256,7 @@ static int __init microcode_dev_init(voi
>>>  	return 0;
>>>  }
>>>  
>>> -static void microcode_dev_exit(void)
>>> +static void __exit microcode_dev_exit(void)
>>>  {
>>>  	misc_deregister(&microcode_dev);
>>>  }
>>> @@ -519,10 +519,8 @@ static int __init microcode_init(void)
>>>  
>>>  	microcode_pdev = platform_device_register_simple("microcode", -1,
>>>  							 NULL, 0);
>>> -	if (IS_ERR(microcode_pdev)) {
>>> -		microcode_dev_exit();
>>> +	if (IS_ERR(microcode_pdev))
>>>  		return PTR_ERR(microcode_pdev);
>>> -	}
>>>  
>>>  	get_online_cpus();
>>>  	mutex_lock(&microcode_mutex);
>>> @@ -538,8 +536,18 @@ static int __init microcode_init(void)
>>>  	}
>>>  
>>>  	error = microcode_dev_init();
>>> -	if (error)
>>> +	if (error) {
>>> +		get_online_cpus();
>>> +		mutex_lock(&microcode_mutex);
>>> +
>>> +		sysdev_driver_unregister(&cpu_sysdev_class, &mc_sysdev_driver);
>>> +
>>> +		mutex_unlock(&microcode_mutex);
>>> +		put_online_cpus();
>>> +
>>> +		platform_device_unregister(microcode_pdev);
>>>  		return error;
>>> +	}
>>
>> Actually, Srivatsa made a similar patch already which I sent to x86
>> guys (I don't think they've pulled yet) but yours is additionally more
>> careful to do proper locking before doing sysdev_driver_unregister().
>>
>> Would you like to add that part ontop of Srivatsa's patch at the
>> out_sysdev_driver label and resend?
> 
> Why shouldn't this one be taken in its entirety instead?
> 


Link to my patch: https://lkml.org/lkml/2011/11/7/136

Your patch fixes the issue more properly than mine, but adding your part
on top of my patch makes the code look better. For example,
platform_device_unregister() wouldn't need to be called twice; and we 
can use the quite popular way of handling error path via goto statements,
which makes the code flow much more comprehensible and intuitive.

[Btw, in addition to that, since Boris has already asked Ingo to pull my
patch, perhaps it also makes it easier that way. But I definitely see a
plus point in putting your part on top of mine, for code clarity reasons.]

Regards,
Srivatsa S. Bhat
IBM Linux Technology Center

Re: [PATCH] x86: fix error paths in microcode_init()

[Borislav Petkov]

On Fri, Dec 02, 2011 at 08:45:09PM +0530, Srivatsa S. Bhat wrote:
> Your patch fixes the issue more properly than mine, but adding your part
> on top of my patch makes the code look better. For example,
> platform_device_unregister() wouldn't need to be called twice; and we 
> can use the quite popular way of handling error path via goto statements,
> which makes the code flow much more comprehensible and intuitive.

Yes,

goto labels is the proper way for spelling error handling in the kernel
so I could very well take your patch Jan, instead, if you change it to
use goto labels for the error path as Srivatsa's patch does it. That is,
in case Ingo hasn't pulled yet.

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [PATCH] x86: fix error paths in microcode_init()

[Jan Beulich]

>>> On 02.12.11 at 16:24, Borislav Petkov <bp@amd64.org> wrote:
> On Fri, Dec 02, 2011 at 08:45:09PM +0530, Srivatsa S. Bhat wrote:
>> Your patch fixes the issue more properly than mine, but adding your part
>> on top of my patch makes the code look better. For example,
>> platform_device_unregister() wouldn't need to be called twice; and we 
>> can use the quite popular way of handling error path via goto statements,
>> which makes the code flow much more comprehensible and intuitive.
> 
> Yes,
> 
> goto labels is the proper way for spelling error handling in the kernel
> so I could very well take your patch Jan, instead, if you change it to
> use goto labels for the error path as Srivatsa's patch does it. That is,
> in case Ingo hasn't pulled yet.

Sorry, no, I won't introduce new labels in functions not already using
some (I'm already feeling guilty enough each time I end up doing so
when a function already is coded that way, to limit the impact of a
particular change). This is just bad programming style in my opinion, no
matter what other developers may think on this subject.

Jan

Re: [PATCH] x86: fix error paths in microcode_init()

[Srivatsa S. Bhat]

On 12/02/2011 09:03 PM, Jan Beulich wrote:

>>>> On 02.12.11 at 16:24, Borislav Petkov <bp@amd64.org> wrote:
>> On Fri, Dec 02, 2011 at 08:45:09PM +0530, Srivatsa S. Bhat wrote:
>>> Your patch fixes the issue more properly than mine, but adding your part
>>> on top of my patch makes the code look better. For example,
>>> platform_device_unregister() wouldn't need to be called twice; and we 
>>> can use the quite popular way of handling error path via goto statements,
>>> which makes the code flow much more comprehensible and intuitive.
>>
>> Yes,
>>
>> goto labels is the proper way for spelling error handling in the kernel
>> so I could very well take your patch Jan, instead, if you change it to
>> use goto labels for the error path as Srivatsa's patch does it. That is,
>> in case Ingo hasn't pulled yet.
> 
> Sorry, no, I won't introduce new labels in functions not already using
> some (I'm already feeling guilty enough each time I end up doing so
> when a function already is coded that way, to limit the impact of a
> particular change). This is just bad programming style in my opinion, no
> matter what other developers may think on this subject.
> 


Hi Jan,
Honestly no offense meant, but I like using goto for error handling in
functions, especially when it results in clear-cut code flows such as:

do step1
do step2
...
undo step2
undo step1

It just makes it look so obvious and very easy to spot errors. However if
you have an if-else for everything and duplicate the undo code, chances are
that we might miss something/mess up the ordering. But in the flow shown
above, if you get the ordering right for once (which is quite easy), you
can just forget about it later on.

[I wouldn't have insisted if usage of goto statements would have messed up
code readability by not having a neat clear-cut sequence like that shown above.
But in the microcode driver case, since we are lucky to get this sequence,
I prefer to go by that.]

So, Boris, here is a patch which applies on top of my previous patch.
It is up to you to pull whichever patch you like (either mine or Jan's),
since it is only a choice of programming style, but functionality-wise,
the two are equivalent.

Thanks to Jan for pointing out the missing locking around
sysdev_driver_unregister() and that we can add __exit qualifier to
microcode_dev_exit().

And if Boris is indeed going to take this patch, Jan, feel free to add
your "Signed-off-by" (if you agree to the code below) since I would like
to give due credit to you for pointing out the missing parts.

---
[PATCH] x86 Microcode: Fix the microcode module initialization error path and improve code

The function sysdev_driver_unregister() must be called with proper locking
around it. So add this synchronization to the call to sysdev_driver_unregister()
in the microcode module initialization error path.

Also, add the __exit qualifier to microcode_dev_exit(), since it is called only
from microcode_exit().

Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 arch/x86/kernel/microcode_core.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/microcode_core.c b/arch/x86/kernel/microcode_core.c
index c7bdbfa..9d46f5e 100644
--- a/arch/x86/kernel/microcode_core.c
+++ b/arch/x86/kernel/microcode_core.c
@@ -256,7 +256,7 @@ static int __init microcode_dev_init(void)
 	return 0;
 }
 
-static void microcode_dev_exit(void)
+static void __exit microcode_dev_exit(void)
 {
 	misc_deregister(&microcode_dev);
 }
@@ -546,7 +546,14 @@ static int __init microcode_init(void)
 	return 0;
 
 out_sysdev_driver:
+	get_online_cpus();
+	mutex_lock(&microcode_mutex);
+
 	sysdev_driver_unregister(&cpu_sysdev_class, &mc_sysdev_driver);
+
+	mutex_unlock(&microcode_mutex);
+	put_online_cpus();
+
 out_pdev:
 	platform_device_unregister(microcode_pdev);
 	return error;

Re: [PATCH] x86: fix error paths in microcode_init()

[Jan Beulich]

>>> On 02.12.11 at 17:40, "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
> And if Boris is indeed going to take this patch, Jan, feel free to add
> your "Signed-off-by" (if you agree to the code below) since I would like
> to give due credit to you for pointing out the missing parts.

Feel free to add that or an acked-by.

Jan

Re: [PATCH] x86: fix error paths in microcode_init()

[Srivatsa S. Bhat]

On 12/02/2011 10:15 PM, Jan Beulich wrote:

>>>> On 02.12.11 at 17:40, "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
>> And if Boris is indeed going to take this patch, Jan, feel free to add
>> your "Signed-off-by" (if you agree to the code below) since I would like
>> to give due credit to you for pointing out the missing parts.
> 
> Feel free to add that or an acked-by.


Wow! Good to know that we have an agreement :-)
Boris, it is again up to you to choose among the two(Signed-off-by or Acked-by)
for Jan. Hehe ;-)

Thanks,
Srivatsa S. Bhat

Re: [PATCH] x86: fix error paths in microcode_init()

[Borislav Petkov]

On Fri, Dec 02, 2011 at 10:21:01PM +0530, Srivatsa S. Bhat wrote:
> Wow! Good to know that we have an agreement :-)
> Boris, it is again up to you to choose among the two(Signed-off-by or Acked-by)
> for Jan. Hehe ;-)

Well,

since Ingo hasn't pulled yet, I merged the two patches after adding the
required S-O-Bs into the following:

--
From: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
Date: Mon, 7 Nov 2011 18:05:32 +0530
Subject: [PATCH] x86, microcode: Fix the failure path of microcode update
 driver init code

The microcode update driver's initialization code does not handle
failures correctly. This patch fixes this issue.

Signed-off-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/20111107123530.12164.31227.stgit@srivatsabhat.in.ibm.com
Link: http://lkml.kernel.org/r/4ED8E2270200007800065120@nat28.tlf.novell.com
Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 arch/x86/kernel/microcode_core.c |   28 +++++++++++++++++++---------
 1 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/microcode_core.c b/arch/x86/kernel/microcode_core.c
index f2d2a66..9d46f5e 100644
--- a/arch/x86/kernel/microcode_core.c
+++ b/arch/x86/kernel/microcode_core.c
@@ -256,7 +256,7 @@ static int __init microcode_dev_init(void)
 	return 0;
 }
 
-static void microcode_dev_exit(void)
+static void __exit microcode_dev_exit(void)
 {
 	misc_deregister(&microcode_dev);
 }
@@ -519,10 +519,8 @@ static int __init microcode_init(void)
 
 	microcode_pdev = platform_device_register_simple("microcode", -1,
 							 NULL, 0);
-	if (IS_ERR(microcode_pdev)) {
-		microcode_dev_exit();
+	if (IS_ERR(microcode_pdev))
 		return PTR_ERR(microcode_pdev);
-	}
 
 	get_online_cpus();
 	mutex_lock(&microcode_mutex);
@@ -532,14 +530,12 @@ static int __init microcode_init(void)
 	mutex_unlock(&microcode_mutex);
 	put_online_cpus();
 
-	if (error) {
-		platform_device_unregister(microcode_pdev);
-		return error;
-	}
+	if (error)
+		goto out_pdev;
 
 	error = microcode_dev_init();
 	if (error)
-		return error;
+		goto out_sysdev_driver;
 
 	register_syscore_ops(&mc_syscore_ops);
 	register_hotcpu_notifier(&mc_cpu_notifier);
@@ -548,6 +544,20 @@ static int __init microcode_init(void)
 		" <tigran@aivazian.fsnet.co.uk>, Peter Oruba\n");
 
 	return 0;
+
+out_sysdev_driver:
+	get_online_cpus();
+	mutex_lock(&microcode_mutex);
+
+	sysdev_driver_unregister(&cpu_sysdev_class, &mc_sysdev_driver);
+
+	mutex_unlock(&microcode_mutex);
+	put_online_cpus();
+
+out_pdev:
+	platform_device_unregister(microcode_pdev);
+	return error;
+
 }
 module_init(microcode_init);
 
-- 
1.7.8.rc0



-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [PATCH] x86: fix error paths in microcode_init()

[Srivatsa S. Bhat]

On 12/03/2011 01:15 AM, Borislav Petkov wrote:

> On Fri, Dec 02, 2011 at 10:21:01PM +0530, Srivatsa S. Bhat wrote:
>> Wow! Good to know that we have an agreement :-)
>> Boris, it is again up to you to choose among the two(Signed-off-by or Acked-by)
>> for Jan. Hehe ;-)
> 
> Well,
> 
> since Ingo hasn't pulled yet, I merged the two patches after adding the
> required S-O-Bs into the following:
> 


Great! Thank you.

Regards,
Srivatsa S. Bhat

[tip:x86/urgent] x86, microcode: Fix the failure path of microcode update driver init code

[tip-bot for Srivatsa S. Bhat]

Commit-ID:  bd399063976c6c7a09beb4730ed1d93cadbcc739
Gitweb:     http://git.kernel.org/tip/bd399063976c6c7a09beb4730ed1d93cadbcc739
Author:     Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
AuthorDate: Mon, 7 Nov 2011 18:05:32 +0530
Committer:  Borislav Petkov <borislav.petkov@amd.com>
CommitDate: Mon, 5 Dec 2011 14:21:01 +0100

x86, microcode: Fix the failure path of microcode update driver init code

The microcode update driver's initialization code does not handle
failures correctly. This patch fixes this issue.

Signed-off-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/20111107123530.12164.31227.stgit@srivatsabhat.in.ibm.com
Link: http://lkml.kernel.org/r/4ED8E2270200007800065120@nat28.tlf.novell.com
Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 arch/x86/kernel/microcode_core.c |   28 +++++++++++++++++++---------
 1 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/microcode_core.c b/arch/x86/kernel/microcode_core.c
index f2d2a66..9d46f5e 100644
--- a/arch/x86/kernel/microcode_core.c
+++ b/arch/x86/kernel/microcode_core.c
@@ -256,7 +256,7 @@ static int __init microcode_dev_init(void)
 	return 0;
 }
 
-static void microcode_dev_exit(void)
+static void __exit microcode_dev_exit(void)
 {
 	misc_deregister(&microcode_dev);
 }
@@ -519,10 +519,8 @@ static int __init microcode_init(void)
 
 	microcode_pdev = platform_device_register_simple("microcode", -1,
 							 NULL, 0);
-	if (IS_ERR(microcode_pdev)) {
-		microcode_dev_exit();
+	if (IS_ERR(microcode_pdev))
 		return PTR_ERR(microcode_pdev);
-	}
 
 	get_online_cpus();
 	mutex_lock(&microcode_mutex);
@@ -532,14 +530,12 @@ static int __init microcode_init(void)
 	mutex_unlock(&microcode_mutex);
 	put_online_cpus();
 
-	if (error) {
-		platform_device_unregister(microcode_pdev);
-		return error;
-	}
+	if (error)
+		goto out_pdev;
 
 	error = microcode_dev_init();
 	if (error)
-		return error;
+		goto out_sysdev_driver;
 
 	register_syscore_ops(&mc_syscore_ops);
 	register_hotcpu_notifier(&mc_cpu_notifier);
@@ -548,6 +544,20 @@ static int __init microcode_init(void)
 		" <tigran@aivazian.fsnet.co.uk>, Peter Oruba\n");
 
 	return 0;
+
+out_sysdev_driver:
+	get_online_cpus();
+	mutex_lock(&microcode_mutex);
+
+	sysdev_driver_unregister(&cpu_sysdev_class, &mc_sysdev_driver);
+
+	mutex_unlock(&microcode_mutex);
+	put_online_cpus();
+
+out_pdev:
+	platform_device_unregister(microcode_pdev);
+	return error;
+
 }
 module_init(microcode_init);