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

[Jan Beulich]

>>> On 02.12.11 at 15:35, Borislav Petkov <bp@xxxxxxxxx> 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@xxxxxxxx>
>> 
>> ---
>>  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

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