public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: "Andersen, John S" <john.s.andersen@intel.com>
To: "liran.alon@oracle.com" <liran.alon@oracle.com>
Cc: "jmattson@google.com" <jmattson@google.com>,
	"joro@8bytes.org" <joro@8bytes.org>,
	"bp@alien8.de" <bp@alien8.de>, "x86@kernel.org" <x86@kernel.org>,
	"vkuznets@redhat.com" <vkuznets@redhat.com>,
	"hpa@zytor.com" <hpa@zytor.com>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	"Christopherson, Sean J" <sean.j.christopherson@intel.com>,
	"wanpengli@tencent.com" <wanpengli@tencent.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RESEND RFC 0/2] Paravirtualized Control Register pinning
Date: Tue, 24 Dec 2019 19:44:36 +0000	[thread overview]
Message-ID: <73950aff51bdf908f75ffa5e5cb629fc1d4ebbb6.camel@intel.com> (raw)
In-Reply-To: <1EBCD42E-9109-47A1-B959-6363A509D48D@oracle.com>

On Mon, 2019-12-23 at 16:48 +0200, Liran Alon wrote:
> > On 20 Dec 2019, at 21:26, John Andersen <john.s.andersen@intel.com>
> > wrote:
> > 
> > Pinning is not active when running in SMM. Entering SMM disables
> > pinned
> > bits, writes to control registers within SMM would therefore
> > trigger
> > general protection faults if pinning was enforced.
> 
> For compatibility reasons, it’s reasonable that pinning won’t be
> active when running in SMM.
> However, I do think we should not allow vSMM code to change pinned
> values when returning back from SMM.
> This would prevent a vulnerable vSMI handler from modifying vSMM
> state-area to modify CR4 when running outside of vSMM.
> I believe in this case it’s legit to just forcibly restore original
> CR0/CR4 pinned values. Ignoring vSMM changes.
> 

In em_rsm could we just OR with the value of the PINNED MSRs right
before the final return?

> > The guest may never read pinned bits. If an attacker were to read
> > the
> > CR pinned MSRs, they might decide to preform another attack which
> > would
> > not cause a general protection fault.
> 
> I disagree with this statement.
> An attacker knows what is the system it is attacking and can deduce
> by that which bits it pinned…
> Therefore, protecting from guest reading these is not important at
> all.
> 

Sure, I'll make it readable.

> > Should userspace expose the CR pining CPUID feature bit, it must
> > zero CR
> > pinned MSRs on reboot. If it does not, it runs the risk of having
> > the
> > guest enable pinning and subsequently cause general protection
> > faults on
> > next boot due to early boot code setting control registers to
> > values
> > which do not contain the pinned bits.
> 
> Why reset CR pinned MSRs by userspace instead of KVM INIT handling?
> 
> > When running with KVM guest support and paravirtualized CR pinning
> > enabled, paravirtualized and existing pinning are setup at the same
> > point on the boot CPU. Non-boot CPUs setup pinning upon
> > identification.
> > 
> > Guests using the kexec system call currently do not support
> > paravirtualized control register pinning. This is due to early boot
> > code writing known good values to control registers, these values
> > do
> > not contain the protected bits. This is due to CPU feature
> > identification being done at a later time, when the kernel properly
> > checks if it can enable protections.
> > 
> > Most distributions enable kexec. However, kexec could be made boot
> > time
> > disableable. In this case if a user has disabled kexec at boot time
> > the guest will request that paravirtualized control register
> > pinning
> > be enabled. This would expand the userbase to users of major
> > distributions.
> > 
> > Paravirtualized CR pinning will likely be incompatible with kexec
> > for
> > the foreseeable future. Early boot code could possibly be changed
> > to
> > not clear protected bits. However, a kernel that requests CR bits
> > be
> > pinned can't know if the kernel it's kexecing has been updated to
> > not
> > clear protected bits. This would result in the kernel being kexec'd
> > almost immediately receiving a general protection fault.
> 
> Instead of disabling kexec entirely, I think it makes more sense to
> invent
> some generic mechanism in which new kernel can describe to old kernel
> a set of flags that specifies which features hand-over it supports.
> One of them
> being pinned CRs.
> 
> For example, isn’t this also relevant for IOMMU DMA protection?
> i.e. Doesn’t old kernel need to know if it should disable or enable
> IOMMU DMAR
> before kexec to new kernel? Similar to EDK2 IOMMU DMA protection
> hand-over?

Great idea.

Making kexec work will require changes to these files and maybe more:

arch/x86/boot/compressed/head_64.S
arch/x86/kernel/head_64.S
arch/x86/kernel/relocate_kernel_64.S

Which my previous attempts showed different results when running
virtualized vs. unvirtualized. Specificity different behavior with SMAP
and UMIP bits.

This would be a longer process though. As validating that everything
still works in both the VM and on physical hosts will be required. As
it stands this patchset could pick up a fairly large userbase via the
virtualized container projects. Should we pursue kexec in this patchset
or a later one?

Thanks,
John

  parent reply	other threads:[~2019-12-24 19:44 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-20 19:26 [RESEND RFC 0/2] Paravirtualized Control Register pinning John Andersen
2019-12-20 19:27 ` [RESEND RFC 1/2] KVM: X86: Add CR pin MSRs John Andersen
2019-12-20 19:27 ` [RESEND RFC 2/2] X86: Use KVM " John Andersen
2019-12-23  7:39   ` Andy Lutomirski
2019-12-23 12:06     ` Borislav Petkov
2019-12-24 21:18     ` Andersen, John S
2019-12-21 13:59 ` [RESEND RFC 0/2] Paravirtualized Control Register pinning Paolo Bonzini
2019-12-23 17:28   ` Andersen, John S
2019-12-23 14:30 ` Liran Alon
2019-12-24 22:56   ` Liran Alon
2019-12-25  2:04   ` Andy Lutomirski
2019-12-25 13:05     ` Liran Alon
2019-12-23 14:48 ` Liran Alon
2019-12-23 17:09   ` Paolo Bonzini
2019-12-23 17:27     ` Andersen, John S
2019-12-23 17:28     ` Liran Alon
2019-12-23 17:46       ` Paolo Bonzini
2019-12-23 22:49         ` Liran Alon
2019-12-24 19:44   ` Andersen, John S [this message]
2019-12-24 20:35     ` Liran Alon
2019-12-24 21:17       ` Andersen, John S

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=73950aff51bdf908f75ffa5e5cb629fc1d4ebbb6.camel@intel.com \
    --to=john.s.andersen@intel.com \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liran.alon@oracle.com \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=sean.j.christopherson@intel.com \
    --cc=tglx@linutronix.de \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --cc=x86@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox