On 12/21/2017 01:03 PM, David Hildenbrand wrote:
> On 21.12.2017 12:57, David Hildenbrand wrote:
>> On 21.12.2017 10:04, Christian Borntraeger wrote:
>>> This is targetted as minimal fixups. Claudio is reworking this
>>> to handle this better in general (e.g. memory wholes).
>>>
>>> I will wait for a review for patch2 (which is new) and then spin
>>> a pull request as we now have 2 patches for master
>>>
>>> Christian Borntraeger (2):
>>> KVM: s390: fix cmma migration for multiple memory slots
>>> KVM: s390: prevent buffer overrun on memory hotplug during migration
>>>
>>> arch/s390/kvm/kvm-s390.c | 9 +++++----
>>> arch/s390/kvm/priv.c | 2 +-
>>> 2 files changed, 6 insertions(+), 5 deletions(-)
>>>
>>
>> [waking up from vacation mode]
>>
>> Sorry to say, but I guess there is more.
>>
>> do_essa() can race with
>> kvm_s390_vm_set_migration(KVM_S390_VM_MIGRATION_STOP).
>>
>> CPU0: handle_essa()
>> -> vcpu->kvm->arch.migration_state == true
>> -> do_essa()
>> CPU1: kvm_s390_vm_set_migration(KVM_S390_VM_MIGRATION_STOP)
>> -> vcpu->kvm->arch.migration_state = false;
>> -> vfree(mgs->pgste_bitmap)
>> CPU0: test_and_set_bit(gfn, ms->pgste_bitmap)
>> -> access to freed data structure
>>
>> The kvm_s390_sync_request_broadcast(kvm, KVM_REQ_STOP_MIGRATION) will
>> only make sure that all CPUs have left SIE mode, not that the request
>> has been processed.
>>
>> Does that make sense?
>>
>> [going back to vacation mode]
>>
>
> FWIW, doesn't apply the same thing to kvm_s390_get_cmma_bits(), too? I
> can't find locking while quickly glimpsing over it. But I might be wrong.
If there is a bug in a piece of code it is certainly a good idea to look deeper....
I think we can prevent both bugs by something like
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 8951ad4d33be..6ee12afced5a 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -841,6 +841,7 @@ static int kvm_s390_vm_stop_migration(struct kvm *kvm)
if (kvm->arch.use_cmma) {
kvm_s390_sync_request_broadcast(kvm, KVM_REQ_STOP_MIGRATION);
+ synchronize_srcu(&kvm->srcu);
vfree(mgs->pgste_bitmap);
}
kfree(mgs);
Need to have a look.
--
To unsubscribe from this list: send the line "unsubscribe linux-s390" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
