| 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
| ||
|
Date: Fri, 2 Oct 2020 16:30:50 +0200
From: Andrew Jones <drjones@...hat.com>
To: Steven Price <steven.price@....com>
Cc: Catalin Marinas <catalin.marinas@....com>,
Marc Zyngier <maz@...nel.org>, Will Deacon <will@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Peter Maydell <peter.maydell@...aro.org>,
"Dr. David Alan Gilbert" <dgilbert@...hat.com>,
Haibo Xu <Haibo.Xu@....com>,
Suzuki K Poulose <suzuki.poulose@....com>,
qemu-devel@...gnu.org, Dave Martin <Dave.Martin@....com>,
Juan Quintela <quintela@...hat.com>,
Richard Henderson <richard.henderson@...aro.org>,
linux-kernel@...r.kernel.org, James Morse <james.morse@....com>,
Julien Thierry <julien.thierry.kdev@...il.com>,
Thomas Gleixner <tglx@...utronix.de>,
kvmarm@...ts.cs.columbia.edu, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v3 2/2] arm64: kvm: Introduce MTE VCPU feature
On Fri, Sep 25, 2020 at 10:36:07AM +0100, Steven Price wrote:
> Add a new VM feature 'KVM_ARM_CAP_MTE' which enables memory tagging
> for a VM. This exposes the feature to the guest and automatically tags
> memory pages touched by the VM as PG_mte_tagged (and clears the tags
> storage) to ensure that the guest cannot see stale tags, and so that the
> tags are correctly saved/restored across swap.
>
> Signed-off-by: Steven Price <steven.price@....com>
> ---
> arch/arm64/include/asm/kvm_emulate.h | 3 +++
> arch/arm64/include/asm/kvm_host.h | 3 +++
> arch/arm64/kvm/arm.c | 9 +++++++++
> arch/arm64/kvm/mmu.c | 15 +++++++++++++++
> arch/arm64/kvm/sys_regs.c | 6 +++++-
> include/uapi/linux/kvm.h | 1 +
> 6 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 49a55be2b9a2..4923a566ae6e 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -79,6 +79,9 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
> if (cpus_have_const_cap(ARM64_MISMATCHED_CACHE_TYPE) ||
> vcpu_el1_is_32bit(vcpu))
> vcpu->arch.hcr_el2 |= HCR_TID2;
> +
> + if (vcpu->kvm->arch.mte_enabled)
> + vcpu->arch.hcr_el2 |= HCR_ATA;
> }
>
> static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 4f4360dd149e..1379300c1487 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -110,6 +110,9 @@ struct kvm_arch {
> * supported.
> */
> bool return_nisv_io_abort_to_user;
> +
> + /* Memory Tagging Extension enabled for the guest */
> + bool mte_enabled;
> };
>
> struct kvm_vcpu_fault_info {
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 46dc3d75cf13..624edca0a1fa 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -87,6 +87,12 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
> r = 0;
> kvm->arch.return_nisv_io_abort_to_user = true;
> break;
> + case KVM_CAP_ARM_MTE:
> + if (!system_supports_mte() || kvm->created_vcpus)
> + return -EINVAL;
> + r = 0;
> + kvm->arch.mte_enabled = true;
> + break;
> default:
> r = -EINVAL;
> break;
> @@ -206,6 +212,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> */
> r = 1;
> break;
> + case KVM_CAP_ARM_MTE:
> + r = system_supports_mte();
> + break;
> default:
> r = kvm_arch_vm_ioctl_check_extension(kvm, ext);
> break;
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index ba00bcc0c884..befb9e1f0aa6 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1949,6 +1949,21 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> if (vma_pagesize == PAGE_SIZE && !force_pte)
> vma_pagesize = transparent_hugepage_adjust(memslot, hva,
> &pfn, &fault_ipa);
> + if (system_supports_mte() && kvm->arch.mte_enabled && pfn_valid(pfn)) {
'system_supports_mte() && kvm->arch.mte_enabled' is redundant, but I
assume system_supports_mte() is there to improve the efficiency of the
branch, as it's using cpus_have_const_cap(). Maybe a helper like
static inline bool kvm_arm_mte_enabled(struct kvm *kvm)
{
return system_supports_mte() && kvm->arch.mte_enabled;
}
would allow both the more efficient branch and look less confusing
where it gets used.
> + /*
> + * VM will be able to see the page's tags, so we must ensure
> + * they have been initialised.
> + */
> + struct page *page = pfn_to_page(pfn);
> + long i, nr_pages = compound_nr(page);
> +
> + /* if PG_mte_tagged is set, tags have already been initialised */
> + for (i = 0; i < nr_pages; i++, page++) {
> + if (!test_and_set_bit(PG_mte_tagged, &page->flags))
> + mte_clear_page_tags(page_address(page));
> + }
> + }
> +
> if (writable)
> kvm_set_pfn_dirty(pfn);
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index a655f172b5ad..5010a47152b4 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1132,7 +1132,8 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
> val &= ~(0xfUL << ID_AA64PFR0_SVE_SHIFT);
> val &= ~(0xfUL << ID_AA64PFR0_AMU_SHIFT);
> } else if (id == SYS_ID_AA64PFR1_EL1) {
> - val &= ~(0xfUL << ID_AA64PFR1_MTE_SHIFT);
> + if (!vcpu->kvm->arch.mte_enabled)
> + val &= ~(0xfUL << ID_AA64PFR1_MTE_SHIFT);
> } else if (id == SYS_ID_AA64ISAR1_EL1 && !vcpu_has_ptrauth(vcpu)) {
> val &= ~((0xfUL << ID_AA64ISAR1_APA_SHIFT) |
> (0xfUL << ID_AA64ISAR1_API_SHIFT) |
> @@ -1394,6 +1395,9 @@ static bool access_mte_regs(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> static unsigned int mte_visibility(const struct kvm_vcpu *vcpu,
> const struct sys_reg_desc *rd)
> {
> + if (vcpu->kvm->arch.mte_enabled)
> + return 0;
> +
> return REG_HIDDEN_USER | REG_HIDDEN_GUEST;
> }
>
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index f6d86033c4fa..87678ed82ab4 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1035,6 +1035,7 @@ struct kvm_ppc_resize_hpt {
> #define KVM_CAP_LAST_CPU 184
> #define KVM_CAP_SMALLER_MAXPHYADDR 185
> #define KVM_CAP_S390_DIAG318 186
> +#define KVM_CAP_ARM_MTE 188
>
> #ifdef KVM_CAP_IRQ_ROUTING
>
> --
> 2.20.1
>
>
Besides the helper suggestion nit
Reviewed-by: Andrew Jones <drjones@...hat.com>
Powered by blists - more mailing lists