All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Mackerras <paulus@ozlabs.org>
To: Greg Kurz <groug@kaod.org>
Cc: Laurent Dufour <ldufour@linux.ibm.com>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	kvm-ppc@vger.kernel.org, Bharata B Rao <bharata@linux.ibm.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Michael Ellerman <mpe@ellerman.id.au>
Subject: Re: [PATCH 1/2] KVM: PPC: Book3S HV: check caller of H_SVM_* Hcalls
Date: Tue, 24 Mar 2020 10:43:23 +1100	[thread overview]
Message-ID: <20200323234323.GA5604@blackberry> (raw)
In-Reply-To: <20200320132248.44b81b3b@bahia.lan>

On Fri, Mar 20, 2020 at 01:22:48PM +0100, Greg Kurz wrote:
> On Fri, 20 Mar 2020 11:26:42 +0100
> Laurent Dufour <ldufour@linux.ibm.com> wrote:
> 
> > The Hcall named H_SVM_* are reserved to the Ultravisor. However, nothing
> > prevent a malicious VM or SVM to call them. This could lead to weird result
> > and should be filtered out.
> > 
> > Checking the Secure bit of the calling MSR ensure that the call is coming
> > from either the Ultravisor or a SVM. But any system call made from a SVM
> > are going through the Ultravisor, and the Ultravisor should filter out
> > these malicious call. This way, only the Ultravisor is able to make such a
> > Hcall.
> 
> "Ultravisor should filter" ? And what if it doesn't (eg. because of a bug) ?
> 
> Shouldn't we also check the HV bit of the calling MSR as well to
> disambiguate SVM and UV ?

The trouble with doing that (checking the HV bit) is that KVM does not
expect to see the HV bit set on an interrupt that occurred while we
were in the guest, and if it is set, it indicates a serious problem,
i.e. that an interrupt occurred while we were in the code that
transitions from host context to guest context, or from guest context
to host context.  In those cases we don't know how much of the
transition has been completed and therefore whether we have guest
values or host values in the CPU registers (GPRs, FPRs/VSRs, SPRs).
If we do see HV set then KVM reports a severe error to userspace which
should cause userspace to terminate the guest.

Therefore the UV should *always* have the HV bit clear in HSRR1/SRR1
when transitioning to KVM.

Paul.

WARNING: multiple messages have this Message-ID (diff)
From: Paul Mackerras <paulus@ozlabs.org>
To: Greg Kurz <groug@kaod.org>
Cc: linux-kernel@vger.kernel.org, kvm-ppc@vger.kernel.org,
	Bharata B Rao <bharata@linux.ibm.com>,
	Laurent Dufour <ldufour@linux.ibm.com>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH 1/2] KVM: PPC: Book3S HV: check caller of H_SVM_* Hcalls
Date: Tue, 24 Mar 2020 10:43:23 +1100	[thread overview]
Message-ID: <20200323234323.GA5604@blackberry> (raw)
In-Reply-To: <20200320132248.44b81b3b@bahia.lan>

On Fri, Mar 20, 2020 at 01:22:48PM +0100, Greg Kurz wrote:
> On Fri, 20 Mar 2020 11:26:42 +0100
> Laurent Dufour <ldufour@linux.ibm.com> wrote:
> 
> > The Hcall named H_SVM_* are reserved to the Ultravisor. However, nothing
> > prevent a malicious VM or SVM to call them. This could lead to weird result
> > and should be filtered out.
> > 
> > Checking the Secure bit of the calling MSR ensure that the call is coming
> > from either the Ultravisor or a SVM. But any system call made from a SVM
> > are going through the Ultravisor, and the Ultravisor should filter out
> > these malicious call. This way, only the Ultravisor is able to make such a
> > Hcall.
> 
> "Ultravisor should filter" ? And what if it doesn't (eg. because of a bug) ?
> 
> Shouldn't we also check the HV bit of the calling MSR as well to
> disambiguate SVM and UV ?

The trouble with doing that (checking the HV bit) is that KVM does not
expect to see the HV bit set on an interrupt that occurred while we
were in the guest, and if it is set, it indicates a serious problem,
i.e. that an interrupt occurred while we were in the code that
transitions from host context to guest context, or from guest context
to host context.  In those cases we don't know how much of the
transition has been completed and therefore whether we have guest
values or host values in the CPU registers (GPRs, FPRs/VSRs, SPRs).
If we do see HV set then KVM reports a severe error to userspace which
should cause userspace to terminate the guest.

Therefore the UV should *always* have the HV bit clear in HSRR1/SRR1
when transitioning to KVM.

Paul.

WARNING: multiple messages have this Message-ID (diff)
From: Paul Mackerras <paulus@ozlabs.org>
To: Greg Kurz <groug@kaod.org>
Cc: Laurent Dufour <ldufour@linux.ibm.com>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	kvm-ppc@vger.kernel.org, Bharata B Rao <bharata@linux.ibm.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Michael Ellerman <mpe@ellerman.id.au>
Subject: Re: [PATCH 1/2] KVM: PPC: Book3S HV: check caller of H_SVM_* Hcalls
Date: Mon, 23 Mar 2020 23:43:23 +0000	[thread overview]
Message-ID: <20200323234323.GA5604@blackberry> (raw)
In-Reply-To: <20200320132248.44b81b3b@bahia.lan>

On Fri, Mar 20, 2020 at 01:22:48PM +0100, Greg Kurz wrote:
> On Fri, 20 Mar 2020 11:26:42 +0100
> Laurent Dufour <ldufour@linux.ibm.com> wrote:
> 
> > The Hcall named H_SVM_* are reserved to the Ultravisor. However, nothing
> > prevent a malicious VM or SVM to call them. This could lead to weird result
> > and should be filtered out.
> > 
> > Checking the Secure bit of the calling MSR ensure that the call is coming
> > from either the Ultravisor or a SVM. But any system call made from a SVM
> > are going through the Ultravisor, and the Ultravisor should filter out
> > these malicious call. This way, only the Ultravisor is able to make such a
> > Hcall.
> 
> "Ultravisor should filter" ? And what if it doesn't (eg. because of a bug) ?
> 
> Shouldn't we also check the HV bit of the calling MSR as well to
> disambiguate SVM and UV ?

The trouble with doing that (checking the HV bit) is that KVM does not
expect to see the HV bit set on an interrupt that occurred while we
were in the guest, and if it is set, it indicates a serious problem,
i.e. that an interrupt occurred while we were in the code that
transitions from host context to guest context, or from guest context
to host context.  In those cases we don't know how much of the
transition has been completed and therefore whether we have guest
values or host values in the CPU registers (GPRs, FPRs/VSRs, SPRs).
If we do see HV set then KVM reports a severe error to userspace which
should cause userspace to terminate the guest.

Therefore the UV should *always* have the HV bit clear in HSRR1/SRR1
when transitioning to KVM.

Paul.

  parent reply	other threads:[~2020-03-24  0:55 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-20 10:26 [PATCH 0/2] Fix SVM hang at startup Laurent Dufour
2020-03-20 10:26 ` Laurent Dufour
2020-03-20 10:26 ` [PATCH 1/2] KVM: PPC: Book3S HV: check caller of H_SVM_* Hcalls Laurent Dufour
2020-03-20 10:26   ` Laurent Dufour
2020-03-20 10:26   ` Laurent Dufour
2020-03-20 12:22   ` Greg Kurz
2020-03-20 12:22     ` Greg Kurz
2020-03-20 12:22     ` Greg Kurz
2020-03-20 14:43     ` Laurent Dufour
2020-03-20 14:43       ` Laurent Dufour
2020-03-20 14:43       ` Laurent Dufour
2020-03-23 23:43     ` Paul Mackerras [this message]
2020-03-23 23:43       ` Paul Mackerras
2020-03-23 23:43       ` Paul Mackerras
2020-03-24 12:00       ` Greg Kurz
2020-03-24 12:00         ` Greg Kurz
2020-03-24 12:00         ` Greg Kurz
2020-03-24 13:13         ` Laurent Dufour
2020-03-24 13:13           ` Laurent Dufour
2020-03-24 13:13           ` Laurent Dufour
2020-03-21  0:40   ` Ram Pai
2020-03-21  0:40     ` Ram Pai
2020-03-20 10:26 ` [PATCH 2/2] KVM: PPC: Book3S HV: H_SVM_INIT_START must call UV_RETURN Laurent Dufour
2020-03-20 10:26   ` Laurent Dufour
2020-03-20 10:26   ` Laurent Dufour
2020-03-20 11:24   ` Bharata B Rao
2020-03-20 11:36     ` Bharata B Rao
2020-03-20 11:24     ` Bharata B Rao
2020-03-20 14:36     ` Laurent Dufour
2020-03-20 14:36       ` Laurent Dufour
2020-03-20 14:36       ` Laurent Dufour
2020-03-23  4:21       ` Bharata B Rao
2020-03-23  4:33         ` Bharata B Rao
2020-03-23  4:21         ` Bharata B Rao
2020-03-21  0:47   ` Ram Pai
2020-03-21  0:47     ` Ram Pai
2020-03-21  0:47     ` Ram Pai
2020-03-23 14:09   ` Fabiano Rosas
2020-03-23 14:09     ` Fabiano Rosas
2020-03-23 14:09     ` Fabiano Rosas
2020-03-24  2:54 ` [PATCH 0/2] Fix SVM hang at startup Paul Mackerras
2020-03-24  2:54   ` Paul Mackerras

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=20200323234323.GA5604@blackberry \
    --to=paulus@ozlabs.org \
    --cc=benh@kernel.crashing.org \
    --cc=bharata@linux.ibm.com \
    --cc=groug@kaod.org \
    --cc=kvm-ppc@vger.kernel.org \
    --cc=ldufour@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.