2017-10-13 19:31+0200, Christoffer Dall:

On Fri, Oct 13, 2017 at 07:13:07PM +0200, Radim Kr?m?? wrote:

quoted

I think that other (special) callsites of vcpu_load()/vcpu_put() have a
well defined IOCTL that can be used instead of vcpu->ioctl, so we could
just pass the ioctl value all the way to arch code and never save it
anywhere,

I don't think that works; what would you do with preempt notifier calls?

Right, BUG :), I didn't consider them before and they need to know.

One solution is to add a parameter to vcpu_put, lie for vcpu_load, which
also sets the ioctl, and other callers than the final vcpu_put in
kvm_vcpu_ioctl() just pass the existing value, where the kvm_vcpu_ioctl
call can pass 0 which gets set before releasing the mutex.

Can you think of a more elegant solution?

Not really, only thought of touching preempt notifiers and it seems to
be more complicated.

I think we shouldn't restore ioctl on vcpu_put() at all -- the value
isn't well defined outside of the mutex, so there is no point in looking
and we can just zero the ioctl.

Actually, I wouldn't rely on the existing value at all because that.
The need for load/put depends on the current code path, not on the one
we race with.

x86 seems to be the only user of vcpu_load() outside of kvm_vcpu_ioctl()
and the callers are either under a VM ioctl or under a VM destruction
paths (invalid IOCTL) and we can just hardcode that.

Passing 0 to all other vcpu_load()s and unconditionally zeroing ioctl
before mutex_unlock() should work.