From: Peter Zijlstra <peterz@infradead.org>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
stable@vger.kernel.org, Ingo Molnar <mingo@kernel.org>
Subject: Re: [PATCH] static_key: fix concurrent static_key_slow_inc
Date: Tue, 21 Jun 2016 21:20:02 +0200 [thread overview]
Message-ID: <20160621192002.GS30154@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <1466527937-69798-1-git-send-email-pbonzini@redhat.com>
On Tue, Jun 21, 2016 at 06:52:17PM +0200, 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.
>
> As explained in the commit that introduced the bug (which is 706249c222f6,
> "locking/static_keys: Rework update logic", 2015-07-24), jump_label_update
> needs key.enabled to be true. The solution adopted here is to temporarily
> make key.enabled == -1, and use go down the slow path when key.enabled
> <= 0.
Thanks!
(I frobbed a whitespace fail and fixed the Fixes line).
next prev parent reply other threads:[~2016-06-21 19:20 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-21 16:52 [PATCH] static_key: fix concurrent static_key_slow_inc Paolo Bonzini
2016-06-21 19:20 ` Peter Zijlstra [this message]
2016-06-22 8:50 ` Christian Borntraeger
2016-06-22 9:52 ` Paolo Bonzini
2016-06-24 8:59 ` [tip:locking/urgent] locking/static_key: Fix concurrent static_key_slow_inc() tip-bot for Paolo Bonzini
2017-07-31 9:36 ` Peter Zijlstra
2017-07-31 13:04 ` Paolo Bonzini
2017-07-31 13:33 ` Peter Zijlstra
2017-07-31 15:35 ` Paolo Bonzini
2017-07-31 15:45 ` Peter Zijlstra
2017-07-31 19:21 ` Paolo Bonzini
2017-07-31 17:15 ` Dima Zavin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160621192002.GS30154@twins.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=pbonzini@redhat.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).