Messages in this thread | | | Date | Tue, 21 Jun 2016 21:20:02 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH] static_key: fix concurrent static_key_slow_inc |
| |
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).
| |