Commit bf308242 authored by Andre Przywara's avatar Andre Przywara Committed by Paolo Bonzini

KVM: arm/arm64: VGIC/ITS: protect kvm_read_guest() calls with SRCU lock

kvm_read_guest() will eventually look up in kvm_memslots(), which requires
either to hold the kvm->slots_lock or to be inside a kvm->srcu critical
section.
In contrast to x86 and s390 we don't take the SRCU lock on every guest
exit, so we have to do it individually for each kvm_read_guest() call.

Provide a wrapper which does that and use that everywhere.

Note that ending the SRCU critical section before returning from the
kvm_read_guest() wrapper is safe, because the data has been *copied*, so
we don't need to rely on valid references to the memslot anymore.

Cc: Stable <stable@vger.kernel.org> # 4.8+
Reported-by: default avatarJan Glauber <jan.glauber@caviumnetworks.com>
Signed-off-by: default avatarAndre Przywara <andre.przywara@arm.com>
Acked-by: default avatarChristoffer Dall <christoffer.dall@arm.com>
Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
parent 9c418876
...@@ -309,6 +309,22 @@ static inline unsigned int kvm_get_vmid_bits(void) ...@@ -309,6 +309,22 @@ static inline unsigned int kvm_get_vmid_bits(void)
return 8; return 8;
} }
/*
* We are not in the kvm->srcu critical section most of the time, so we take
* the SRCU read lock here. Since we copy the data from the user page, we
* can immediately drop the lock again.
*/
static inline int kvm_read_guest_lock(struct kvm *kvm,
gpa_t gpa, void *data, unsigned long len)
{
int srcu_idx = srcu_read_lock(&kvm->srcu);
int ret = kvm_read_guest(kvm, gpa, data, len);
srcu_read_unlock(&kvm->srcu, srcu_idx);
return ret;
}
static inline void *kvm_get_hyp_vector(void) static inline void *kvm_get_hyp_vector(void)
{ {
return kvm_ksym_ref(__kvm_hyp_vector); return kvm_ksym_ref(__kvm_hyp_vector);
......
...@@ -360,6 +360,22 @@ static inline unsigned int kvm_get_vmid_bits(void) ...@@ -360,6 +360,22 @@ static inline unsigned int kvm_get_vmid_bits(void)
return (cpuid_feature_extract_unsigned_field(reg, ID_AA64MMFR1_VMIDBITS_SHIFT) == 2) ? 16 : 8; return (cpuid_feature_extract_unsigned_field(reg, ID_AA64MMFR1_VMIDBITS_SHIFT) == 2) ? 16 : 8;
} }
/*
* We are not in the kvm->srcu critical section most of the time, so we take
* the SRCU read lock here. Since we copy the data from the user page, we
* can immediately drop the lock again.
*/
static inline int kvm_read_guest_lock(struct kvm *kvm,
gpa_t gpa, void *data, unsigned long len)
{
int srcu_idx = srcu_read_lock(&kvm->srcu);
int ret = kvm_read_guest(kvm, gpa, data, len);
srcu_read_unlock(&kvm->srcu, srcu_idx);
return ret;
}
#ifdef CONFIG_KVM_INDIRECT_VECTORS #ifdef CONFIG_KVM_INDIRECT_VECTORS
/* /*
* EL2 vectors can be mapped and rerouted in a number of ways, * EL2 vectors can be mapped and rerouted in a number of ways,
......
...@@ -281,8 +281,8 @@ static int update_lpi_config(struct kvm *kvm, struct vgic_irq *irq, ...@@ -281,8 +281,8 @@ static int update_lpi_config(struct kvm *kvm, struct vgic_irq *irq,
int ret; int ret;
unsigned long flags; unsigned long flags;
ret = kvm_read_guest(kvm, propbase + irq->intid - GIC_LPI_OFFSET, ret = kvm_read_guest_lock(kvm, propbase + irq->intid - GIC_LPI_OFFSET,
&prop, 1); &prop, 1);
if (ret) if (ret)
return ret; return ret;
...@@ -444,8 +444,9 @@ static int its_sync_lpi_pending_table(struct kvm_vcpu *vcpu) ...@@ -444,8 +444,9 @@ static int its_sync_lpi_pending_table(struct kvm_vcpu *vcpu)
* this very same byte in the last iteration. Reuse that. * this very same byte in the last iteration. Reuse that.
*/ */
if (byte_offset != last_byte_offset) { if (byte_offset != last_byte_offset) {
ret = kvm_read_guest(vcpu->kvm, pendbase + byte_offset, ret = kvm_read_guest_lock(vcpu->kvm,
&pendmask, 1); pendbase + byte_offset,
&pendmask, 1);
if (ret) { if (ret) {
kfree(intids); kfree(intids);
return ret; return ret;
...@@ -789,7 +790,7 @@ static bool vgic_its_check_id(struct vgic_its *its, u64 baser, u32 id, ...@@ -789,7 +790,7 @@ static bool vgic_its_check_id(struct vgic_its *its, u64 baser, u32 id,
return false; return false;
/* Each 1st level entry is represented by a 64-bit value. */ /* Each 1st level entry is represented by a 64-bit value. */
if (kvm_read_guest(its->dev->kvm, if (kvm_read_guest_lock(its->dev->kvm,
BASER_ADDRESS(baser) + index * sizeof(indirect_ptr), BASER_ADDRESS(baser) + index * sizeof(indirect_ptr),
&indirect_ptr, sizeof(indirect_ptr))) &indirect_ptr, sizeof(indirect_ptr)))
return false; return false;
...@@ -1370,8 +1371,8 @@ static void vgic_its_process_commands(struct kvm *kvm, struct vgic_its *its) ...@@ -1370,8 +1371,8 @@ static void vgic_its_process_commands(struct kvm *kvm, struct vgic_its *its)
cbaser = CBASER_ADDRESS(its->cbaser); cbaser = CBASER_ADDRESS(its->cbaser);
while (its->cwriter != its->creadr) { while (its->cwriter != its->creadr) {
int ret = kvm_read_guest(kvm, cbaser + its->creadr, int ret = kvm_read_guest_lock(kvm, cbaser + its->creadr,
cmd_buf, ITS_CMD_SIZE); cmd_buf, ITS_CMD_SIZE);
/* /*
* If kvm_read_guest() fails, this could be due to the guest * If kvm_read_guest() fails, this could be due to the guest
* programming a bogus value in CBASER or something else going * programming a bogus value in CBASER or something else going
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment