linux-kernel - Re: [PATCH] x86: fix error paths in microcode

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <4ED8F46E0200007800065178@nat28.tlf.novell.com>
Date:	Fri, 02 Dec 2011 14:53:18 +0000
From:	"Jan Beulich" <JBeulich@...e.com>
To:	"Borislav Petkov" <bp@...64.org>
Cc:	<mingo@...e.hu>, <tglx@...utronix.de>,
	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>,
	<linux-kernel@...r.kernel.org>, <hpa@...or.com>
Subject: Re: [PATCH] x86: fix error paths in microcode_init()

>>> On 02.12.11 at 15:35, Borislav Petkov <bp@...64.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@...e.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

--
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/