linux-kernel - Re: [PATCH v2 5/7] KVM: arm64: MTE: Use stage-2 NoTagAccess memory attribute if supported

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: <CAMn1gO4huP4D_1mFdC8FsmvHkaQn+hC02ULcfBuS30VDM9=9gA@mail.gmail.com>
Date: Mon, 13 Jan 2025 12:47:54 -0800
From: Peter Collingbourne <pcc@...gle.com>
To: Catalin Marinas <catalin.marinas@....com>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@...nel.org>, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.linux.dev, 
	Suzuki K Poulose <Suzuki.Poulose@....com>, Steven Price <steven.price@....com>, 
	Will Deacon <will@...nel.org>, Marc Zyngier <maz@...nel.org>, Mark Rutland <mark.rutland@....com>, 
	Oliver Upton <oliver.upton@...ux.dev>, Joey Gouly <joey.gouly@....com>, 
	Zenghui Yu <yuzenghui@...wei.com>
Subject: Re: [PATCH v2 5/7] KVM: arm64: MTE: Use stage-2 NoTagAccess memory
 attribute if supported

On Mon, Jan 13, 2025 at 11:09 AM Catalin Marinas
<catalin.marinas@....com> wrote:
>
> On Sat, Jan 11, 2025 at 06:49:55PM +0530, Aneesh Kumar K.V wrote:
> > Catalin Marinas <catalin.marinas@....com> writes:
> > > On Fri, Jan 10, 2025 at 04:30:21PM +0530, Aneesh Kumar K.V (Arm) wrote:
> > >> Currently, the kernel won't start a guest if the MTE feature is enabled
> >
> > ...
> >
> > >> @@ -2152,7 +2162,8 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
> > >>            if (!vma)
> > >>                    break;
> > >>
> > >> -          if (kvm_has_mte(kvm) && !kvm_vma_mte_allowed(vma)) {
> > >> +          if (kvm_has_mte(kvm) &&
> > >> +              !kvm_has_mte_perm(kvm) && !kvm_vma_mte_allowed(vma)) {
> > >>                    ret = -EINVAL;
> > >>                    break;
> > >>            }
> > >
> > > I don't think we should change this, or at least not how it's done above
> > > (Suzuki raised a related issue internally relaxing this for VM_PFNMAP).
> > >
> > > For standard memory slots, we want to reject them upfront rather than
> > > deferring to the fault handler. An example here is file mmap() passed as
> > > standard RAM to the VM. It's an unnecessary change in behaviour IMHO.
> > > I'd only relax this for VM_PFNMAP mappings further down in this
> > > function (and move the VM_PFNMAP check above; see Suzuki's internal
> > > patch, unless he posted it publicly already).
> >
> > But we want to handle memslots backed by pagecache pages for virtio-shm
> > here (virtiofs dax use case).
>
> Ah, I forgot about this use case. So with virtiofs DAX, does a host page
> cache page (host VMM mmap()) get mapped directly into the guest as a
> separate memory slot? In this case, the host vma would not have
> VM_MTE_ALLOWED set.
>
> > With MTE_PERM, we can essentially skip the
> > kvm_vma_mte_allowed(vma) check because we handle all types in the fault
> > handler.
>
> This was pretty much the early behaviour when we added KVM support for
> MTE, allow !VM_MTE_ALLOWED and trap them later. However, we disallowed
> VM_SHARED because of some non-trivial race. Commit d89585fbb308 ("KVM:
> arm64: unify the tests for VMAs in memslots when MTE is enabled")
> changed this behaviour and the VM_MTE_ALLOWED check happens upfront. A
> subsequent commit removed the VM_SHARED check.
>
> It's a minor ABI change but I'm trying to figure out why we needed this
> upfront check rather than simply dropping the VM_SHARED check. Adding
> Peter in case he remembers. I can't see any race if we simply skipped
> this check altogether, irrespective of FEAT_MTE_PERM.

I don't see a problem with removing the upfront check. The reason I
kept the check was IIRC just that there was already a check there and
its logic needed to be adjusted for my VM_SHARED changes.

Peter