From: Sean Christopherson <seanjc@google•com>
To: Maxim Levitsky <mlevitsk@redhat•com>
Cc: Albert Ou <aou@eecs•berkeley.edu>,
Wanpeng Li <wanpengli@tencent•com>,
Janosch Frank <frankja@linux•ibm.com>,
kvm@vger•kernel.org, David Hildenbrand <david@redhat•com>,
Claudio Imbrenda <imbrenda@linux•ibm.com>,
Anup Patel <anup@brainfault•org>, Joerg Roedel <joro@8bytes•org>,
Atish Patra <atishp@atishpatra•org>,
linux-kernel@vger•kernel.org, Palmer Dabbelt <palmer@dabbelt•com>,
kvm-riscv@lists•infradead.org,
Paul Walmsley <paul.walmsley@sifive•com>,
Paolo Bonzini <pbonzini@redhat•com>,
Vitaly Kuznetsov <vkuznets@redhat•com>,
Christian Borntraeger <borntraeger@linux•ibm.com>,
linuxppc-dev@lists•ozlabs.org, linux-riscv@lists•infradead.org,
Jim Mattson <jmattson@google•com>
Subject: Re: [PATCH 3/3] KVM: Add helpers to wrap vcpu->srcu_idx and yell if it's abused
Date: Tue, 19 Apr 2022 15:45:52 +0000 [thread overview]
Message-ID: <Yl7ZMJ/takmHh7tY@google.com> (raw)
In-Reply-To: <5b561bf1a0bbf140ea09d516f946a4e8fee8dd2d.camel@redhat.com>
On Tue, Apr 19, 2022, Maxim Levitsky wrote:
> On Fri, 2022-04-15 at 00:43 +0000, Sean Christopherson wrote:
> > Add wrappers to acquire/release KVM's SRCU lock when stashing the index
> > in vcpu->src_idx, along with rudimentary detection of illegal usage,
> > e.g. re-acquiring SRCU and thus overwriting vcpu->src_idx. Because the
> > SRCU index is (currently) either 0 or 1, illegal nesting bugs can go
> > unnoticed for quite some time and only cause problems when the nested
> > lock happens to get a different index.
> >
> > Wrap the WARNs in PROVE_RCU=y, and make them ONCE, otherwise KVM will
> > likely yell so loudly that it will bring the kernel to its knees.
> >
> > Signed-off-by: Sean Christopherson <seanjc@google•com>
> > ---
...
> Looks good to me overall.
>
> Note that there are still places that acquire the lock and store the idx into
> a local variable, for example kvm_xen_vcpu_set_attr and such.
> I didn't check yet if these should be converted as well.
Using a local variable is ok, even desirable. Nested/multiple readers is not an
issue, the bug fixed by patch 1 is purely that kvm_vcpu.srcu_idx gets corrupted.
In an ideal world, KVM would _only_ track the SRCU index in local variables, but
that would require plumbing the local variable down into vcpu_enter_guest() and
kvm_vcpu_block() so that SRCU can be unlocked prior to entering the guest or
scheduling out the vCPU.
next prev parent reply other threads:[~2022-04-19 15:46 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-15 0:43 [PATCH 0/3] KVM: x86 SRCU bug fix and SRCU hardening Sean Christopherson
2022-04-15 0:43 ` [PATCH 1/3] KVM: x86: Don't re-acquire SRCU lock in complete_emulated_io() Sean Christopherson
2022-04-19 8:55 ` Maxim Levitsky
2022-04-15 0:43 ` [PATCH 2/3] KVM: RISC-V: Use kvm_vcpu.srcu_idx, drop RISC-V's unnecessary copy Sean Christopherson
2022-04-15 0:43 ` [PATCH 3/3] KVM: Add helpers to wrap vcpu->srcu_idx and yell if it's abused Sean Christopherson
2022-04-19 9:04 ` Maxim Levitsky
2022-04-19 15:45 ` Sean Christopherson [this message]
2022-04-20 4:36 ` Maxim Levitsky
2022-04-19 17:18 ` Fabiano Rosas
2022-04-20 10:00 ` [PATCH 0/3] KVM: x86 SRCU bug fix and SRCU hardening Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Yl7ZMJ/takmHh7tY@google.com \
--to=seanjc@google$(echo .)com \
--cc=anup@brainfault$(echo .)org \
--cc=aou@eecs$(echo .)berkeley.edu \
--cc=atishp@atishpatra$(echo .)org \
--cc=borntraeger@linux$(echo .)ibm.com \
--cc=david@redhat$(echo .)com \
--cc=frankja@linux$(echo .)ibm.com \
--cc=imbrenda@linux$(echo .)ibm.com \
--cc=jmattson@google$(echo .)com \
--cc=joro@8bytes$(echo .)org \
--cc=kvm-riscv@lists$(echo .)infradead.org \
--cc=kvm@vger$(echo .)kernel.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-riscv@lists$(echo .)infradead.org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=mlevitsk@redhat$(echo .)com \
--cc=palmer@dabbelt$(echo .)com \
--cc=paul.walmsley@sifive$(echo .)com \
--cc=pbonzini@redhat$(echo .)com \
--cc=vkuznets@redhat$(echo .)com \
--cc=wanpengli@tencent$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox