linux-kernel - Re: [PATCH v4 1/2] arm64: kvm: Save/restore MTE registers

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: <f5e9fe3a-2147-4326-5e78-5eaf88c72789@arm.com>
Date:   Thu, 19 Nov 2020 12:45:54 +0000
From:   Steven Price <steven.price@....com>
To:     Catalin Marinas <catalin.marinas@....com>
Cc:     Marc Zyngier <maz@...nel.org>, Will Deacon <will@...nel.org>,
        James Morse <james.morse@....com>,
        Julien Thierry <julien.thierry.kdev@...il.com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        kvmarm@...ts.cs.columbia.edu, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, Dave Martin <Dave.Martin@....com>,
        Mark Rutland <mark.rutland@....com>,
        Thomas Gleixner <tglx@...utronix.de>, qemu-devel@...gnu.org,
        Juan Quintela <quintela@...hat.com>,
        "Dr. David Alan Gilbert" <dgilbert@...hat.com>,
        Richard Henderson <richard.henderson@...aro.org>,
        Peter Maydell <peter.maydell@...aro.org>,
        Haibo Xu <Haibo.Xu@....com>, Andrew Jones <drjones@...hat.com>
Subject: Re: [PATCH v4 1/2] arm64: kvm: Save/restore MTE registers

On 18/11/2020 17:02, Catalin Marinas wrote:
> On Wed, Nov 18, 2020 at 04:01:18PM +0000, Steven Price wrote:
>> On 17/11/2020 19:20, Marc Zyngier wrote:
>>> On 2020-10-26 15:57, Steven Price wrote:
>>>> diff --git a/arch/arm64/include/asm/sysreg.h
>>>> b/arch/arm64/include/asm/sysreg.h
>>>> index d52c1b3ce589..7727df0bc09d 100644
>>>> --- a/arch/arm64/include/asm/sysreg.h
>>>> +++ b/arch/arm64/include/asm/sysreg.h
>>>> @@ -565,7 +565,8 @@
>>>> ï¿½#define SCTLR_ELx_Mï¿½ï¿½ï¿½ (BIT(0))
>>>>
>>>> ï¿½#define SCTLR_ELx_FLAGSï¿½ï¿½ï¿½ (SCTLR_ELx_Mï¿½ | SCTLR_ELx_A | SCTLR_ELx_C | \
>>>> -ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB)
>>>> +ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB | \
>>>> +ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ SCTLR_ELx_ITFSB)
>>>>
>>>> ï¿½/* SCTLR_EL2 specific flags. */
>>>> ï¿½#define SCTLR_EL2_RES1ï¿½ï¿½ï¿½ ((BIT(4))ï¿½ | (BIT(5))ï¿½ | (BIT(11)) |
>>>> (BIT(16)) | \
>>>> diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
>>>> b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
>>>> index 7a986030145f..a124ffa49ba3 100644
>>>> --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
>>>> +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
>>>> @@ -18,6 +18,11 @@
>>>> ï¿½static inline void __sysreg_save_common_state(struct
>>>> kvm_cpu_context *ctxt)
>>>> ï¿½{
>>>> ï¿½ï¿½ï¿½ï¿½ ctxt_sys_reg(ctxt, MDSCR_EL1)ï¿½ï¿½ï¿½ = read_sysreg(mdscr_el1);
>>>> +ï¿½ï¿½ï¿½ if (system_supports_mte()) {
>>>> +ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ ctxt_sys_reg(ctxt, RGSR_EL1)ï¿½ï¿½ï¿½ = read_sysreg_s(SYS_RGSR_EL1);
>>>> +ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ ctxt_sys_reg(ctxt, GCR_EL1)ï¿½ï¿½ï¿½ = read_sysreg_s(SYS_GCR_EL1);
>>>> +ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ ctxt_sys_reg(ctxt, TFSRE0_EL1)ï¿½ï¿½ï¿½ =
>>>> read_sysreg_s(SYS_TFSRE0_EL1);
>>>
>>> As far as I can tell, HCR_EL2.ATA is still clear when running a guest.
>>> So why, do we save/restore this state yet?
>>>
>>> Also, I wonder whether we should keep these in the C code. If one day
>>> we enable MTE in the kernel, we will have to move them to the assembly
>>> part, much like we do for PAuth. And I fear that "one day" is pretty
>>> soon:
>>>
>>> https://lore.kernel.org/linux-arm-kernel/cover.1605046192.git.andreyknvl@google.com/
>>
>> Good point. Although for MTE we do have the option of setting TCO in PSTATE
>> so this could remain in C if we're not bothered about the 'gap' in KASAN
>> coverage. I haven't yet got my head around how (or indeed if) that series
>> handles guests.
> 
> I think we should be fine with the currently proposed in-kernel MTE
> support. However, setting GCR_EL1 can get in the way if stack tagging is
> ever enabled (it breaks single image). The compiler uses GCR_EL1 to
> generate different colours for variables on the stack and changing it in
> the middle of a function may cause confusion. You'd have to set
> PSTATE.TCO for the whole function, either from the caller or, if the
> compiler gets smarter, some function attribute.
> 

If the compiler might start playing with TCO then this could also be an 
issue for VMMs which will (at least with the current design) need to use 
TCO to safely access guest memory. Especially if we enforce PROT_MTE 
mappings for the VMM.

Steve