BACKPORT: arm64: kasan: Revert "arm64: mte: reset the page tag in page->flags"

(Backport: no real conflicts, inverse commit order commit 918002bdbe
 "arm64: mte: Avoid setting PG_mte_tagged if no tags cleared or restored".

 This revert patch has previously made into Android through stable in
 commit add4bc9281, but was then reverted in stable commit a8a007c5b1.
 The reason for the revert: this revert patch was initially considered a
 standalone fix, but it actually required more patches to work properly:

 70c248aca9 ("mm: kasan: Skip unpoisoning of user pages")
 6d05141a39 ("mm: kasan: Skip page unpoisoning only if __GFP_SKIP_KASAN_UNPOISON")

 Now that these patches are backported (in 1f2cb45568 and bc6ed581f4),
 apply the revert patch again.)

This reverts commit e5b8d92189.

Pages mapped in user-space with PROT_MTE have the allocation tags either
zeroed or copied/restored to some user values. In order for the kernel
to access such pages via page_address(), resetting the tag in
page->flags was necessary. This tag resetting was deferred to
set_pte_at() -> mte_sync_page_tags() but it can race with another CPU
reading the flags (via page_to_virt()):

P0 (mte_sync_page_tags):	P1 (memcpy from virt_to_page):
				  Rflags!=0xff
  Wflags=0xff
  DMB (doesn't help)
  Wtags=0
				  Rtags=0   // fault

Since now the post_alloc_hook() function resets the page->flags tag when
unpoisoning is skipped for user pages (including the __GFP_ZEROTAGS
case), revert the arm64 commit calling page_kasan_tag_reset().

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: Andrey Konovalov <andreyknvl@gmail.com>
Cc: Peter Collingbourne <pcc@google.com>
Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Acked-by: Andrey Konovalov <andreyknvl@gmail.com>
Link: https://lore.kernel.org/r/20220610152141.2148929-5-catalin.marinas@arm.com
Signed-off-by: Will Deacon <will@kernel.org>
Bug: 254721825
(cherry picked from commit 20794545c1)
Change-Id: I2a1cae227249e386056894af40f61b86425569f2
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
This commit is contained in:
Catalin Marinas
2022-06-10 16:21:41 +01:00
committed by Andrey Konovalov
parent 9f3841f265
commit 2f0d251c42
4 changed files with 0 additions and 32 deletions

View File

@@ -326,11 +326,6 @@ static void swsusp_mte_restore_tags(void)
unsigned long pfn = xa_state.xa_index;
struct page *page = pfn_to_online_page(pfn);
/*
* It is not required to invoke page_kasan_tag_reset(page)
* at this point since the tags stored in page->flags are
* already restored.
*/
mte_restore_page_tags(page_address(page), tags);
mte_free_tag_storage(tags);

View File

@@ -48,15 +48,6 @@ static void mte_sync_page_tags(struct page *page, pte_t old_pte,
if (!pte_is_tagged)
return;
page_kasan_tag_reset(page);
/*
* We need smp_wmb() in between setting the flags and clearing the
* tags because if another thread reads page->flags and builds a
* tagged address out of it, there is an actual dependency to the
* memory access, but on the current thread we do not guarantee that
* the new page->flags are visible before the tags were updated.
*/
smp_wmb();
/*
* Test PG_mte_tagged again in case it was racing with another
* set_pte_at().

View File

@@ -23,15 +23,6 @@ void copy_highpage(struct page *to, struct page *from)
if (system_supports_mte() && test_bit(PG_mte_tagged, &from->flags)) {
set_bit(PG_mte_tagged, &to->flags);
page_kasan_tag_reset(to);
/*
* We need smp_wmb() in between setting the flags and clearing the
* tags because if another thread reads page->flags and builds a
* tagged address out of it, there is an actual dependency to the
* memory access, but on the current thread we do not guarantee that
* the new page->flags are visible before the tags were updated.
*/
smp_wmb();
mte_copy_page_tags(kto, kfrom);
}
}

View File

@@ -53,15 +53,6 @@ bool mte_restore_tags(swp_entry_t entry, struct page *page)
if (!tags)
return false;
page_kasan_tag_reset(page);
/*
* We need smp_wmb() in between setting the flags and clearing the
* tags because if another thread reads page->flags and builds a
* tagged address out of it, there is an actual dependency to the
* memory access, but on the current thread we do not guarantee that
* the new page->flags are visible before the tags were updated.
*/
smp_wmb();
/*
* Test PG_mte_tagged again in case it was racing with another
* set_pte_at().