From mboxrd@z Thu Jan  1 00:00:00 1970
From: Christian Borntraeger <borntraeger@de.ibm.com>
Subject: Re: [PATCH] static_key: fix concurrent static_key_slow_inc
Date: Wed, 22 Jun 2016 10:50:00 +0200
Message-ID: <576A5138.8040604@de.ibm.com>
References: <1466527937-69798-1-git-send-email-pbonzini@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Cc: stable@vger.kernel.org, Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>
To: Paolo Bonzini <pbonzini@redhat.com>, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:18091 "EHLO
	mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK)
	by vger.kernel.org with ESMTP id S1751572AbcFVIuJ (ORCPT
	<rfc822;kvm@vger.kernel.org>); Wed, 22 Jun 2016 04:50:09 -0400
Received: from pps.filterd (m0098404.ppops.net [127.0.0.1])
	by mx0a-001b2d01.pphosted.com (8.16.0.11/8.16.0.11) with SMTP id u5M8ncpE086024
	for <kvm@vger.kernel.org>; Wed, 22 Jun 2016 04:50:08 -0400
Received: from e06smtp05.uk.ibm.com (e06smtp05.uk.ibm.com [195.75.94.101])
	by mx0a-001b2d01.pphosted.com with ESMTP id 23qaeeek1m-1
	(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)
	for <kvm@vger.kernel.org>; Wed, 22 Jun 2016 04:50:08 -0400
Received: from localhost
	by e06smtp05.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted
	for <kvm@vger.kernel.org> from <borntraeger@de.ibm.com>;
	Wed, 22 Jun 2016 09:50:06 +0100
In-Reply-To: <1466527937-69798-1-git-send-email-pbonzini@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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 <fcntl.h>
>     #include <unistd.h>
>     #include <sys/ioctl.h>
>     #include <linux/kvm.h>
> 
>     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.