Re: kvm: use-after-free in process_srcu

[Dmitry Vyukov]

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.