arm64: mte: Lock a page for MTE tag initialisation

Initialising the tags and setting PG_mte_tagged flag for a page can race
between multiple set_pte_at() on shared pages or setting the stage 2 pte
via user_mem_abort(). Introduce a new PG_mte_lock flag as PG_arch_3 and
set it before attempting page initialisation. Given that PG_mte_tagged
is never cleared for a page, consider setting this flag to mean page
unlocked and wait on this bit with acquire semantics if the page is
locked:

- try_page_mte_tagging() - lock the page for tagging, return true if it
  can be tagged, false if already tagged. No acquire semantics if it
  returns true (PG_mte_tagged not set) as there is no serialisation with
  a previous set_page_mte_tagged().

- set_page_mte_tagged() - set PG_mte_tagged with release semantics.

The two-bit locking is based on Peter Collingbourne's idea.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Peter Collingbourne <pcc@google.com>
Reviewed-by: Steven Price <steven.price@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Peter Collingbourne <pcc@google.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20221104011041.290951-6-pcc@google.com

diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
index 3f8199b..20dd06d 100644 (file)
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -25,7 +25,7 @@ unsigned long mte_copy_tags_to_user(void __user *to, void *from,
                                    unsigned long n);
 int mte_save_tags(struct page *page);
 void mte_save_page_tags(const void *page_addr, void *tag_storage);
-bool mte_restore_tags(swp_entry_t entry, struct page *page);
+void mte_restore_tags(swp_entry_t entry, struct page *page);
 void mte_restore_page_tags(void *page_addr, const void *tag_storage);
 void mte_invalidate_tags(int type, pgoff_t offset);
 void mte_invalidate_tags_area(int type);
@@ -36,6 +36,8 @@ void mte_free_tag_storage(char *storage);
 
 /* track which pages have valid allocation tags */
 #define PG_mte_tagged  PG_arch_2
+/* simple lock to avoid multiple threads tagging the same page */
+#define PG_mte_lock    PG_arch_3
 
 static inline void set_page_mte_tagged(struct page *page)
 {
@@ -60,6 +62,33 @@ static inline bool page_mte_tagged(struct page *page)
        return ret;
 }
 
+/*
+ * Lock the page for tagging and return 'true' if the page can be tagged,
+ * 'false' if already tagged. PG_mte_tagged is never cleared and therefore the
+ * locking only happens once for page initialisation.
+ *
+ * The page MTE lock state:
+ *
+ *   Locked:   PG_mte_lock && !PG_mte_tagged
+ *   Unlocked: !PG_mte_lock || PG_mte_tagged
+ *
+ * Acquire semantics only if the page is tagged (returning 'false').
+ */
+static inline bool try_page_mte_tagging(struct page *page)
+{
+       if (!test_and_set_bit(PG_mte_lock, &page->flags))
+               return true;
+
+       /*
+        * The tags are either being initialised or may have been initialised
+        * already. Check if the PG_mte_tagged flag has been set or wait
+        * otherwise.
+        */
+       smp_cond_load_acquire(&page->flags, VAL & (1UL << PG_mte_tagged));
+
+       return false;
+}
+
 void mte_zero_clear_page_tags(void *addr);
 void mte_sync_tags(pte_t old_pte, pte_t pte);
 void mte_copy_page_tags(void *kto, const void *kfrom);
@@ -86,6 +115,10 @@ static inline bool page_mte_tagged(struct page *page)
 {
        return false;
 }
+static inline bool try_page_mte_tagging(struct page *page)
+{
+       return false;
+}
 static inline void mte_zero_clear_page_tags(void *addr)
 {
 }

diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 98b6384..8735ac1 100644 (file)
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -1049,8 +1049,8 @@ static inline void arch_swap_invalidate_area(int type)
 #define __HAVE_ARCH_SWAP_RESTORE
 static inline void arch_swap_restore(swp_entry_t entry, struct folio *folio)
 {
-       if (system_supports_mte() && mte_restore_tags(entry, &folio->page))
-               set_page_mte_tagged(&folio->page);
+       if (system_supports_mte())
+               mte_restore_tags(entry, &folio->page);
 }
 
 #endif /* CONFIG_ARM64_MTE */

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index df11cfe..afb4ffd 100644 (file)
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2050,7 +2050,7 @@ static void cpu_enable_mte(struct arm64_cpu_capabilities const *cap)
         * Clear the tags in the zero page. This needs to be done via the
         * linear map which has the Tagged attribute.
         */
-       if (!page_mte_tagged(ZERO_PAGE(0))) {
+       if (try_page_mte_tagging(ZERO_PAGE(0))) {
                mte_clear_page_tags(lm_alias(empty_zero_page));
                set_page_mte_tagged(ZERO_PAGE(0));
        }

diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index 84a085d..f5bcb0d 100644 (file)
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -41,20 +41,14 @@ static void mte_sync_page_tags(struct page *page, pte_t old_pte,
        if (check_swap && is_swap_pte(old_pte)) {
                swp_entry_t entry = pte_to_swp_entry(old_pte);
 
-               if (!non_swap_entry(entry) && mte_restore_tags(entry, page)) {
-                       set_page_mte_tagged(page);
-                       return;
-               }
+               if (!non_swap_entry(entry))
+                       mte_restore_tags(entry, page);
        }
 
        if (!pte_is_tagged)
                return;
 
-       /*
-        * Test PG_mte_tagged again in case it was racing with another
-        * set_pte_at().
-        */
-       if (!page_mte_tagged(page)) {
+       if (try_page_mte_tagging(page)) {
                mte_clear_page_tags(page_address(page));
                set_page_mte_tagged(page);
        }

diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 817fdd1..5626ddb 100644 (file)
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -1068,15 +1068,19 @@ long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm,
                                        clear_user(tags, MTE_GRANULES_PER_PAGE);
                        kvm_release_pfn_clean(pfn);
                } else {
+                       /*
+                        * Only locking to serialise with a concurrent
+                        * set_pte_at() in the VMM but still overriding the
+                        * tags, hence ignoring the return value.
+                        */
+                       try_page_mte_tagging(page);
                        num_tags = mte_copy_tags_from_user(maddr, tags,
                                                        MTE_GRANULES_PER_PAGE);
 
-                       /*
-                        * Set the flag after checking the write
-                        * completed fully
-                        */
-                       if (num_tags == MTE_GRANULES_PER_PAGE)
-                               set_page_mte_tagged(page);
+                       /* uaccess failed, don't leave stale tags */
+                       if (num_tags != MTE_GRANULES_PER_PAGE)
+                               mte_clear_page_tags(page);
+                       set_page_mte_tagged(page);
 
                        kvm_release_pfn_dirty(pfn);
                }

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index e81bfb7..fa2c85b 100644 (file)
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1101,7 +1101,7 @@ static void sanitise_mte_tags(struct kvm *kvm, kvm_pfn_t pfn,
                return;
 
        for (i = 0; i < nr_pages; i++, page++) {
-               if (!page_mte_tagged(page)) {
+               if (try_page_mte_tagging(page)) {
                        mte_clear_page_tags(page_address(page));
                        set_page_mte_tagged(page);
                }

diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
index 731d8a3..8dd5a8f 100644 (file)
--- a/arch/arm64/mm/copypage.c
+++ b/arch/arm64/mm/copypage.c
@@ -23,6 +23,8 @@ void copy_highpage(struct page *to, struct page *from)
 
        if (system_supports_mte() && page_mte_tagged(from)) {
                page_kasan_tag_reset(to);
+               /* It's a new page, shouldn't have been tagged yet */
+               WARN_ON_ONCE(!try_page_mte_tagging(to));
                mte_copy_page_tags(kto, kfrom);
                set_page_mte_tagged(to);
        }

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 629e886..b8b299d 100644 (file)
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -933,6 +933,8 @@ struct page *alloc_zeroed_user_highpage_movable(struct vm_area_struct *vma,
 
 void tag_clear_highpage(struct page *page)
 {
+       /* Newly allocated page, shouldn't have been tagged yet */
+       WARN_ON_ONCE(!try_page_mte_tagging(page));
        mte_zero_clear_page_tags(page_address(page));
        set_page_mte_tagged(page);
 }

diff --git a/arch/arm64/mm/mteswap.c b/arch/arm64/mm/mteswap.c
index 70f9132..cd508ba 100644 (file)
--- a/arch/arm64/mm/mteswap.c
+++ b/arch/arm64/mm/mteswap.c
@@ -46,21 +46,17 @@ int mte_save_tags(struct page *page)
        return 0;
 }
 
-bool mte_restore_tags(swp_entry_t entry, struct page *page)
+void mte_restore_tags(swp_entry_t entry, struct page *page)
 {
        void *tags = xa_load(&mte_pages, entry.val);
 
        if (!tags)
-               return false;
+               return;
 
-       /*
-        * Test PG_mte_tagged again in case it was racing with another
-        * set_pte_at().
-        */
-       if (!test_and_set_bit(PG_mte_tagged, &page->flags))
+       if (try_page_mte_tagging(page)) {
                mte_restore_page_tags(page_address(page), tags);
-
-       return true;
+               set_page_mte_tagged(page);
+       }
 }
 
 void mte_invalidate_tags(int type, pgoff_t offset)