BACKPORT: KVM: arm64: Make vcpu flag updates non-preemptible

Per-vcpu flags are updated using a non-atomic RMW operation.
Which means it is possible to get preempted between the read and
write operations.

Another interesting thing to note is that preemption also updates
flags, as we have some flag manipulation in both the load and put
operations.

It is thus possible to lose information communicated by either
load or put, as the preempted flag update will overwrite the flags
when the thread is resumed. This is specially critical if either
load or put has stored information which depends on the physical
CPU the vcpu runs on.

This results in really elusive bugs, and kudos must be given to
Mostafa for the long hours of debugging, and finally spotting
the problem.

Fix it by disabling preemption during the RMW operation, which
ensures that the state stays consistent. Also upgrade vcpu_get_flag
path to use READ_ONCE() to make sure the field is always atomically
accessed.

Fixes: e87abb73e5 ("KVM: arm64: Add helpers to manipulate vcpu flags among a set")
Reported-by: Mostafa Saleh <smostafa@google.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20230418125737.2327972-1-maz@kernel.org
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
(cherry picked from commit 35dcb3ac66)
[willdeacon@: also update __vcpu_copy_flag()]
Signed-off-by: Will Deacon <willdeacon@google.com>
Bug: 278750073
Change-Id: I63058ff1494e4092dab9d29cb66c295dd8fe9d86
This commit is contained in:
Marc Zyngier
2023-04-18 13:57:37 +01:00
committed by Will Deacon
parent 73e072e857
commit ef8b5cddf1

View File

@@ -562,9 +562,22 @@ struct kvm_vcpu_arch {
({ \
__build_check_flag(v, flagset, f, m); \
\
v->arch.flagset & (m); \
READ_ONCE(v->arch.flagset) & (m); \
})
/*
* Note that the set/clear accessors must be preempt-safe in order to
* avoid nesting them with load/put which also manipulate flags...
*/
#ifdef __KVM_NVHE_HYPERVISOR__
/* the nVHE hypervisor is always non-preemptible */
#define __vcpu_flags_preempt_disable()
#define __vcpu_flags_preempt_enable()
#else
#define __vcpu_flags_preempt_disable() preempt_disable()
#define __vcpu_flags_preempt_enable() preempt_enable()
#endif
#define __vcpu_set_flag(v, flagset, f, m) \
do { \
typeof(v->arch.flagset) *fset; \
@@ -572,9 +585,11 @@ struct kvm_vcpu_arch {
__build_check_flag(v, flagset, f, m); \
\
fset = &v->arch.flagset; \
__vcpu_flags_preempt_disable(); \
if (HWEIGHT(m) > 1) \
*fset &= ~(m); \
*fset |= (f); \
__vcpu_flags_preempt_enable(); \
} while (0)
#define __vcpu_clear_flag(v, flagset, f, m) \
@@ -584,7 +599,9 @@ struct kvm_vcpu_arch {
__build_check_flag(v, flagset, f, m); \
\
fset = &v->arch.flagset; \
__vcpu_flags_preempt_disable(); \
*fset &= ~(m); \
__vcpu_flags_preempt_enable(); \
} while (0)
#define __vcpu_copy_flag(vt, vs, flagset, f, m) \
@@ -593,12 +610,14 @@ struct kvm_vcpu_arch {
\
__build_check_flag(vs, flagset, f, m); \
\
__vcpu_flags_preempt_disable(); \
val = READ_ONCE(vs->arch.flagset); \
val &= (m); \
tmp = READ_ONCE(vt->arch.flagset); \
tmp &= ~(m); \
tmp |= val; \
WRITE_ONCE(vt->arch.flagset, tmp); \
__vcpu_flags_preempt_enable(); \
} while (0)