linux-kernel - Re: [PATCH] KVM: Deal with nested sleeps in kvm_vcpu

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]

Message-ID: <CAOLenvaUpnk=w7TSArwK-WqUsQDQ=GY=vOHcW4apH1E5hWTCJg@mail.gmail.com>
Date:   Fri, 2 Dec 2022 16:52:20 +0100
From:   Space Meyer <spm@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        kpsingh@...nel.org
Subject: Re: [PATCH] KVM: Deal with nested sleeps in kvm_vcpu_block()

On Wed, Nov 30, 2022 at 5:59 PM Paolo Bonzini <pbonzini@...hat.com> wrote:
>
> On 11/30/22 17:19, Space Meyer wrote:
> >   bool kvm_vcpu_block(struct kvm_vcpu *vcpu)
> >   {
> > +     DEFINE_WAIT_FUNC(vcpu_block_wait, woken_wake_function);
> >       struct rcuwait *wait = kvm_arch_vcpu_get_wait(vcpu);
> >       bool waited = false;
> >
> > @@ -3437,13 +3439,11 @@ bool kvm_vcpu_block(struct kvm_vcpu *vcpu)
> >       preempt_enable();
> >
> >       for (;;) {
> > -             set_current_state(TASK_INTERRUPTIBLE);
> > -
> >               if (kvm_vcpu_check_block(vcpu) < 0)
> >                       break;
> >
> >               waited = true;
> > -             schedule();
> > +             wait_woken(&vcpu_block_wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
> >       }
>
> Also, this does not work I think, because there is
> add_wait_queue()/remove_wait_queue() pair.  Adding it is not easy
> because KVM is using a struct rcuwait here instead of a wait_queue_t.

Ah, sorry. I really was a bit quick on this one. I agree nothing would ever call
woken_wake_function, hence my patch doesn't make sense. Looking at the rcuwait
code I don't see something similar to wait_woken.

Do you see some other way we could avoid the pattern susceptible to the nested
sleeping problem?