From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752451AbcFVKEt (ORCPT ); Wed, 22 Jun 2016 06:04:49 -0400 Received: from mail-lf0-f65.google.com ([209.85.215.65]:34446 "EHLO mail-lf0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752396AbcFVKEq (ORCPT ); Wed, 22 Jun 2016 06:04:46 -0400 Subject: Re: [PATCH] static_key: fix concurrent static_key_slow_inc To: Christian Borntraeger , linux-kernel@vger.kernel.org, kvm@vger.kernel.org References: <1466527937-69798-1-git-send-email-pbonzini@redhat.com> <576A5138.8040604@de.ibm.com> Cc: stable@vger.kernel.org, Peter Zijlstra , Ingo Molnar From: Paolo Bonzini Message-ID: <09161aef-e96c-1a44-733a-9bebaac4996a@redhat.com> Date: Wed, 22 Jun 2016 11:52:36 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <576A5138.8040604@de.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 22/06/2016 10:50, Christian Borntraeger wrote: > On 06/21/2016 06:52 PM, Paolo Bonzini wrote: >> The following scenario is possible: >> >> CPU 1 CPU 2 >> static_key_slow_inc >> atomic_inc_not_zero >> -> key.enabled == 0, no increment >> jump_label_lock >> atomic_inc_return >> -> key.enabled == 1 now >> static_key_slow_inc >> atomic_inc_not_zero >> -> key.enabled == 1, inc to 2 >> return >> ** static key is wrong! >> jump_label_update >> jump_label_unlock >> >> Testing the static key at the point marked by (**) will follow the wrong >> path for jumps that have not been patched yet. This can actually happen >> when creating many KVM virtual machines with userspace LAPIC emulation; >> just run several copies of the following program: >> >> #include >> #include >> #include >> #include >> >> int main(void) >> { >> for (;;) { >> int kvmfd = open("/dev/kvm", O_RDONLY); >> int vmfd = ioctl(kvmfd, KVM_CREATE_VM, 0); >> close(ioctl(vmfd, KVM_CREATE_VCPU, 1)); >> close(vmfd); >> close(kvmfd); >> } >> return 0; >> } >> >> Every KVM_CREATE_VCPU ioctl will attempt a static_key_slow_inc. The >> static key's purpose is to skip NULL pointer checks and indeed one of >> the processes eventually dereferences NULL. > > Interesting. Some time ago I had a spurious bug on the preempt_notifier > when starting/stopping lots of guests, but I was never able to reliably > reproduce it. I was chasing some other bug, so I did not even considered > static_key to be broken, but this might actually be the fix for that > problem. It could be the same that was reported here: http://article.gmane.org/gmane.comp.emulators.kvm.devel/154069 Paolo