linux-kernel - Re: [patch] Re: [Bug #12100] resume (S2R) broken by Intel microcode module, on A110L

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] [day] [month] [year] [list]

Date:	Sun, 11 Jan 2009 12:11:37 -0800
From:	Mike Travis <travis@....com>
To:	Dmitry Adamushko <dmitry.adamushko@...il.com>
CC:	Ingo Molnar <mingo@...e.hu>, andeas.herrmann3@....com,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	"Rafael J. Wysocki" <rjw@...k.pl>, Andreas Mohr <andi@...as.de>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Kernel Testers List <kernel-testers@...r.kernel.org>,
	Rusty Russell <rusty@...tcorp.com.au>
Subject: Re: [patch] Re: [Bug #12100] resume (S2R) broken by Intel microcode
 module, on A110L

Dmitry Adamushko wrote:
> 2009/1/11 Ingo Molnar <mingo@...e.hu>:
>> * Ingo Molnar <mingo@...e.hu> wrote:
>>
>>> * Dmitry Adamushko <dmitry.adamushko@...il.com> wrote:
>>>
>>>> Hi,
>>>>
>>>>
>>>> This is in response to the following bug report:
>>>>
>>>> Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=12100
>>>> Subject         : resume (S2R) broken by Intel microcode module, on A110L
>>>> Submitter       : Andreas Mohr <andi@...as.de>
>>>> Date            : 2008-11-25 08:48 (19 days old)
>>>> Handled-By      : Dmitry Adamushko <dmitry.adamushko@...il.com>
>>> applied to tip/x86/microcode, thanks Dmitry!
>>>
>>> The fix looks right but somewhat intrusive in scope, so i'm a bit
>>> reluctant to push it towards .28 straight away - without having feedback
>>> in the bugzilla. If feedback is positive (the bug reported there goes
>>> away completely) we can cherry-pick it over into x86/urgent, ok? And in
>>> any case i've marked it as a -stable backport for .28.1.
>> hm, -tip testing just found this microcode locking lockdep splat:
>>
>> [   48.004158] SMP alternatives: switching to UP code
>> [   48.342853] CPU0 attaching NULL sched-domain.
>> [   48.344288] CPU1 attaching NULL sched-domain.
>> [   48.354696] CPU0 attaching NULL sched-domain.
>> [   48.361215] device: 'cpu1': device_unregister
>> [   48.364231] device: 'cpu1': device_create_release
>> [   48.368138]
>> [   48.368139] =======================================================
>> [   48.372039] [ INFO: possible circular locking dependency detected ]
>> [   48.372039] 2.6.29-rc1-tip-00901-g9699183-dirty #15577
>> [   48.372039] -------------------------------------------------------
>> [   48.372039] S99local/3496 is trying to acquire lock:
>> [   48.372039]  (microcode_mutex){--..}, at: [<c0118489>] microcode_fini_cpu+0x17/0x2b
>> [   48.372039]
>> [   48.372039] but task is already holding lock:
>> [   48.372039]  (&cpu_hotplug.lock){--..}, at: [<c012f508>] cpu_hotplug_begin+0x1f/0x47
>> [   48.372039]
>> [   48.372039] which lock already depends on the new lock.
>> [   48.372039]
>> [   48.372039]
>> [   48.372039] the existing dependency chain (in reverse order) is:
>> [   48.372039]
>> [   48.372039] -> #1 (&cpu_hotplug.lock){--..}:
>> [   48.372039]        [<c014d3f1>] validate_chain+0x8e9/0xb94
>> [   48.372039]        [<c014dd03>] __lock_acquire+0x667/0x6e1
>> [   48.372039]        [<c014ddda>] lock_acquire+0x5d/0x7a
>> [   48.372039]        [<c0a6fac3>] mutex_lock_nested+0xdc/0x170
>> [   48.372039]        [<c012f552>] get_online_cpus+0x22/0x34
>> [   48.372039]        [<c013ce08>] work_on_cpu+0x50/0x8a
>> [   48.372039]        [<c0118465>] microcode_init_cpu+0x25/0x32
>> [   48.372039]        [<c0118699>] mc_sysdev_add+0x91/0x9b
>> [   48.372039]        [<c04cbd09>] sysdev_driver_register+0x9b/0xea
> 
> I'll check more carefully... At the first glance, the presence of
> work_on_cpu() looks strange.
> 
> My first idea was that it's used somewhere by request_firmware() but
> even assuming some functions might have been inlined (and a call via a
> function pointer is not shown either), I don't immediately see how we
> might end up with microcode_init_cpu() -> ... -> work_on_cpu().

It was in a commit that (it appears) Ingo has reverted:

Subject: x86: cleanup remaining cpumask_t code in microcode_core.c

Impact: Reduce problem with changing current->cpus_allowed mask directly.

Use "work_on_cpu" to replace instances where set_cpus_allowed_ptr was being used.

Signed-off-by: Mike Travis <travis@....com>


This work_on_cpu is to replace setting current->cpus_allowed when it's
only for one cpu.  But it has a call to get_online_cpus() that (I believe)
is just to keep from offlining the cpu the work function is running on.
And it's also the cause of the circular lock dependencies.

Thanks,
Mike


> 
> I've locked up all the use cases of work_on_cpu() in the current -tip
> (about 20), and none of them seem to explain its appearance in the
> trace. weird...
> 
> 
>>        Ingo
>>
> 

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