linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: Kristina Martsenko <kristina.martsenko@arm.com>
Cc: Mark Rutland <Mark.Rutland@arm.com>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	Andrew Jones <drjones@redhat.com>,
	Jacob Bramley <Jacob.Bramley@arm.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Marc Zyngier <Marc.Zyngier@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Adam Wallis <awallis@codeaurora.org>,
	Suzuki Poulose <Suzuki.Poulose@arm.com>,
	Will Deacon <Will.Deacon@arm.com>,
	Christoffer Dall <Christoffer.Dall@arm.com>,
	Dave P Martin <Dave.Martin@arm.com>,
	Amit Kachhap <Amit.Kachhap@arm.com>,
	Ramana Radhakrishnan <ramana.radhakrishnan@foss.arm.com>,
	"kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
	"linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lis>
Subject: Re: [PATCH v5 11/17] arm64: docs: document pointer authentication
Date: Fri, 19 Oct 2018 16:10:29 +0100	[thread overview]
Message-ID: <20181019151029.GD3985@arrakis.emea.arm.com> (raw)
In-Reply-To: <e73035eb-2157-7a9c-3631-d4d2ad0b7d38@arm.com>

On Fri, Oct 19, 2018 at 03:42:23PM +0100, Kristina Martsenko wrote:
> On 19/10/2018 12:35, Catalin Marinas wrote:
> > On Tue, Oct 16, 2018 at 05:14:39PM +0100, Kristina Martsenko wrote:
> >> On 05/10/2018 10:04, Ramana Radhakrishnan wrote:
> >>> On 05/10/2018 09:47, Kristina Martsenko wrote:
> >>>> +Virtualization
> >>>> +--------------
> >>>> +
> >>>> +Pointer authentication is not currently supported in KVM guests. KVM
> >>>> +will mask the feature bits from ID_AA64ISAR1_EL1, and attempted use of
> >>>> +the feature will result in an UNDEFINED exception being injected into
> >>>> +the guest.
> >>>
> >>> However applications using instructions from the hint space will
> >>> continue to work albeit without any protection (as they would just be
> >>> nops) ?
> >>
> >> Mostly, yes. If the guest leaves SCTLR_EL1.EnIA unset (and
> >> EnIB/EnDA/EnDB), then PAC* and AUT* instructions in the HINT space will
> >> execute as NOPs. If the guest sets EnIA, then PAC*/AUT* instructions
> >> will trap and KVM will inject an "Unknown reason" exception into the
> >> guest (which will cause a Linux guest to send a SIGILL to the application).
> > 
> > I think that part is fine. If KVM (a fairly recent version with CPUID
> > sanitisation) does not enable ptr auth, the CPUID should not advertise
> > this feature either so the guest kernel should not enable it. For the
> > above instructions in the HINT space, they will just be NOPs. If the
> > guest kernel enables the feature regardless of the CPUID information, it
> > deserves to get an "Unknown reason" exception.
> > 
> >> In the latter case we could instead pretend the instruction was a NOP
> >> and not inject an exception, but trapping twice per every function would
> >> probably be terrible for performance. The guest shouldn't be setting
> >> EnIA anyway if ID_AA64ISAR1_EL1 reports that pointer authentication is
> >> not present (because KVM has hidden it).
> > 
> > I don't think we should. The SCTLR_EL1 bits are RES0 unless you know
> > that the feature is present via CPUID.
> > 
> >> The other special case is the XPACLRI instruction, which is also in the
> >> HINT space. Currently it will trap and KVM will inject an exception into
> >> the guest. We should probably change this to NOP instead, as that's what
> >> applications will expect. Unfortunately there is no EnIA-like control to
> >> make it NOP.
> > 
> > Very good catch. Basically if EL2 doesn't know about ptr auth (older
> > distro), EL1 may or may not know but leaves SCTLR_EL1 disabled (based on
> > CPUID), the default HCR_EL2 is to trap (I'm ignoring EL3 as that's like
> > to have ptr auth enabled, being built for the specific HW). So a user
> > app considering XPACLRI a NOP (or inoffensive) will get a SIGILL
> > (injected by the guest kernel following the injection of "Unknown
> > reason" exception by KVM).
> > 
> > Ramana, is XPACLRI commonly generated by gcc and expects it to be a NOP?
> > Could we restrict it to only being used at run-time if the corresponding
> > HWCAP is set? This means redefining this instruction as no longer in the
> > NOP space.
> 
> I think an alternative solution is to just disable trapping of pointer
> auth instructions in KVM. This will mean that the instructions will
> behave the same in the guest as they do in the host. HINT-space
> instructions (including XPACLRI) will behave as NOPs (or perform their
> function, if enabled by the guest), and will not trap.

OK, so this means disabling the trap (during early EL2 setup) but still
sanitizing the CPUID not to report the feature to EL1 unless fully
supported on all CPUs.

> A side effect of disabling trapping is that keys may effectively leak
> from one guest to another, since one guest may set a key and another
> guest may use an instruction that uses that key. But this can be fixed
> by zeroing the keys every time we enter a guest. We can additionally
> trap key accesses (which is separate from instruction trapping), to have
> guests fail more reliably and avoid restoring host keys on guest exit.

Actually, if the CPUID doesn't report the feature present, do we care
that a stupid guest writes some register and expects it to be preserved
and not leaked?

> Things still won't work well on big.LITTLE systems with mismatched
> pointer auth support between CPUs, but as Marc pointed out in the other
> email, we can just disable KVM on such systems when we detect a pointer
> auth mismatch.

If we ever see such system, we could follow the approach above - disable
trapping at EL2 if the feature is present but do not report it to EL1
via CPUID. A guest should not expect it to work properly.

> If we want current stable kernels to support guests that use HINT-space
> pointer auth instructions, we'll need to backport the above changes to
> stable kernels as well.

I agree.

> Even if we restricted userspace to only use XPACLRI if the HWCAP is set,
> current stable kernels would still not be able to handle the HINT-space
> PAC/AUT instructions that GCC generates, if the guest is pointer auth
> aware. None of the stable kernels have the CPUID sanitisation patches,
> so the guest would enable pointer auth, which would cause the PAC/AUT
> instructions to trap.

Ah, CPUID sanitisation would have to be backported as well. We can
probably get away with something simpler which does not sanitise
big.LITTLE (and I would not expect "enterprise" big.LITTLE machines) but
simply cap/mask the CPUID to what is known to the respective kernel
version. That shouldn't be too intrusive.

-- 
Catalin

WARNING: multiple messages have this Message-ID (diff)
From: Catalin Marinas <catalin.marinas@arm.com>
To: Kristina Martsenko <kristina.martsenko@arm.com>
Cc: Mark Rutland <Mark.Rutland@arm.com>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	Andrew Jones <drjones@redhat.com>,
	Jacob Bramley <Jacob.Bramley@arm.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Marc Zyngier <Marc.Zyngier@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Adam Wallis <awallis@codeaurora.org>,
	Suzuki Poulose <Suzuki.Poulose@arm.com>,
	Will Deacon <Will.Deacon@arm.com>,
	Christoffer Dall <Christoffer.Dall@arm.com>,
	Dave P Martin <Dave.Martin@arm.com>,
	Amit Kachhap <Amit.Kachhap@arm.com>,
	Ramana Radhakrishnan <ramana.radhakrishnan@foss.arm.com>,
	"kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Kees Cook <keescook@chromium.org>
Subject: Re: [PATCH v5 11/17] arm64: docs: document pointer authentication
Date: Fri, 19 Oct 2018 16:10:29 +0100	[thread overview]
Message-ID: <20181019151029.GD3985@arrakis.emea.arm.com> (raw)
Message-ID: <20181019151029.GwlAqC0mezplhUFTb9fwoN9ZQYDoO0EwY8EYwNLcWmQ@z> (raw)
In-Reply-To: <e73035eb-2157-7a9c-3631-d4d2ad0b7d38@arm.com>

On Fri, Oct 19, 2018 at 03:42:23PM +0100, Kristina Martsenko wrote:
> On 19/10/2018 12:35, Catalin Marinas wrote:
> > On Tue, Oct 16, 2018 at 05:14:39PM +0100, Kristina Martsenko wrote:
> >> On 05/10/2018 10:04, Ramana Radhakrishnan wrote:
> >>> On 05/10/2018 09:47, Kristina Martsenko wrote:
> >>>> +Virtualization
> >>>> +--------------
> >>>> +
> >>>> +Pointer authentication is not currently supported in KVM guests. KVM
> >>>> +will mask the feature bits from ID_AA64ISAR1_EL1, and attempted use of
> >>>> +the feature will result in an UNDEFINED exception being injected into
> >>>> +the guest.
> >>>
> >>> However applications using instructions from the hint space will
> >>> continue to work albeit without any protection (as they would just be
> >>> nops) ?
> >>
> >> Mostly, yes. If the guest leaves SCTLR_EL1.EnIA unset (and
> >> EnIB/EnDA/EnDB), then PAC* and AUT* instructions in the HINT space will
> >> execute as NOPs. If the guest sets EnIA, then PAC*/AUT* instructions
> >> will trap and KVM will inject an "Unknown reason" exception into the
> >> guest (which will cause a Linux guest to send a SIGILL to the application).
> > 
> > I think that part is fine. If KVM (a fairly recent version with CPUID
> > sanitisation) does not enable ptr auth, the CPUID should not advertise
> > this feature either so the guest kernel should not enable it. For the
> > above instructions in the HINT space, they will just be NOPs. If the
> > guest kernel enables the feature regardless of the CPUID information, it
> > deserves to get an "Unknown reason" exception.
> > 
> >> In the latter case we could instead pretend the instruction was a NOP
> >> and not inject an exception, but trapping twice per every function would
> >> probably be terrible for performance. The guest shouldn't be setting
> >> EnIA anyway if ID_AA64ISAR1_EL1 reports that pointer authentication is
> >> not present (because KVM has hidden it).
> > 
> > I don't think we should. The SCTLR_EL1 bits are RES0 unless you know
> > that the feature is present via CPUID.
> > 
> >> The other special case is the XPACLRI instruction, which is also in the
> >> HINT space. Currently it will trap and KVM will inject an exception into
> >> the guest. We should probably change this to NOP instead, as that's what
> >> applications will expect. Unfortunately there is no EnIA-like control to
> >> make it NOP.
> > 
> > Very good catch. Basically if EL2 doesn't know about ptr auth (older
> > distro), EL1 may or may not know but leaves SCTLR_EL1 disabled (based on
> > CPUID), the default HCR_EL2 is to trap (I'm ignoring EL3 as that's like
> > to have ptr auth enabled, being built for the specific HW). So a user
> > app considering XPACLRI a NOP (or inoffensive) will get a SIGILL
> > (injected by the guest kernel following the injection of "Unknown
> > reason" exception by KVM).
> > 
> > Ramana, is XPACLRI commonly generated by gcc and expects it to be a NOP?
> > Could we restrict it to only being used at run-time if the corresponding
> > HWCAP is set? This means redefining this instruction as no longer in the
> > NOP space.
> 
> I think an alternative solution is to just disable trapping of pointer
> auth instructions in KVM. This will mean that the instructions will
> behave the same in the guest as they do in the host. HINT-space
> instructions (including XPACLRI) will behave as NOPs (or perform their
> function, if enabled by the guest), and will not trap.

OK, so this means disabling the trap (during early EL2 setup) but still
sanitizing the CPUID not to report the feature to EL1 unless fully
supported on all CPUs.

> A side effect of disabling trapping is that keys may effectively leak
> from one guest to another, since one guest may set a key and another
> guest may use an instruction that uses that key. But this can be fixed
> by zeroing the keys every time we enter a guest. We can additionally
> trap key accesses (which is separate from instruction trapping), to have
> guests fail more reliably and avoid restoring host keys on guest exit.

Actually, if the CPUID doesn't report the feature present, do we care
that a stupid guest writes some register and expects it to be preserved
and not leaked?

> Things still won't work well on big.LITTLE systems with mismatched
> pointer auth support between CPUs, but as Marc pointed out in the other
> email, we can just disable KVM on such systems when we detect a pointer
> auth mismatch.

If we ever see such system, we could follow the approach above - disable
trapping at EL2 if the feature is present but do not report it to EL1
via CPUID. A guest should not expect it to work properly.

> If we want current stable kernels to support guests that use HINT-space
> pointer auth instructions, we'll need to backport the above changes to
> stable kernels as well.

I agree.

> Even if we restricted userspace to only use XPACLRI if the HWCAP is set,
> current stable kernels would still not be able to handle the HINT-space
> PAC/AUT instructions that GCC generates, if the guest is pointer auth
> aware. None of the stable kernels have the CPUID sanitisation patches,
> so the guest would enable pointer auth, which would cause the PAC/AUT
> instructions to trap.

Ah, CPUID sanitisation would have to be backported as well. We can
probably get away with something simpler which does not sanitise
big.LITTLE (and I would not expect "enterprise" big.LITTLE machines) but
simply cap/mask the CPUID to what is known to the respective kernel
version. That shouldn't be too intrusive.

-- 
Catalin

  parent reply	other threads:[~2018-10-19 15:10 UTC|newest]

Thread overview: 124+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-05  8:47 [PATCH 00/17] ARMv8.3 pointer authentication support Kristina Martsenko
2018-10-05  8:47 ` Kristina Martsenko
2018-10-05  8:47 ` [PATCH v5 01/17] arm64: add pointer authentication register bits Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-11 16:28   ` Will Deacon
2018-10-11 16:28     ` Will Deacon
2018-10-12  8:53     ` Mark Rutland
2018-10-12  8:53       ` Mark Rutland
2018-10-12  8:56       ` Will Deacon
2018-10-12  8:56         ` Will Deacon
2018-10-12  9:50         ` Mark Rutland
2018-10-12  9:50           ` Mark Rutland
2018-10-05  8:47 ` [PATCH v5 02/17] arm64/kvm: consistently handle host HCR_EL2 flags Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-05  8:47 ` [PATCH v5 03/17] arm64/kvm: hide ptrauth from guests Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-05  8:47 ` [PATCH v5 04/17] arm64: Don't trap host pointer auth use to EL2 Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-05  8:47 ` [PATCH v5 05/17] arm64/cpufeature: detect pointer authentication Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-05  8:47 ` [PATCH v5 06/17] asm-generic: mm_hooks: allow hooks to be overridden individually Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-05  8:47 ` [PATCH v5 07/17] arm64: add basic pointer authentication support Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-11 16:00   ` Suzuki K Poulose
2018-10-11 16:00     ` Suzuki K Poulose
2018-10-19 11:15   ` Catalin Marinas
2018-10-19 11:15     ` Catalin Marinas
2018-10-19 11:24     ` Will Deacon
2018-10-19 11:24       ` Will Deacon
2018-10-19 15:36       ` Kees Cook
2018-10-19 15:36         ` Kees Cook
2018-10-19 15:49         ` Will Deacon
2018-10-19 15:49           ` Will Deacon
2018-10-19 16:05           ` Kees Cook
2018-10-19 16:05             ` Kees Cook
2018-10-19 16:16             ` Will Deacon
2018-10-19 16:16               ` Will Deacon
2018-10-19 15:54         ` Mark Rutland
2018-10-19 15:54           ` Mark Rutland
2018-10-19 16:49       ` Cyrill Gorcunov
2018-10-19 16:49         ` Cyrill Gorcunov
2018-11-14 18:11       ` Will Deacon
2018-11-14 18:11         ` Will Deacon
2018-11-15 10:25         ` Dave Martin
2018-11-15 10:25           ` Dave Martin
2018-10-23  8:36     ` Ramana Radhakrishnan
2018-10-23  8:36       ` Ramana Radhakrishnan
2018-10-23 10:20       ` Will Deacon
2018-10-23 10:20         ` Will Deacon
2018-10-05  8:47 ` [PATCH v5 08/17] arm64: expose user PAC bit positions via ptrace Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-05  8:47 ` [PATCH v5 09/17] arm64: perf: strip PAC when unwinding userspace Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-05  8:47 ` [PATCH v5 10/17] arm64: enable pointer authentication Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-05  8:47 ` [PATCH v5 11/17] arm64: docs: document " Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-05  9:04   ` Ramana Radhakrishnan
2018-10-05  9:04     ` Ramana Radhakrishnan
2018-10-16 16:14     ` Kristina Martsenko
2018-10-16 16:14       ` Kristina Martsenko
2018-10-19 11:35       ` Catalin Marinas
2018-10-19 11:35         ` Catalin Marinas
2018-10-19 11:47         ` Marc Zyngier
2018-10-19 11:47           ` Marc Zyngier
2018-10-19 12:22         ` Will Deacon
2018-10-19 12:22           ` Will Deacon
2018-10-19 14:42         ` Kristina Martsenko
2018-10-19 14:42           ` Kristina Martsenko
2018-10-19 15:10           ` Catalin Marinas [this message]
2018-10-19 15:10             ` Catalin Marinas
2018-10-19 17:45             ` Will Deacon
2018-10-19 17:45               ` Will Deacon
2018-11-02  6:02               ` Jon Masters
2018-11-02  6:02                 ` Jon Masters
2018-10-24 10:56         ` Ramana Radhakrishnan
2018-10-24 10:56           ` Ramana Radhakrishnan
2018-10-15 22:35   ` Kees Cook
2018-10-15 22:35     ` Kees Cook
2018-11-02  9:46     ` Ramana Radhakrishnan
2018-11-02  9:46       ` Ramana Radhakrishnan
2018-10-05  8:47 ` [RFC 12/17] arm64: move ptrauth keys to thread_info Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-19 11:38   ` Catalin Marinas
2018-10-19 11:38     ` Catalin Marinas
2018-10-05  8:47 ` [RFC 13/17] arm64: install user ptrauth keys at kernel exit time Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-05  8:47 ` [RFC 14/17] arm64: unwind: strip PAC from kernel addresses Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-05  8:47 ` [RFC 15/17] arm64: enable ptrauth earlier Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-06 12:51   ` Amit Kachhap
2018-10-06 12:51     ` Amit Kachhap
2018-10-05  8:47 ` [RFC 16/17] arm64: initialize and switch ptrauth kernel keys Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-06 12:56   ` Amit Kachhap
2018-10-06 12:56     ` Amit Kachhap
2018-10-05  8:47 ` [RFC 17/17] arm64: compile the kernel with ptrauth -msign-return-address Kristina Martsenko
2018-10-05  8:47   ` Kristina Martsenko
2018-10-05  9:01   ` Ramana Radhakrishnan
2018-10-05  9:01     ` Ramana Radhakrishnan
2018-10-11 14:00     ` Kristina Martsenko
2018-10-11 14:00       ` Kristina Martsenko
2018-10-11 14:23   ` Vladimir Murzin
2018-10-11 14:23     ` Vladimir Murzin
2018-10-15 22:38     ` Kees Cook
2018-10-15 22:38       ` Kees Cook
2018-10-15 22:42 ` [PATCH 00/17] ARMv8.3 pointer authentication support Kees Cook
2018-10-15 22:42   ` Kees Cook
2018-11-13 16:17   ` Kristina Martsenko
2018-11-13 16:17     ` Kristina Martsenko
2018-11-13 23:09     ` Kees Cook
2018-11-13 23:09       ` Kees Cook
2018-11-14 15:54       ` Kristina Martsenko
2018-11-14 15:54         ` Kristina Martsenko
2018-11-14 21:47       ` Mark Rutland
2018-11-14 21:47         ` Mark Rutland
2018-11-14 22:48         ` Kees Cook
2018-11-14 22:48           ` Kees Cook
2018-10-19 12:36 ` Will Deacon
2018-10-19 12:36   ` Will Deacon
2018-10-23  8:39   ` Ramana Radhakrishnan
2018-10-23  8:39     ` Ramana Radhakrishnan

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=20181019151029.GD3985@arrakis.emea.arm.com \
    --to=catalin.marinas@arm.com \
    --cc=Amit.Kachhap@arm.com \
    --cc=Christoffer.Dall@arm.com \
    --cc=Dave.Martin@arm.com \
    --cc=Jacob.Bramley@arm.com \
    --cc=Marc.Zyngier@arm.com \
    --cc=Mark.Rutland@arm.com \
    --cc=Suzuki.Poulose@arm.com \
    --cc=Will.Deacon@arm.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=arnd@arndb.de \
    --cc=awallis@codeaurora.org \
    --cc=drjones@redhat.com \
    --cc=kristina.martsenko@arm.com \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lis \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ramana.radhakrishnan@foss.arm.com \
    /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;
as well as URLs for NNTP newsgroup(s).