linux-kernel - Re: [PATCH] KVM: arm64: Drop mte_allowed check during memslot creation

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: <86ldtvr0nl.wl-maz@kernel.org>
Date: Mon, 24 Feb 2025 12:24:14 +0000
From: Marc Zyngier <maz@...nel.org>
To: "Aneesh Kumar K.V (Arm)" <aneesh.kumar@...nel.org>, Catalin Marinas <catalin.marinas@....com>
Cc:	linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	kvmarm@...ts.linux.dev,
	Oliver Upton <oliver.upton@...ux.dev>,
	Joey Gouly <joey.gouly@....com>,
	Zenghui Yu <yuzenghui@...wei.com>,
	Will Deacon <will@...nel.org>,
	Suzuki K Poulose <Suzuki.Poulose@....com>,
	Steven Price <steven.price@....com>,
	Peter Collingbourne <pcc@...gle.com>
Subject: Re: [PATCH] KVM: arm64: Drop mte_allowed check during memslot creation

On Mon, 24 Feb 2025 11:05:33 +0000,
Catalin Marinas <catalin.marinas@....com> wrote:
> 
> On Mon, Feb 24, 2025 at 03:09:38PM +0530, Aneesh Kumar K.V (Arm) wrote:
> > Before commit d89585fbb308 ("KVM: arm64: unify the tests for VMAs in
> > memslots when MTE is enabled"), kvm_arch_prepare_memory_region() only
> > rejected a memory slot if VM_SHARED was set. This commit unified the
> > checking with user_mem_abort(), with slots being rejected if either
> > VM_MTE_ALLOWED is not set or VM_SHARED set. A subsequent commit
> > c911f0d46879 ("KVM: arm64: permit all VM_MTE_ALLOWED mappings with MTE
> > enabled") dropped the VM_SHARED check, so we ended up with memory slots
> > being rejected if VM_MTE_ALLOWED is not set. This wasn't the case before
> > the commit d89585fbb308. The rejection of the memory slot with VM_SHARED
> > set was done to avoid a race condition with the test/set of the
> > PG_mte_tagged flag. Before Commit d77e59a8fccd ("arm64: mte: Lock a page
> > for MTE tag initialization") the kernel avoided allowing MTE with shared
> > pages, thereby preventing two tasks sharing a page from setting up the
> > PG_mte_tagged flag racily.
> > 
> > Commit d77e59a8fccd ("arm64: mte: Lock a page for MTE tag
> > initialization") further updated the locking so that the kernel
> > allows VM_SHARED mapping with MTE. With this commit, we can enable
> > memslot creation with VM_SHARED VMA mapping.
> > 
> > This patch results in a minor tweak to the ABI. We now allow creating
> > memslots that don't have the VM_MTE_ALLOWED flag set.
> 
> As I commented here:
> 
> https://lore.kernel.org/r/Z4e04P1bQlFBDHo7@arm.com
> 
> I'm fine with the change, we basically go back to the original ABI prior
> to relaxing this for VM_SHARED.
> 
> > If the guest uses
> > such a memslot with Allocation Tags, the kernel will generate -EFAULT.
> > ie, instead of failing early, we now fail later during KVM_RUN.
> 
> Nit: more like the kernel "will return -EFAULT" to the VMM rather than
> "generate".
> 
> > This change is needed because, without it, users are not able to use MTE
> > with VFIO passthrough (currently the mapping is either Device or
> > NonCacheable for which tag access check is not applied.), as shown
> > below (kvmtool VMM).
> 
> Another nit: "users are not able to user VFIO passthrough when MTE is
> enabled". At a first read, the above sounded to me like one wants to
> enable MTE for VFIO passthrough mappings.

What the commit message doesn't spell out is how MTE and VFIO are
interacting here. I also don't understand the reference to Device or
NC memory here.

Isn't the issue that DMA doesn't check/update tags, and therefore it
makes little sense to prevent non-tagged memory being associated with
a memslot?

My other concern is that this gives pretty poor consistency to the
guest, which cannot know what can be tagged and what cannot, and
breaks a guarantee that the guest should be able to rely on.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.