On 8/4/23 12:33, Paolo Bonzini wrote:
The VMGEXIT handler has a time-of-check/time-of-use vulnerability; due to a double fetch, the guest can exploit a race condition to invoke the VMGEXIT handler recursively. It is extremely difficult to reliably win the race ~100 consecutive times in order to cause an overflow, and the impact is usually mitigated by CONFIG_VMAP_STACK, but it ought to be fixed anyway. One way to do so could be to snapshot the whole GHCB, but this is relatively expensive. Instead, because the VMGEXIT handler already syncs the GHCB to internal KVM state, this series makes sure that the GHCB is not read outside sev_es_sync_from_ghcb(). Patch 1 adds caching for fields that currently are not snapshotted in host memory; patch 2 ensures that the cached fields are always used, thus fixing the race. Finally patch 3 removes some local variables that are prone to incorrect use, to avoid reintroducing the race in other places. Please review!
Sorry, just got back from vacation... I'll review this as soon as I can. Thanks, Tom
Paolo Paolo Bonzini (3): KVM: SEV: snapshot the GHCB before accessing it KVM: SEV: only access GHCB fields once KVM: SEV: remove ghcb variable declarations arch/x86/kvm/svm/sev.c | 124 ++++++++++++++++++++--------------------- arch/x86/kvm/svm/svm.h | 26 +++++++++ 2 files changed, 87 insertions(+), 63 deletions(-)
