Re: kvm: use-after-free in process_srcu

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jan 13, 2017 at 10:19 AM, Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote:
> On Fri, Jan 13, 2017 at 4:30 AM, Steve Rutherford
> <srutherford@xxxxxxxxxx> wrote:
>> I'm not that familiar with the kernel's workqueues, but this seems
>> like the classic "callback outlives the memory it references"
>> use-after-free, where the process_srcu callback is outliving struct
>> kvm (which contains the srcu_struct). If that's right, then calling
>> srcu_barrier (which should wait for all of the call_srcu callbacks to
>> complete, which are what enqueue the process_srcu callbacks) before
>> cleanup_srcu_struct in kvm_destroy_vm probably fixes this.
>>
>> The corresponding patch to virt/kvm/kvm_main.c looks something like:
>> static void kvm_destroy_vm(struct kvm *kvm)
>> {
>> ...
>>         for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++)
>>                 kvm_free_memslots(kvm, kvm->memslots[i]);
>> +      srcu_barrier(&kvm->irq_srcu);
>>         cleanup_srcu_struct(&kvm->irq_srcu);
>> +      srcu_barrier(&kvm->srcu);
>>         cleanup_srcu_struct(&kvm->srcu);
>> ...
>>
>>
>> Since we don't have a repro, this obviously won't be readily testable.
>> I find srcu subtle enough that I don't trust my reasoning fully (in
>> particular, I don't trust that waiting for all of the call_srcu
>> callbacks to complete also waits for all of the process_srcu
>> callbacks). Someone else know if that's the case?
>
>
> From the function description it looks like it should do the trick:
>
> 514 /**
> 515  * srcu_barrier - Wait until all in-flight call_srcu() callbacks complete.
> 516  * @sp: srcu_struct on which to wait for in-flight callbacks.
> 517  */
> 518 void srcu_barrier(struct srcu_struct *sp)
>
> I see this failure happening several times per day. I've applied your
> patch locally and will check if I see these failures happening.


I have not seen the crash in 3 days, when usually I see several
crashes per night. So I think we can consider that the patch fixes the
crash:

Tested-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux