linux-kernel - Re: kvm: use-after-free in process

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 13 Jan 2017 10:19:40 +0100
From:   Dmitry Vyukov <dvyukov@...gle.com>
To:     Steve Rutherford <srutherford@...gle.com>
Cc:     syzkaller <syzkaller@...glegroups.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        KVM list <kvm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: kvm: use-after-free in process_srcu

On Fri, Jan 13, 2017 at 4:30 AM, Steve Rutherford
<srutherford@...gle.com> 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.