On Wed, May 19, 2021 at 10:32:01AM +0100, Steven Price wrote: > On 17/05/2021 17:14, Marc Zyngier wrote: > > On Mon, 17 May 2021 13:32:34 +0100, > > Steven Price <steven.price@xxxxxxx> wrote: > >> > >> A KVM guest could store tags in a page even if the VMM hasn't mapped > >> the page with PROT_MTE. So when restoring pages from swap we will > >> need to check to see if there are any saved tags even if !pte_tagged(). > >> > >> However don't check pages for which pte_access_permitted() returns false > >> as these will not have been swapped out. > >> > >> Signed-off-by: Steven Price <steven.price@xxxxxxx> > >> --- > >> arch/arm64/include/asm/pgtable.h | 9 +++++++-- > >> arch/arm64/kernel/mte.c | 16 ++++++++++++++-- > >> 2 files changed, 21 insertions(+), 4 deletions(-) > >> > >> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h > >> index 0b10204e72fc..275178a810c1 100644 > >> --- a/arch/arm64/include/asm/pgtable.h > >> +++ b/arch/arm64/include/asm/pgtable.h > >> @@ -314,8 +314,13 @@ static inline void set_pte_at(struct mm_struct *mm, unsigned long addr, > >> if (pte_present(pte) && pte_user_exec(pte) && !pte_special(pte)) > >> __sync_icache_dcache(pte); > >> > >> - if (system_supports_mte() && > >> - pte_present(pte) && pte_tagged(pte) && !pte_special(pte)) > >> + /* > >> + * If the PTE would provide user space access to the tags associated > >> + * with it then ensure that the MTE tags are synchronised. Exec-only > >> + * mappings don't expose tags (instruction fetches don't check tags). > > > > I'm not sure I understand this comment. Of course, execution doesn't > > match tags. But the memory could still have tags associated with > > it. Does this mean such a page would lose its tags is swapped out? > > Hmm, I probably should have reread that - the context of the comment is > lost. > > I added the comment when changing to pte_access_permitted(), and the > comment on pte_access_permitted() explains a potential gotcha: > > * p??_access_permitted() is true for valid user mappings (PTE_USER > * bit set, subject to the write permission check). For execute-only > * mappings, like PROT_EXEC with EPAN (both PTE_USER and PTE_UXN bits > * not set) must return false. PROT_NONE mappings do not have the > * PTE_VALID bit set. > > So execute-only mappings return false even though that is effectively a > type of user access. However, because MTE checks are not performed by > the PE for instruction fetches this doesn't matter. I'll update the > comment, how about: > > /* > * If the PTE would provide user space access to the tags associated > * with it then ensure that the MTE tags are synchronised. Although > * pte_access_permitted() returns false for exec only mappings, they > * don't expose tags (instruction fetches don't check tags). > */ This looks fine to me. We basically want to check the PTE_VALID and PTE_USER bits and pte_access_permitted() does this (we could come up with a new macro name like pte_valid_user() but since we don't care about execute-only, it gets unnecessarily complicated). -- Catalin _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
