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).