| 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
| ||
|
Date: Tue, 18 Mar 2014 15:08:06 -0700
From: Christoffer Dall <christoffer.dall@...aro.org>
To: "Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>
Cc: ego@...ux.vnet.ibm.com,
"kvm@...r.kernel.org" <kvm@...r.kernel.org>, peterz@...radead.org,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
linuxppc-dev@...abs.org, Paul Mackerras <paulus@...ba.org>,
Michel Lespinasse <walken@...gle.com>,
"kvmarm@...ts.cs.columbia.edu" <kvmarm@...ts.cs.columbia.edu>,
linux-arch@...r.kernel.org, Russell King <linux@....linux.org.uk>,
mingo@...nel.org, Marc Zyngier <marc.zyngier@....com>,
Paul McKenney <paulmck@...ux.vnet.ibm.com>,
linux-pm@...r.kernel.org, Gleb Natapov <gleb@...nel.org>,
Rusty Russell <rusty@...tcorp.com.au>,
Thomas Gleixner <tglx@...utronix.de>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, rjw@...ysocki.net,
oleg@...hat.com, tj@...nel.org,
Paolo Bonzini <pbonzini@...hat.com>, akpm@...ux-foundation.org
Subject: Re: [UPDATED PATCH v3 10/52] arm, kvm: Fix CPU hotplug callback registration
On 18 March 2014 03:23, Srivatsa S. Bhat
<srivatsa.bhat@...ux.vnet.ibm.com> wrote:
> On 03/15/2014 12:40 AM, Christoffer Dall wrote:
>> On Fri, Mar 14, 2014 at 11:13:29AM +0530, Srivatsa S. Bhat wrote:
>>> On 03/13/2014 04:51 AM, Christoffer Dall wrote:
>>>> On Tue, Mar 11, 2014 at 02:05:38AM +0530, Srivatsa S. Bhat wrote:
>>>>> Subsystems that want to register CPU hotplug callbacks, as well as perform
>>>>> initialization for the CPUs that are already online, often do it as shown
>>>>> below:
>>>>>
> [...]
>>>> Just so we're clear, the existing code was simply racy as not prone to
>>>> deadlocks, right?
>>>>
>>>> This makes it clear that the test above for compatible CPUs can be quite
>>>> easily evaded by using CPU hotplug, but we don't really have a good
>>>> solution for handling that yet... Hmmm, grumble grumble, I guess if you
>>>> hotplug unsupported CPUs on a KVM/ARM system for now, stuff will break.
>>>>
>>>
>>> In this particular case, there was no deadlock possibility, rather the
>>> existing code had insufficient synchronization against CPU hotplug.
>>>
>>> init_hyp_mode() would invoke cpu_init_hyp_mode() on currently online CPUs
>>> using on_each_cpu(). If a CPU came online after this point and before calling
>>> register_cpu_notifier(), that CPU would remain uninitialized because this
>>> subsystem would miss the hot-online event. This patch fixes this bug and
>>> also uses the new synchronization method (instead of get/put_online_cpus())
>>> to ensure that we don't deadlock with CPU hotplug.
>>>
>>
>> Yes, that was my conclusion as well. Thanks for clarifying. (It could
>> be noted in the commit message as well if you should feel so inclined).
>>
>
> Please find the patch with updated changelog (and your Ack) below.
> (No changes in code).
>
perfect, thanks!
> From: Srivatsa S. Bhat <srivatsa.bhat@...ux.vnet.ibm.com>
> Subject: [PATCH] arm, kvm: Fix CPU hotplug callback registration
>
> Subsystems that want to register CPU hotplug callbacks, as well as perform
> initialization for the CPUs that are already online, often do it as shown
> below:
>
> get_online_cpus();
>
> for_each_online_cpu(cpu)
> init_cpu(cpu);
>
> register_cpu_notifier(&foobar_cpu_notifier);
>
> put_online_cpus();
>
> This is wrong, since it is prone to ABBA deadlocks involving the
> cpu_add_remove_lock and the cpu_hotplug.lock (when running concurrently
> with CPU hotplug operations).
>
> Instead, the correct and race-free way of performing the callback
> registration is:
>
> cpu_notifier_register_begin();
>
> for_each_online_cpu(cpu)
> init_cpu(cpu);
>
> /* Note the use of the double underscored version of the API */
> __register_cpu_notifier(&foobar_cpu_notifier);
>
> cpu_notifier_register_done();
>
>
> In the existing arm kvm code, there is no synchronization with CPU hotplug
> to avoid missing the hotplug events that might occur after invoking
> init_hyp_mode() and before calling register_cpu_notifier(). Fix this bug
> and also use the new synchronization method (instead of get/put_online_cpus())
> to ensure that we don't deadlock with CPU hotplug.
>
> Cc: Gleb Natapov <gleb@...nel.org>
> Cc: Russell King <linux@....linux.org.uk>
> Cc: Ingo Molnar <mingo@...nel.org>
> Cc: kvmarm@...ts.cs.columbia.edu
> Cc: kvm@...r.kernel.org
> Cc: linux-arm-kernel@...ts.infradead.org
> Acked-by: Paolo Bonzini <pbonzini@...hat.com>
> Acked-by: Christoffer Dall <christoffer.dall@...aro.org>
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@...ux.vnet.ibm.com>
> ---
>
> arch/arm/kvm/arm.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index bd18bb8..f0e50a0 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -1051,21 +1051,26 @@ int kvm_arch_init(void *opaque)
> }
> }
>
> + cpu_notifier_register_begin();
> +
> err = init_hyp_mode();
> if (err)
> goto out_err;
>
> - err = register_cpu_notifier(&hyp_init_cpu_nb);
> + err = __register_cpu_notifier(&hyp_init_cpu_nb);
> if (err) {
> kvm_err("Cannot register HYP init CPU notifier (%d)\n", err);
> goto out_err;
> }
>
> + cpu_notifier_register_done();
> +
> hyp_cpu_pm_init();
>
> kvm_coproc_table_init();
> return 0;
> out_err:
> + cpu_notifier_register_done();
> return err;
> }
>
>
>
--
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/
Powered by blists - more mailing lists