* [PATCH v4 1/4] mm: arch: remove indirection level in alloc_zeroed_user_highpage_movable()
2021-05-28 1:04 [PATCH v4 0/4] arm64: improve efficiency of setting tags for user pages Peter Collingbourne
@ 2021-05-28 1:04 ` Peter Collingbourne
2021-05-28 16:33 ` kernel test robot
2021-05-28 1:04 ` [PATCH v4 2/4] kasan: use separate (un)poison implementation for integrated init Peter Collingbourne
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Peter Collingbourne @ 2021-05-28 1:04 UTC (permalink / raw)
To: Andrey Konovalov, Alexander Potapenko, Catalin Marinas,
Vincenzo Frascino, Andrew Morton, Jann Horn
Cc: Peter Collingbourne, Evgenii Stepanov, linux-mm, linux-arm-kernel
In an upcoming change we would like to add a flag to
GFP_HIGHUSER_MOVABLE so that it would no longer be an OR
of GFP_HIGHUSER and __GFP_MOVABLE. This poses a problem for
alloc_zeroed_user_highpage_movable() which passes __GFP_MOVABLE
into an arch-specific __alloc_zeroed_user_highpage() hook which ORs
in GFP_HIGHUSER.
Since __alloc_zeroed_user_highpage() is only ever called from
alloc_zeroed_user_highpage_movable(), we can remove one level
of indirection here. Remove __alloc_zeroed_user_highpage(),
make alloc_zeroed_user_highpage_movable() the hook, and use
GFP_HIGHUSER_MOVABLE in the hook implementations so that they will
pick up the new flag that we are going to add.
Signed-off-by: Peter Collingbourne <pcc@google.com>
Link: https://linux-review.googlesource.com/id/Ic6361c657b2cdcd896adbe0cf7cb5a7fbb1ed7bf
---
arch/alpha/include/asm/page.h | 6 +++---
arch/arm64/include/asm/page.h | 6 +++---
arch/ia64/include/asm/page.h | 6 +++---
arch/m68k/include/asm/page_no.h | 6 +++---
arch/s390/include/asm/page.h | 6 +++---
arch/x86/include/asm/page.h | 6 +++---
include/linux/highmem.h | 35 ++++++++-------------------------
7 files changed, 26 insertions(+), 45 deletions(-)
diff --git a/arch/alpha/include/asm/page.h b/arch/alpha/include/asm/page.h
index 268f99b4602b..18f48a6f2ff6 100644
--- a/arch/alpha/include/asm/page.h
+++ b/arch/alpha/include/asm/page.h
@@ -17,9 +17,9 @@
extern void clear_page(void *page);
#define clear_user_page(page, vaddr, pg) clear_page(page)
-#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
- alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vmaddr)
-#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE
+#define alloc_zeroed_user_highpage_movable(vma, vaddr) \
+ alloc_page_vma(GFP_HIGHUSER_MOVABLE | __GFP_ZERO, vma, vmaddr)
+#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE_MOVABLE
extern void copy_page(void * _to, void * _from);
#define copy_user_page(to, from, vaddr, pg) copy_page(to, from)
diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h
index 012cffc574e8..0cfe4f7e7055 100644
--- a/arch/arm64/include/asm/page.h
+++ b/arch/arm64/include/asm/page.h
@@ -28,9 +28,9 @@ void copy_user_highpage(struct page *to, struct page *from,
void copy_highpage(struct page *to, struct page *from);
#define __HAVE_ARCH_COPY_HIGHPAGE
-#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
- alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
-#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE
+#define alloc_zeroed_user_highpage_movable(movableflags, vma, vaddr) \
+ alloc_page_vma(GFP_HIGHUSER_MOVABLE | __GFP_ZERO, vma, vaddr)
+#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE_MOVABLE
#define clear_user_page(page, vaddr, pg) clear_page(page)
#define copy_user_page(to, from, vaddr, pg) copy_page(to, from)
diff --git a/arch/ia64/include/asm/page.h b/arch/ia64/include/asm/page.h
index f4dc81fa7146..1b990466d540 100644
--- a/arch/ia64/include/asm/page.h
+++ b/arch/ia64/include/asm/page.h
@@ -82,16 +82,16 @@ do { \
} while (0)
-#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
+#define alloc_zeroed_user_highpage_movable(vma, vaddr) \
({ \
struct page *page = alloc_page_vma( \
- GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr); \
+ GFP_HIGHUSER_MOVABLE | __GFP_ZERO, vma, vaddr); \
if (page) \
flush_dcache_page(page); \
page; \
})
-#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE
+#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE_MOVABLE
#define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
diff --git a/arch/m68k/include/asm/page_no.h b/arch/m68k/include/asm/page_no.h
index 8d0f862ee9d7..c9d0d84158a4 100644
--- a/arch/m68k/include/asm/page_no.h
+++ b/arch/m68k/include/asm/page_no.h
@@ -13,9 +13,9 @@ extern unsigned long memory_end;
#define clear_user_page(page, vaddr, pg) clear_page(page)
#define copy_user_page(to, from, vaddr, pg) copy_page(to, from)
-#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
- alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
-#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE
+#define alloc_zeroed_user_highpage_movable(vma, vaddr) \
+ alloc_page_vma(GFP_HIGHUSER_MOVABLE | __GFP_ZERO, vma, vaddr)
+#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE_MOVABLE
#define __pa(vaddr) ((unsigned long)(vaddr))
#define __va(paddr) ((void *)((unsigned long)(paddr)))
diff --git a/arch/s390/include/asm/page.h b/arch/s390/include/asm/page.h
index cc98f9b78fd4..346a0cbb6515 100644
--- a/arch/s390/include/asm/page.h
+++ b/arch/s390/include/asm/page.h
@@ -68,9 +68,9 @@ static inline void copy_page(void *to, void *from)
#define clear_user_page(page, vaddr, pg) clear_page(page)
#define copy_user_page(to, from, vaddr, pg) copy_page(to, from)
-#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
- alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
-#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE
+#define __alloc_zeroed_user_highpage_movable(vma, vaddr) \
+ alloc_page_vma(GFP_HIGHUSER_MOVABLE | __GFP_ZERO, vma, vaddr)
+#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE_MOVABLE
/*
* These are used to make use of C type-checking..
diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h
index 7555b48803a8..4d5810c8fab7 100644
--- a/arch/x86/include/asm/page.h
+++ b/arch/x86/include/asm/page.h
@@ -34,9 +34,9 @@ static inline void copy_user_page(void *to, void *from, unsigned long vaddr,
copy_page(to, from);
}
-#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
- alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
-#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE
+#define alloc_zeroed_user_highpage_movable(vma, vaddr) \
+ alloc_page_vma(GFP_HIGHUSER_MOVABLE | __GFP_ZERO, vma, vaddr)
+#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE_MOVABLE
#ifndef __pa
#define __pa(x) __phys_addr((unsigned long)(x))
diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index 832b49b50c7b..54d0643b8fcf 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -152,28 +152,24 @@ static inline void clear_user_highpage(struct page *page, unsigned long vaddr)
}
#endif
-#ifndef __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE
+#ifndef __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE_MOVABLE
/**
- * __alloc_zeroed_user_highpage - Allocate a zeroed HIGHMEM page for a VMA with caller-specified movable GFP flags
- * @movableflags: The GFP flags related to the pages future ability to move like __GFP_MOVABLE
+ * alloc_zeroed_user_highpage_movable - Allocate a zeroed HIGHMEM page for a VMA that the caller knows can move
* @vma: The VMA the page is to be allocated for
* @vaddr: The virtual address the page will be inserted into
*
- * This function will allocate a page for a VMA but the caller is expected
- * to specify via movableflags whether the page will be movable in the
- * future or not
+ * This function will allocate a page for a VMA that the caller knows will
+ * be able to migrate in the future using move_pages() or reclaimed
*
* An architecture may override this function by defining
- * __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE and providing their own
+ * __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE_MOVABLE and providing their own
* implementation.
*/
static inline struct page *
-__alloc_zeroed_user_highpage(gfp_t movableflags,
- struct vm_area_struct *vma,
- unsigned long vaddr)
+alloc_zeroed_user_highpage_movable(struct vm_area_struct *vma,
+ unsigned long vaddr)
{
- struct page *page = alloc_page_vma(GFP_HIGHUSER | movableflags,
- vma, vaddr);
+ struct page *page = alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma, vaddr);
if (page)
clear_user_highpage(page, vaddr);
@@ -182,21 +178,6 @@ __alloc_zeroed_user_highpage(gfp_t movableflags,
}
#endif
-/**
- * alloc_zeroed_user_highpage_movable - Allocate a zeroed HIGHMEM page for a VMA that the caller knows can move
- * @vma: The VMA the page is to be allocated for
- * @vaddr: The virtual address the page will be inserted into
- *
- * This function will allocate a page for a VMA that the caller knows will
- * be able to migrate in the future using move_pages() or reclaimed
- */
-static inline struct page *
-alloc_zeroed_user_highpage_movable(struct vm_area_struct *vma,
- unsigned long vaddr)
-{
- return __alloc_zeroed_user_highpage(__GFP_MOVABLE, vma, vaddr);
-}
-
static inline void clear_highpage(struct page *page)
{
void *kaddr = kmap_atomic(page);
--
2.32.0.rc0.204.g9fa02ecfa5-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v4 1/4] mm: arch: remove indirection level in alloc_zeroed_user_highpage_movable()
2021-05-28 1:04 ` [PATCH v4 1/4] mm: arch: remove indirection level in alloc_zeroed_user_highpage_movable() Peter Collingbourne
@ 2021-05-28 16:33 ` kernel test robot
0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-05-28 16:33 UTC (permalink / raw)
To: Peter Collingbourne, Andrey Konovalov, Alexander Potapenko,
Catalin Marinas, Vincenzo Frascino, Andrew Morton, Jann Horn
Cc: kbuild-all, clang-built-linux, Linux Memory Management List,
Peter Collingbourne, Evgenii Stepanov, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 22230 bytes --]
Hi Peter,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on m68knommu/for-next s390/features tip/x86/core tip/perf/core linus/master v5.13-rc3 next-20210528]
[cannot apply to hnaz-linux-mm/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Peter-Collingbourne/arm64-improve-efficiency-of-setting-tags-for-user-pages/20210528-090548
base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: s390-randconfig-r025-20210528 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 6505c630407c5feec818f0bb1c284f9eeebf2071)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/0day-ci/linux/commit/32afff0556041349738b201907c1c81cabb5ca10
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Peter-Collingbourne/arm64-improve-efficiency-of-setting-tags-for-user-pages/20210528-090548
git checkout 32afff0556041349738b201907c1c81cabb5ca10
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
In file included from mm/memory.c:50:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:22:
In file included from include/linux/writeback.h:14:
In file included from include/linux/blk-cgroup.h:23:
In file included from include/linux/blkdev.h:25:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:36:59: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
^
include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
#define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
^
In file included from mm/memory.c:50:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:22:
In file included from include/linux/writeback.h:14:
In file included from include/linux/blk-cgroup.h:23:
In file included from include/linux/blkdev.h:25:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
^
In file included from mm/memory.c:50:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:22:
In file included from include/linux/writeback.h:14:
In file included from include/linux/blk-cgroup.h:23:
In file included from include/linux/blkdev.h:25:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> mm/memory.c:2891:14: error: implicit declaration of function 'alloc_zeroed_user_highpage_movable' [-Werror,-Wimplicit-function-declaration]
new_page = alloc_zeroed_user_highpage_movable(vma,
^
>> mm/memory.c:2891:12: warning: incompatible integer to pointer conversion assigning to 'struct page *' from 'int' [-Wint-conversion]
new_page = alloc_zeroed_user_highpage_movable(vma,
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mm/memory.c:3589:9: error: implicit declaration of function 'alloc_zeroed_user_highpage_movable' [-Werror,-Wimplicit-function-declaration]
page = alloc_zeroed_user_highpage_movable(vma, vmf->address);
^
mm/memory.c:3589:7: warning: incompatible integer to pointer conversion assigning to 'struct page *' from 'int' [-Wint-conversion]
page = alloc_zeroed_user_highpage_movable(vma, vmf->address);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
14 warnings and 2 errors generated.
vim +/alloc_zeroed_user_highpage_movable +2891 mm/memory.c
4e047f89777122 Shachar Raindel 2015-04-14 2860
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2861 /*
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2862 * Handle the case of a page which we actually need to copy to a new page.
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2863 *
c1e8d7c6a7a682 Michel Lespinasse 2020-06-08 2864 * Called with mmap_lock locked and the old page referenced, but
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2865 * without the ptl held.
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2866 *
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2867 * High level logic flow:
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2868 *
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2869 * - Allocate a page, copy the content of the old page to the new one.
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2870 * - Handle book keeping and accounting - cgroups, mmu-notifiers, etc.
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2871 * - Take the PTL. If the pte changed, bail out and release the allocated page
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2872 * - If the pte is still the way we remember it, update the page table and all
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2873 * relevant references. This includes dropping the reference the page-table
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2874 * held to the old page, as well as updating the rmap.
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2875 * - In any case, unlock the PTL and drop the reference we took to the old page.
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2876 */
2b7403035459c7 Souptick Joarder 2018-08-23 2877 static vm_fault_t wp_page_copy(struct vm_fault *vmf)
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2878 {
82b0f8c39a3869 Jan Kara 2016-12-14 2879 struct vm_area_struct *vma = vmf->vma;
bae473a423f65e Kirill A. Shutemov 2016-07-26 2880 struct mm_struct *mm = vma->vm_mm;
a41b70d6dfc28b Jan Kara 2016-12-14 2881 struct page *old_page = vmf->page;
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2882 struct page *new_page = NULL;
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2883 pte_t entry;
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2884 int page_copied = 0;
ac46d4f3c43241 Jérôme Glisse 2018-12-28 2885 struct mmu_notifier_range range;
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2886
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2887 if (unlikely(anon_vma_prepare(vma)))
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2888 goto oom;
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2889
2994302bc8a171 Jan Kara 2016-12-14 2890 if (is_zero_pfn(pte_pfn(vmf->orig_pte))) {
82b0f8c39a3869 Jan Kara 2016-12-14 @2891 new_page = alloc_zeroed_user_highpage_movable(vma,
82b0f8c39a3869 Jan Kara 2016-12-14 2892 vmf->address);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2893 if (!new_page)
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2894 goto oom;
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2895 } else {
bae473a423f65e Kirill A. Shutemov 2016-07-26 2896 new_page = alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma,
82b0f8c39a3869 Jan Kara 2016-12-14 2897 vmf->address);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2898 if (!new_page)
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2899 goto oom;
83d116c53058d5 Jia He 2019-10-11 2900
83d116c53058d5 Jia He 2019-10-11 2901 if (!cow_user_page(new_page, old_page, vmf)) {
83d116c53058d5 Jia He 2019-10-11 2902 /*
83d116c53058d5 Jia He 2019-10-11 2903 * COW failed, if the fault was solved by other,
83d116c53058d5 Jia He 2019-10-11 2904 * it's fine. If not, userspace would re-fault on
83d116c53058d5 Jia He 2019-10-11 2905 * the same address and we will handle the fault
83d116c53058d5 Jia He 2019-10-11 2906 * from the second attempt.
83d116c53058d5 Jia He 2019-10-11 2907 */
83d116c53058d5 Jia He 2019-10-11 2908 put_page(new_page);
83d116c53058d5 Jia He 2019-10-11 2909 if (old_page)
83d116c53058d5 Jia He 2019-10-11 2910 put_page(old_page);
83d116c53058d5 Jia He 2019-10-11 2911 return 0;
83d116c53058d5 Jia He 2019-10-11 2912 }
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2913 }
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2914
d9eb1ea2bf8734 Johannes Weiner 2020-06-03 2915 if (mem_cgroup_charge(new_page, mm, GFP_KERNEL))
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2916 goto oom_free_new;
9d82c69438d0df Johannes Weiner 2020-06-03 2917 cgroup_throttle_swaprate(new_page, GFP_KERNEL);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2918
eb3c24f305e56c Mel Gorman 2015-06-24 2919 __SetPageUptodate(new_page);
eb3c24f305e56c Mel Gorman 2015-06-24 2920
7269f999934b28 Jérôme Glisse 2019-05-13 2921 mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma, mm,
6f4f13e8d9e27c Jérôme Glisse 2019-05-13 2922 vmf->address & PAGE_MASK,
ac46d4f3c43241 Jérôme Glisse 2018-12-28 2923 (vmf->address & PAGE_MASK) + PAGE_SIZE);
ac46d4f3c43241 Jérôme Glisse 2018-12-28 2924 mmu_notifier_invalidate_range_start(&range);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2925
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2926 /*
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2927 * Re-check the pte - we dropped the lock
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2928 */
82b0f8c39a3869 Jan Kara 2016-12-14 2929 vmf->pte = pte_offset_map_lock(mm, vmf->pmd, vmf->address, &vmf->ptl);
2994302bc8a171 Jan Kara 2016-12-14 2930 if (likely(pte_same(*vmf->pte, vmf->orig_pte))) {
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2931 if (old_page) {
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2932 if (!PageAnon(old_page)) {
eca56ff906bdd0 Jerome Marchand 2016-01-14 2933 dec_mm_counter_fast(mm,
eca56ff906bdd0 Jerome Marchand 2016-01-14 2934 mm_counter_file(old_page));
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2935 inc_mm_counter_fast(mm, MM_ANONPAGES);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2936 }
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2937 } else {
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2938 inc_mm_counter_fast(mm, MM_ANONPAGES);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2939 }
2994302bc8a171 Jan Kara 2016-12-14 2940 flush_cache_page(vma, vmf->address, pte_pfn(vmf->orig_pte));
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2941 entry = mk_pte(new_page, vma->vm_page_prot);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2942 entry = maybe_mkwrite(pte_mkdirty(entry), vma);
111fe7186b29d1 Nicholas Piggin 2020-12-29 2943
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2944 /*
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2945 * Clear the pte entry and flush it first, before updating the
111fe7186b29d1 Nicholas Piggin 2020-12-29 2946 * pte with the new entry, to keep TLBs on different CPUs in
111fe7186b29d1 Nicholas Piggin 2020-12-29 2947 * sync. This code used to set the new PTE then flush TLBs, but
111fe7186b29d1 Nicholas Piggin 2020-12-29 2948 * that left a window where the new PTE could be loaded into
111fe7186b29d1 Nicholas Piggin 2020-12-29 2949 * some TLBs while the old PTE remains in others.
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2950 */
82b0f8c39a3869 Jan Kara 2016-12-14 2951 ptep_clear_flush_notify(vma, vmf->address, vmf->pte);
82b0f8c39a3869 Jan Kara 2016-12-14 2952 page_add_new_anon_rmap(new_page, vma, vmf->address, false);
b518154e59aab3 Joonsoo Kim 2020-08-11 2953 lru_cache_add_inactive_or_unevictable(new_page, vma);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2954 /*
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2955 * We call the notify macro here because, when using secondary
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2956 * mmu page tables (such as kvm shadow page tables), we want the
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2957 * new page to be mapped directly into the secondary page table.
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2958 */
82b0f8c39a3869 Jan Kara 2016-12-14 2959 set_pte_at_notify(mm, vmf->address, vmf->pte, entry);
82b0f8c39a3869 Jan Kara 2016-12-14 2960 update_mmu_cache(vma, vmf->address, vmf->pte);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2961 if (old_page) {
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2962 /*
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2963 * Only after switching the pte to the new page may
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2964 * we remove the mapcount here. Otherwise another
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2965 * process may come and find the rmap count decremented
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2966 * before the pte is switched to the new page, and
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2967 * "reuse" the old page writing into it while our pte
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2968 * here still points into it and can be read by other
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2969 * threads.
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2970 *
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2971 * The critical issue is to order this
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2972 * page_remove_rmap with the ptp_clear_flush above.
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2973 * Those stores are ordered by (if nothing else,)
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2974 * the barrier present in the atomic_add_negative
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2975 * in page_remove_rmap.
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2976 *
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2977 * Then the TLB flush in ptep_clear_flush ensures that
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2978 * no process can access the old page before the
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2979 * decremented mapcount is visible. And the old page
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2980 * cannot be reused until after the decremented
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2981 * mapcount is visible. So transitively, TLBs to
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2982 * old page will be flushed before it can be reused.
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2983 */
d281ee61451835 Kirill A. Shutemov 2016-01-15 2984 page_remove_rmap(old_page, false);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2985 }
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2986
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2987 /* Free the old page.. */
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2988 new_page = old_page;
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2989 page_copied = 1;
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2990 } else {
7df676974359f9 Bibo Mao 2020-05-27 2991 update_mmu_tlb(vma, vmf->address, vmf->pte);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2992 }
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2993
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2994 if (new_page)
09cbfeaf1a5a67 Kirill A. Shutemov 2016-04-01 2995 put_page(new_page);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 2996
82b0f8c39a3869 Jan Kara 2016-12-14 2997 pte_unmap_unlock(vmf->pte, vmf->ptl);
4645b9fe84bf48 Jérôme Glisse 2017-11-15 2998 /*
4645b9fe84bf48 Jérôme Glisse 2017-11-15 2999 * No need to double call mmu_notifier->invalidate_range() callback as
4645b9fe84bf48 Jérôme Glisse 2017-11-15 3000 * the above ptep_clear_flush_notify() did already call it.
4645b9fe84bf48 Jérôme Glisse 2017-11-15 3001 */
ac46d4f3c43241 Jérôme Glisse 2018-12-28 3002 mmu_notifier_invalidate_range_only_end(&range);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3003 if (old_page) {
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3004 /*
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3005 * Don't let another task, with possibly unlocked vma,
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3006 * keep the mlocked page.
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3007 */
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3008 if (page_copied && (vma->vm_flags & VM_LOCKED)) {
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3009 lock_page(old_page); /* LRU manipulation */
e90309c9f7722d Kirill A. Shutemov 2016-01-15 3010 if (PageMlocked(old_page))
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3011 munlock_vma_page(old_page);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3012 unlock_page(old_page);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3013 }
09cbfeaf1a5a67 Kirill A. Shutemov 2016-04-01 3014 put_page(old_page);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3015 }
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3016 return page_copied ? VM_FAULT_WRITE : 0;
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3017 oom_free_new:
09cbfeaf1a5a67 Kirill A. Shutemov 2016-04-01 3018 put_page(new_page);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3019 oom:
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3020 if (old_page)
09cbfeaf1a5a67 Kirill A. Shutemov 2016-04-01 3021 put_page(old_page);
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3022 return VM_FAULT_OOM;
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3023 }
2f38ab2c3c7fef Shachar Raindel 2015-04-14 3024
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 19535 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 2/4] kasan: use separate (un)poison implementation for integrated init
2021-05-28 1:04 [PATCH v4 0/4] arm64: improve efficiency of setting tags for user pages Peter Collingbourne
2021-05-28 1:04 ` [PATCH v4 1/4] mm: arch: remove indirection level in alloc_zeroed_user_highpage_movable() Peter Collingbourne
@ 2021-05-28 1:04 ` Peter Collingbourne
2021-05-28 10:25 ` Andrey Konovalov
2021-05-28 1:04 ` [PATCH v4 3/4] arm64: mte: handle tags zeroing at page allocation time Peter Collingbourne
2021-05-28 1:04 ` [PATCH v4 4/4] kasan: disable freed user page poisoning with HW tags Peter Collingbourne
3 siblings, 1 reply; 9+ messages in thread
From: Peter Collingbourne @ 2021-05-28 1:04 UTC (permalink / raw)
To: Andrey Konovalov, Alexander Potapenko, Catalin Marinas,
Vincenzo Frascino, Andrew Morton, Jann Horn
Cc: Peter Collingbourne, Evgenii Stepanov, linux-mm, linux-arm-kernel
Currently with integrated init page_alloc.c needs to know whether
kasan_alloc_pages() will zero initialize memory, but this will start
becoming more complicated once we start adding tag initialization
support for user pages. To avoid page_alloc.c needing to know more
details of what integrated init will do, move the unpoisoning logic
for integrated init into the HW tags implementation. Currently the
logic is identical but it will diverge in subsequent patches.
For symmetry do the same for poisoning although this logic will
be unaffected by subsequent patches.
Signed-off-by: Peter Collingbourne <pcc@google.com>
Link: https://linux-review.googlesource.com/id/I2c550234c6c4a893c48c18ff0c6ce658c7c67056
---
v4:
- use IS_ENABLED(CONFIG_KASAN)
- add comments to kasan_alloc_pages and kasan_free_pages
- remove line break
v3:
- use BUILD_BUG()
v2:
- fix build with KASAN disabled
include/linux/kasan.h | 64 +++++++++++++++++++++++++------------------
mm/kasan/common.c | 4 +--
mm/kasan/hw_tags.c | 22 +++++++++++++++
mm/mempool.c | 6 ++--
mm/page_alloc.c | 55 +++++++++++++++++++------------------
5 files changed, 95 insertions(+), 56 deletions(-)
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index b1678a61e6a7..a1c7ce5f3e4f 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -2,6 +2,7 @@
#ifndef _LINUX_KASAN_H
#define _LINUX_KASAN_H
+#include <linux/bug.h>
#include <linux/static_key.h>
#include <linux/types.h>
@@ -79,14 +80,6 @@ static inline void kasan_disable_current(void) {}
#endif /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */
-#ifdef CONFIG_KASAN
-
-struct kasan_cache {
- int alloc_meta_offset;
- int free_meta_offset;
- bool is_kmalloc;
-};
-
#ifdef CONFIG_KASAN_HW_TAGS
DECLARE_STATIC_KEY_FALSE(kasan_flag_enabled);
@@ -101,11 +94,14 @@ static inline bool kasan_has_integrated_init(void)
return kasan_enabled();
}
+void kasan_alloc_pages(struct page *page, unsigned int order, gfp_t flags);
+void kasan_free_pages(struct page *page, unsigned int order);
+
#else /* CONFIG_KASAN_HW_TAGS */
static inline bool kasan_enabled(void)
{
- return true;
+ return IS_ENABLED(CONFIG_KASAN);
}
static inline bool kasan_has_integrated_init(void)
@@ -113,8 +109,30 @@ static inline bool kasan_has_integrated_init(void)
return false;
}
+static __always_inline void kasan_alloc_pages(struct page *page,
+ unsigned int order, gfp_t flags)
+{
+ /* Only available for integrated init. */
+ BUILD_BUG();
+}
+
+static __always_inline void kasan_free_pages(struct page *page,
+ unsigned int order)
+{
+ /* Only available for integrated init. */
+ BUILD_BUG();
+}
+
#endif /* CONFIG_KASAN_HW_TAGS */
+#ifdef CONFIG_KASAN
+
+struct kasan_cache {
+ int alloc_meta_offset;
+ int free_meta_offset;
+ bool is_kmalloc;
+};
+
slab_flags_t __kasan_never_merge(void);
static __always_inline slab_flags_t kasan_never_merge(void)
{
@@ -130,20 +148,20 @@ static __always_inline void kasan_unpoison_range(const void *addr, size_t size)
__kasan_unpoison_range(addr, size);
}
-void __kasan_alloc_pages(struct page *page, unsigned int order, bool init);
-static __always_inline void kasan_alloc_pages(struct page *page,
+void __kasan_poison_pages(struct page *page, unsigned int order, bool init);
+static __always_inline void kasan_poison_pages(struct page *page,
unsigned int order, bool init)
{
if (kasan_enabled())
- __kasan_alloc_pages(page, order, init);
+ __kasan_poison_pages(page, order, init);
}
-void __kasan_free_pages(struct page *page, unsigned int order, bool init);
-static __always_inline void kasan_free_pages(struct page *page,
- unsigned int order, bool init)
+void __kasan_unpoison_pages(struct page *page, unsigned int order, bool init);
+static __always_inline void kasan_unpoison_pages(struct page *page,
+ unsigned int order, bool init)
{
if (kasan_enabled())
- __kasan_free_pages(page, order, init);
+ __kasan_unpoison_pages(page, order, init);
}
void __kasan_cache_create(struct kmem_cache *cache, unsigned int *size,
@@ -285,21 +303,15 @@ void kasan_restore_multi_shot(bool enabled);
#else /* CONFIG_KASAN */
-static inline bool kasan_enabled(void)
-{
- return false;
-}
-static inline bool kasan_has_integrated_init(void)
-{
- return false;
-}
static inline slab_flags_t kasan_never_merge(void)
{
return 0;
}
static inline void kasan_unpoison_range(const void *address, size_t size) {}
-static inline void kasan_alloc_pages(struct page *page, unsigned int order, bool init) {}
-static inline void kasan_free_pages(struct page *page, unsigned int order, bool init) {}
+static inline void kasan_poison_pages(struct page *page, unsigned int order,
+ bool init) {}
+static inline void kasan_unpoison_pages(struct page *page, unsigned int order,
+ bool init) {}
static inline void kasan_cache_create(struct kmem_cache *cache,
unsigned int *size,
slab_flags_t *flags) {}
diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index 6bb87f2acd4e..0ecd293af344 100644
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -97,7 +97,7 @@ slab_flags_t __kasan_never_merge(void)
return 0;
}
-void __kasan_alloc_pages(struct page *page, unsigned int order, bool init)
+void __kasan_unpoison_pages(struct page *page, unsigned int order, bool init)
{
u8 tag;
unsigned long i;
@@ -111,7 +111,7 @@ void __kasan_alloc_pages(struct page *page, unsigned int order, bool init)
kasan_unpoison(page_address(page), PAGE_SIZE << order, init);
}
-void __kasan_free_pages(struct page *page, unsigned int order, bool init)
+void __kasan_poison_pages(struct page *page, unsigned int order, bool init)
{
if (likely(!PageHighMem(page)))
kasan_poison(page_address(page), PAGE_SIZE << order,
diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
index 4004388b4e4b..9d0f6f934016 100644
--- a/mm/kasan/hw_tags.c
+++ b/mm/kasan/hw_tags.c
@@ -238,6 +238,28 @@ struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
return &alloc_meta->free_track[0];
}
+void kasan_alloc_pages(struct page *page, unsigned int order, gfp_t flags)
+{
+ /*
+ * This condition should match the one in post_alloc_hook() in
+ * page_alloc.c.
+ */
+ bool init = !want_init_on_free() && want_init_on_alloc(flags);
+
+ kasan_unpoison_pages(page, order, init);
+}
+
+void kasan_free_pages(struct page *page, unsigned int order)
+{
+ /*
+ * This condition should match the one in free_pages_prepare() in
+ * page_alloc.c.
+ */
+ bool init = want_init_on_free();
+
+ kasan_poison_pages(page, order, init);
+}
+
#if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
void kasan_set_tagging_report_once(bool state)
diff --git a/mm/mempool.c b/mm/mempool.c
index a258cf4de575..0b8afbec3e35 100644
--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -106,7 +106,8 @@ static __always_inline void kasan_poison_element(mempool_t *pool, void *element)
if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc)
kasan_slab_free_mempool(element);
else if (pool->alloc == mempool_alloc_pages)
- kasan_free_pages(element, (unsigned long)pool->pool_data, false);
+ kasan_poison_pages(element, (unsigned long)pool->pool_data,
+ false);
}
static void kasan_unpoison_element(mempool_t *pool, void *element)
@@ -114,7 +115,8 @@ static void kasan_unpoison_element(mempool_t *pool, void *element)
if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc)
kasan_unpoison_range(element, __ksize(element));
else if (pool->alloc == mempool_alloc_pages)
- kasan_alloc_pages(element, (unsigned long)pool->pool_data, false);
+ kasan_unpoison_pages(element, (unsigned long)pool->pool_data,
+ false);
}
static __always_inline void add_element(mempool_t *pool, void *element)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index aaa1655cf682..4fddb7cac3c6 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -382,7 +382,7 @@ int page_group_by_mobility_disabled __read_mostly;
static DEFINE_STATIC_KEY_TRUE(deferred_pages);
/*
- * Calling kasan_free_pages() only after deferred memory initialization
+ * Calling kasan_poison_pages() only after deferred memory initialization
* has completed. Poisoning pages during deferred memory init will greatly
* lengthen the process and cause problem in large memory systems as the
* deferred pages initialization is done with interrupt disabled.
@@ -394,15 +394,11 @@ static DEFINE_STATIC_KEY_TRUE(deferred_pages);
* on-demand allocation and then freed again before the deferred pages
* initialization is done, but this is not likely to happen.
*/
-static inline void kasan_free_nondeferred_pages(struct page *page, int order,
- bool init, fpi_t fpi_flags)
+static inline bool should_skip_kasan_poison(fpi_t fpi_flags)
{
- if (static_branch_unlikely(&deferred_pages))
- return;
- if (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
- (fpi_flags & FPI_SKIP_KASAN_POISON))
- return;
- kasan_free_pages(page, order, init);
+ return static_branch_unlikely(&deferred_pages) ||
+ (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
+ (fpi_flags & FPI_SKIP_KASAN_POISON));
}
/* Returns true if the struct page for the pfn is uninitialised */
@@ -453,13 +449,10 @@ defer_init(int nid, unsigned long pfn, unsigned long end_pfn)
return false;
}
#else
-static inline void kasan_free_nondeferred_pages(struct page *page, int order,
- bool init, fpi_t fpi_flags)
+static inline bool should_skip_kasan_poison(fpi_t fpi_flags)
{
- if (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
- (fpi_flags & FPI_SKIP_KASAN_POISON))
- return;
- kasan_free_pages(page, order, init);
+ return (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
+ (fpi_flags & FPI_SKIP_KASAN_POISON));
}
static inline bool early_page_uninitialised(unsigned long pfn)
@@ -1245,7 +1238,7 @@ static __always_inline bool free_pages_prepare(struct page *page,
unsigned int order, bool check_free, fpi_t fpi_flags)
{
int bad = 0;
- bool init;
+ bool skip_kasan_poison = should_skip_kasan_poison(fpi_flags);
VM_BUG_ON_PAGE(PageTail(page), page);
@@ -1314,10 +1307,17 @@ static __always_inline bool free_pages_prepare(struct page *page,
* With hardware tag-based KASAN, memory tags must be set before the
* page becomes unavailable via debug_pagealloc or arch_free_page.
*/
- init = want_init_on_free();
- if (init && !kasan_has_integrated_init())
- kernel_init_free_pages(page, 1 << order);
- kasan_free_nondeferred_pages(page, order, init, fpi_flags);
+ if (kasan_has_integrated_init()) {
+ if (!skip_kasan_poison)
+ kasan_free_pages(page, order);
+ } else {
+ bool init = want_init_on_free();
+
+ if (init)
+ kernel_init_free_pages(page, 1 << order);
+ if (!skip_kasan_poison)
+ kasan_poison_pages(page, order, init);
+ }
/*
* arch_free_page() can make the page's contents inaccessible. s390
@@ -2324,8 +2324,6 @@ static bool check_new_pages(struct page *page, unsigned int order)
inline void post_alloc_hook(struct page *page, unsigned int order,
gfp_t gfp_flags)
{
- bool init;
-
set_page_private(page, 0);
set_page_refcounted(page);
@@ -2344,10 +2342,15 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
* kasan_alloc_pages and kernel_init_free_pages must be
* kept together to avoid discrepancies in behavior.
*/
- init = !want_init_on_free() && want_init_on_alloc(gfp_flags);
- kasan_alloc_pages(page, order, init);
- if (init && !kasan_has_integrated_init())
- kernel_init_free_pages(page, 1 << order);
+ if (kasan_has_integrated_init()) {
+ kasan_alloc_pages(page, order, gfp_flags);
+ } else {
+ bool init = !want_init_on_free() && want_init_on_alloc(gfp_flags);
+
+ kasan_unpoison_pages(page, order, init);
+ if (init)
+ kernel_init_free_pages(page, 1 << order);
+ }
set_page_owner(page, order, gfp_flags);
}
--
2.32.0.rc0.204.g9fa02ecfa5-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v4 2/4] kasan: use separate (un)poison implementation for integrated init
2021-05-28 1:04 ` [PATCH v4 2/4] kasan: use separate (un)poison implementation for integrated init Peter Collingbourne
@ 2021-05-28 10:25 ` Andrey Konovalov
2021-06-01 19:28 ` Peter Collingbourne
0 siblings, 1 reply; 9+ messages in thread
From: Andrey Konovalov @ 2021-05-28 10:25 UTC (permalink / raw)
To: Peter Collingbourne
Cc: Alexander Potapenko, Catalin Marinas, Vincenzo Frascino,
Andrew Morton, Jann Horn, Evgenii Stepanov,
Linux Memory Management List, linux-arm-kernel
On Fri, May 28, 2021 at 4:04 AM Peter Collingbourne <pcc@google.com> wrote:
>
> Currently with integrated init page_alloc.c needs to know whether
> kasan_alloc_pages() will zero initialize memory, but this will start
> becoming more complicated once we start adding tag initialization
> support for user pages. To avoid page_alloc.c needing to know more
> details of what integrated init will do, move the unpoisoning logic
> for integrated init into the HW tags implementation. Currently the
> logic is identical but it will diverge in subsequent patches.
>
> For symmetry do the same for poisoning although this logic will
> be unaffected by subsequent patches.
>
> Signed-off-by: Peter Collingbourne <pcc@google.com>
> Link: https://linux-review.googlesource.com/id/I2c550234c6c4a893c48c18ff0c6ce658c7c67056
> ---
> v4:
> - use IS_ENABLED(CONFIG_KASAN)
> - add comments to kasan_alloc_pages and kasan_free_pages
> - remove line break
>
> v3:
> - use BUILD_BUG()
>
> v2:
> - fix build with KASAN disabled
>
> include/linux/kasan.h | 64 +++++++++++++++++++++++++------------------
> mm/kasan/common.c | 4 +--
> mm/kasan/hw_tags.c | 22 +++++++++++++++
> mm/mempool.c | 6 ++--
> mm/page_alloc.c | 55 +++++++++++++++++++------------------
> 5 files changed, 95 insertions(+), 56 deletions(-)
>
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index b1678a61e6a7..a1c7ce5f3e4f 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -2,6 +2,7 @@
> #ifndef _LINUX_KASAN_H
> #define _LINUX_KASAN_H
>
> +#include <linux/bug.h>
> #include <linux/static_key.h>
> #include <linux/types.h>
>
> @@ -79,14 +80,6 @@ static inline void kasan_disable_current(void) {}
>
> #endif /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */
>
> -#ifdef CONFIG_KASAN
> -
> -struct kasan_cache {
> - int alloc_meta_offset;
> - int free_meta_offset;
> - bool is_kmalloc;
> -};
> -
> #ifdef CONFIG_KASAN_HW_TAGS
>
> DECLARE_STATIC_KEY_FALSE(kasan_flag_enabled);
> @@ -101,11 +94,14 @@ static inline bool kasan_has_integrated_init(void)
> return kasan_enabled();
> }
>
> +void kasan_alloc_pages(struct page *page, unsigned int order, gfp_t flags);
> +void kasan_free_pages(struct page *page, unsigned int order);
> +
> #else /* CONFIG_KASAN_HW_TAGS */
>
> static inline bool kasan_enabled(void)
> {
> - return true;
> + return IS_ENABLED(CONFIG_KASAN);
> }
>
> static inline bool kasan_has_integrated_init(void)
> @@ -113,8 +109,30 @@ static inline bool kasan_has_integrated_init(void)
> return false;
> }
>
> +static __always_inline void kasan_alloc_pages(struct page *page,
> + unsigned int order, gfp_t flags)
> +{
> + /* Only available for integrated init. */
> + BUILD_BUG();
> +}
> +
> +static __always_inline void kasan_free_pages(struct page *page,
> + unsigned int order)
> +{
> + /* Only available for integrated init. */
> + BUILD_BUG();
> +}
> +
> #endif /* CONFIG_KASAN_HW_TAGS */
>
> +#ifdef CONFIG_KASAN
> +
> +struct kasan_cache {
> + int alloc_meta_offset;
> + int free_meta_offset;
> + bool is_kmalloc;
> +};
> +
> slab_flags_t __kasan_never_merge(void);
> static __always_inline slab_flags_t kasan_never_merge(void)
> {
> @@ -130,20 +148,20 @@ static __always_inline void kasan_unpoison_range(const void *addr, size_t size)
> __kasan_unpoison_range(addr, size);
> }
>
> -void __kasan_alloc_pages(struct page *page, unsigned int order, bool init);
> -static __always_inline void kasan_alloc_pages(struct page *page,
> +void __kasan_poison_pages(struct page *page, unsigned int order, bool init);
> +static __always_inline void kasan_poison_pages(struct page *page,
> unsigned int order, bool init)
> {
> if (kasan_enabled())
> - __kasan_alloc_pages(page, order, init);
> + __kasan_poison_pages(page, order, init);
> }
>
> -void __kasan_free_pages(struct page *page, unsigned int order, bool init);
> -static __always_inline void kasan_free_pages(struct page *page,
> - unsigned int order, bool init)
> +void __kasan_unpoison_pages(struct page *page, unsigned int order, bool init);
> +static __always_inline void kasan_unpoison_pages(struct page *page,
> + unsigned int order, bool init)
> {
> if (kasan_enabled())
> - __kasan_free_pages(page, order, init);
> + __kasan_unpoison_pages(page, order, init);
> }
>
> void __kasan_cache_create(struct kmem_cache *cache, unsigned int *size,
> @@ -285,21 +303,15 @@ void kasan_restore_multi_shot(bool enabled);
>
> #else /* CONFIG_KASAN */
>
> -static inline bool kasan_enabled(void)
> -{
> - return false;
> -}
> -static inline bool kasan_has_integrated_init(void)
> -{
> - return false;
> -}
> static inline slab_flags_t kasan_never_merge(void)
> {
> return 0;
> }
> static inline void kasan_unpoison_range(const void *address, size_t size) {}
> -static inline void kasan_alloc_pages(struct page *page, unsigned int order, bool init) {}
> -static inline void kasan_free_pages(struct page *page, unsigned int order, bool init) {}
> +static inline void kasan_poison_pages(struct page *page, unsigned int order,
> + bool init) {}
> +static inline void kasan_unpoison_pages(struct page *page, unsigned int order,
> + bool init) {}
> static inline void kasan_cache_create(struct kmem_cache *cache,
> unsigned int *size,
> slab_flags_t *flags) {}
> diff --git a/mm/kasan/common.c b/mm/kasan/common.c
> index 6bb87f2acd4e..0ecd293af344 100644
> --- a/mm/kasan/common.c
> +++ b/mm/kasan/common.c
> @@ -97,7 +97,7 @@ slab_flags_t __kasan_never_merge(void)
> return 0;
> }
>
> -void __kasan_alloc_pages(struct page *page, unsigned int order, bool init)
> +void __kasan_unpoison_pages(struct page *page, unsigned int order, bool init)
> {
> u8 tag;
> unsigned long i;
> @@ -111,7 +111,7 @@ void __kasan_alloc_pages(struct page *page, unsigned int order, bool init)
> kasan_unpoison(page_address(page), PAGE_SIZE << order, init);
> }
>
> -void __kasan_free_pages(struct page *page, unsigned int order, bool init)
> +void __kasan_poison_pages(struct page *page, unsigned int order, bool init)
> {
> if (likely(!PageHighMem(page)))
> kasan_poison(page_address(page), PAGE_SIZE << order,
> diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
> index 4004388b4e4b..9d0f6f934016 100644
> --- a/mm/kasan/hw_tags.c
> +++ b/mm/kasan/hw_tags.c
> @@ -238,6 +238,28 @@ struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
> return &alloc_meta->free_track[0];
> }
>
> +void kasan_alloc_pages(struct page *page, unsigned int order, gfp_t flags)
> +{
> + /*
> + * This condition should match the one in post_alloc_hook() in
> + * page_alloc.c.
> + */
> + bool init = !want_init_on_free() && want_init_on_alloc(flags);
Now we have a comment here ...
> +
> + kasan_unpoison_pages(page, order, init);
> +}
> +
> +void kasan_free_pages(struct page *page, unsigned int order)
> +{
> + /*
> + * This condition should match the one in free_pages_prepare() in
> + * page_alloc.c.
> + */
> + bool init = want_init_on_free();
and here, ...
> +
> + kasan_poison_pages(page, order, init);
> +}
> +
> #if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
>
> void kasan_set_tagging_report_once(bool state)
> diff --git a/mm/mempool.c b/mm/mempool.c
> index a258cf4de575..0b8afbec3e35 100644
> --- a/mm/mempool.c
> +++ b/mm/mempool.c
> @@ -106,7 +106,8 @@ static __always_inline void kasan_poison_element(mempool_t *pool, void *element)
> if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc)
> kasan_slab_free_mempool(element);
> else if (pool->alloc == mempool_alloc_pages)
> - kasan_free_pages(element, (unsigned long)pool->pool_data, false);
> + kasan_poison_pages(element, (unsigned long)pool->pool_data,
> + false);
> }
>
> static void kasan_unpoison_element(mempool_t *pool, void *element)
> @@ -114,7 +115,8 @@ static void kasan_unpoison_element(mempool_t *pool, void *element)
> if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc)
> kasan_unpoison_range(element, __ksize(element));
> else if (pool->alloc == mempool_alloc_pages)
> - kasan_alloc_pages(element, (unsigned long)pool->pool_data, false);
> + kasan_unpoison_pages(element, (unsigned long)pool->pool_data,
> + false);
> }
>
> static __always_inline void add_element(mempool_t *pool, void *element)
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index aaa1655cf682..4fddb7cac3c6 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -382,7 +382,7 @@ int page_group_by_mobility_disabled __read_mostly;
> static DEFINE_STATIC_KEY_TRUE(deferred_pages);
>
> /*
> - * Calling kasan_free_pages() only after deferred memory initialization
> + * Calling kasan_poison_pages() only after deferred memory initialization
> * has completed. Poisoning pages during deferred memory init will greatly
> * lengthen the process and cause problem in large memory systems as the
> * deferred pages initialization is done with interrupt disabled.
> @@ -394,15 +394,11 @@ static DEFINE_STATIC_KEY_TRUE(deferred_pages);
> * on-demand allocation and then freed again before the deferred pages
> * initialization is done, but this is not likely to happen.
> */
> -static inline void kasan_free_nondeferred_pages(struct page *page, int order,
> - bool init, fpi_t fpi_flags)
> +static inline bool should_skip_kasan_poison(fpi_t fpi_flags)
> {
> - if (static_branch_unlikely(&deferred_pages))
> - return;
> - if (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
> - (fpi_flags & FPI_SKIP_KASAN_POISON))
> - return;
> - kasan_free_pages(page, order, init);
> + return static_branch_unlikely(&deferred_pages) ||
> + (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
> + (fpi_flags & FPI_SKIP_KASAN_POISON));
> }
>
> /* Returns true if the struct page for the pfn is uninitialised */
> @@ -453,13 +449,10 @@ defer_init(int nid, unsigned long pfn, unsigned long end_pfn)
> return false;
> }
> #else
> -static inline void kasan_free_nondeferred_pages(struct page *page, int order,
> - bool init, fpi_t fpi_flags)
> +static inline bool should_skip_kasan_poison(fpi_t fpi_flags)
> {
> - if (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
> - (fpi_flags & FPI_SKIP_KASAN_POISON))
> - return;
> - kasan_free_pages(page, order, init);
> + return (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
> + (fpi_flags & FPI_SKIP_KASAN_POISON));
> }
>
> static inline bool early_page_uninitialised(unsigned long pfn)
> @@ -1245,7 +1238,7 @@ static __always_inline bool free_pages_prepare(struct page *page,
> unsigned int order, bool check_free, fpi_t fpi_flags)
> {
> int bad = 0;
> - bool init;
> + bool skip_kasan_poison = should_skip_kasan_poison(fpi_flags);
>
> VM_BUG_ON_PAGE(PageTail(page), page);
>
> @@ -1314,10 +1307,17 @@ static __always_inline bool free_pages_prepare(struct page *page,
> * With hardware tag-based KASAN, memory tags must be set before the
> * page becomes unavailable via debug_pagealloc or arch_free_page.
> */
> - init = want_init_on_free();
> - if (init && !kasan_has_integrated_init())
> - kernel_init_free_pages(page, 1 << order);
> - kasan_free_nondeferred_pages(page, order, init, fpi_flags);
> + if (kasan_has_integrated_init()) {
> + if (!skip_kasan_poison)
> + kasan_free_pages(page, order);
> + } else {
> + bool init = want_init_on_free();
... but not here ...
> +
> + if (init)
> + kernel_init_free_pages(page, 1 << order);
> + if (!skip_kasan_poison)
> + kasan_poison_pages(page, order, init);
> + }
>
> /*
> * arch_free_page() can make the page's contents inaccessible. s390
> @@ -2324,8 +2324,6 @@ static bool check_new_pages(struct page *page, unsigned int order)
> inline void post_alloc_hook(struct page *page, unsigned int order,
> gfp_t gfp_flags)
> {
> - bool init;
> -
> set_page_private(page, 0);
> set_page_refcounted(page);
>
> @@ -2344,10 +2342,15 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
> * kasan_alloc_pages and kernel_init_free_pages must be
> * kept together to avoid discrepancies in behavior.
> */
> - init = !want_init_on_free() && want_init_on_alloc(gfp_flags);
> - kasan_alloc_pages(page, order, init);
> - if (init && !kasan_has_integrated_init())
> - kernel_init_free_pages(page, 1 << order);
> + if (kasan_has_integrated_init()) {
> + kasan_alloc_pages(page, order, gfp_flags);
> + } else {
> + bool init = !want_init_on_free() && want_init_on_alloc(gfp_flags);
... or here.
So if someone updates one of these conditions, they might forget the
ones in KASAN code.
Is there a strong reason not to use a macro or static inline helper?
If not, let's do that.
> +
> + kasan_unpoison_pages(page, order, init);
> + if (init)
> + kernel_init_free_pages(page, 1 << order);
> + }
>
> set_page_owner(page, order, gfp_flags);
> }
> --
> 2.32.0.rc0.204.g9fa02ecfa5-goog
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v4 2/4] kasan: use separate (un)poison implementation for integrated init
2021-05-28 10:25 ` Andrey Konovalov
@ 2021-06-01 19:28 ` Peter Collingbourne
2021-06-02 12:19 ` Andrey Konovalov
0 siblings, 1 reply; 9+ messages in thread
From: Peter Collingbourne @ 2021-06-01 19:28 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Alexander Potapenko, Catalin Marinas, Vincenzo Frascino,
Andrew Morton, Jann Horn, Evgenii Stepanov,
Linux Memory Management List, Linux ARM
On Fri, May 28, 2021 at 3:25 AM Andrey Konovalov <andreyknvl@gmail.com> wrote:
>
> On Fri, May 28, 2021 at 4:04 AM Peter Collingbourne <pcc@google.com> wrote:
> >
> > Currently with integrated init page_alloc.c needs to know whether
> > kasan_alloc_pages() will zero initialize memory, but this will start
> > becoming more complicated once we start adding tag initialization
> > support for user pages. To avoid page_alloc.c needing to know more
> > details of what integrated init will do, move the unpoisoning logic
> > for integrated init into the HW tags implementation. Currently the
> > logic is identical but it will diverge in subsequent patches.
> >
> > For symmetry do the same for poisoning although this logic will
> > be unaffected by subsequent patches.
> >
> > Signed-off-by: Peter Collingbourne <pcc@google.com>
> > Link: https://linux-review.googlesource.com/id/I2c550234c6c4a893c48c18ff0c6ce658c7c67056
> > ---
> > v4:
> > - use IS_ENABLED(CONFIG_KASAN)
> > - add comments to kasan_alloc_pages and kasan_free_pages
> > - remove line break
> >
> > v3:
> > - use BUILD_BUG()
> >
> > v2:
> > - fix build with KASAN disabled
> >
> > include/linux/kasan.h | 64 +++++++++++++++++++++++++------------------
> > mm/kasan/common.c | 4 +--
> > mm/kasan/hw_tags.c | 22 +++++++++++++++
> > mm/mempool.c | 6 ++--
> > mm/page_alloc.c | 55 +++++++++++++++++++------------------
> > 5 files changed, 95 insertions(+), 56 deletions(-)
> >
> > diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> > index b1678a61e6a7..a1c7ce5f3e4f 100644
> > --- a/include/linux/kasan.h
> > +++ b/include/linux/kasan.h
> > @@ -2,6 +2,7 @@
> > #ifndef _LINUX_KASAN_H
> > #define _LINUX_KASAN_H
> >
> > +#include <linux/bug.h>
> > #include <linux/static_key.h>
> > #include <linux/types.h>
> >
> > @@ -79,14 +80,6 @@ static inline void kasan_disable_current(void) {}
> >
> > #endif /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */
> >
> > -#ifdef CONFIG_KASAN
> > -
> > -struct kasan_cache {
> > - int alloc_meta_offset;
> > - int free_meta_offset;
> > - bool is_kmalloc;
> > -};
> > -
> > #ifdef CONFIG_KASAN_HW_TAGS
> >
> > DECLARE_STATIC_KEY_FALSE(kasan_flag_enabled);
> > @@ -101,11 +94,14 @@ static inline bool kasan_has_integrated_init(void)
> > return kasan_enabled();
> > }
> >
> > +void kasan_alloc_pages(struct page *page, unsigned int order, gfp_t flags);
> > +void kasan_free_pages(struct page *page, unsigned int order);
> > +
> > #else /* CONFIG_KASAN_HW_TAGS */
> >
> > static inline bool kasan_enabled(void)
> > {
> > - return true;
> > + return IS_ENABLED(CONFIG_KASAN);
> > }
> >
> > static inline bool kasan_has_integrated_init(void)
> > @@ -113,8 +109,30 @@ static inline bool kasan_has_integrated_init(void)
> > return false;
> > }
> >
> > +static __always_inline void kasan_alloc_pages(struct page *page,
> > + unsigned int order, gfp_t flags)
> > +{
> > + /* Only available for integrated init. */
> > + BUILD_BUG();
> > +}
> > +
> > +static __always_inline void kasan_free_pages(struct page *page,
> > + unsigned int order)
> > +{
> > + /* Only available for integrated init. */
> > + BUILD_BUG();
> > +}
> > +
> > #endif /* CONFIG_KASAN_HW_TAGS */
> >
> > +#ifdef CONFIG_KASAN
> > +
> > +struct kasan_cache {
> > + int alloc_meta_offset;
> > + int free_meta_offset;
> > + bool is_kmalloc;
> > +};
> > +
> > slab_flags_t __kasan_never_merge(void);
> > static __always_inline slab_flags_t kasan_never_merge(void)
> > {
> > @@ -130,20 +148,20 @@ static __always_inline void kasan_unpoison_range(const void *addr, size_t size)
> > __kasan_unpoison_range(addr, size);
> > }
> >
> > -void __kasan_alloc_pages(struct page *page, unsigned int order, bool init);
> > -static __always_inline void kasan_alloc_pages(struct page *page,
> > +void __kasan_poison_pages(struct page *page, unsigned int order, bool init);
> > +static __always_inline void kasan_poison_pages(struct page *page,
> > unsigned int order, bool init)
> > {
> > if (kasan_enabled())
> > - __kasan_alloc_pages(page, order, init);
> > + __kasan_poison_pages(page, order, init);
> > }
> >
> > -void __kasan_free_pages(struct page *page, unsigned int order, bool init);
> > -static __always_inline void kasan_free_pages(struct page *page,
> > - unsigned int order, bool init)
> > +void __kasan_unpoison_pages(struct page *page, unsigned int order, bool init);
> > +static __always_inline void kasan_unpoison_pages(struct page *page,
> > + unsigned int order, bool init)
> > {
> > if (kasan_enabled())
> > - __kasan_free_pages(page, order, init);
> > + __kasan_unpoison_pages(page, order, init);
> > }
> >
> > void __kasan_cache_create(struct kmem_cache *cache, unsigned int *size,
> > @@ -285,21 +303,15 @@ void kasan_restore_multi_shot(bool enabled);
> >
> > #else /* CONFIG_KASAN */
> >
> > -static inline bool kasan_enabled(void)
> > -{
> > - return false;
> > -}
> > -static inline bool kasan_has_integrated_init(void)
> > -{
> > - return false;
> > -}
> > static inline slab_flags_t kasan_never_merge(void)
> > {
> > return 0;
> > }
> > static inline void kasan_unpoison_range(const void *address, size_t size) {}
> > -static inline void kasan_alloc_pages(struct page *page, unsigned int order, bool init) {}
> > -static inline void kasan_free_pages(struct page *page, unsigned int order, bool init) {}
> > +static inline void kasan_poison_pages(struct page *page, unsigned int order,
> > + bool init) {}
> > +static inline void kasan_unpoison_pages(struct page *page, unsigned int order,
> > + bool init) {}
> > static inline void kasan_cache_create(struct kmem_cache *cache,
> > unsigned int *size,
> > slab_flags_t *flags) {}
> > diff --git a/mm/kasan/common.c b/mm/kasan/common.c
> > index 6bb87f2acd4e..0ecd293af344 100644
> > --- a/mm/kasan/common.c
> > +++ b/mm/kasan/common.c
> > @@ -97,7 +97,7 @@ slab_flags_t __kasan_never_merge(void)
> > return 0;
> > }
> >
> > -void __kasan_alloc_pages(struct page *page, unsigned int order, bool init)
> > +void __kasan_unpoison_pages(struct page *page, unsigned int order, bool init)
> > {
> > u8 tag;
> > unsigned long i;
> > @@ -111,7 +111,7 @@ void __kasan_alloc_pages(struct page *page, unsigned int order, bool init)
> > kasan_unpoison(page_address(page), PAGE_SIZE << order, init);
> > }
> >
> > -void __kasan_free_pages(struct page *page, unsigned int order, bool init)
> > +void __kasan_poison_pages(struct page *page, unsigned int order, bool init)
> > {
> > if (likely(!PageHighMem(page)))
> > kasan_poison(page_address(page), PAGE_SIZE << order,
> > diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
> > index 4004388b4e4b..9d0f6f934016 100644
> > --- a/mm/kasan/hw_tags.c
> > +++ b/mm/kasan/hw_tags.c
> > @@ -238,6 +238,28 @@ struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
> > return &alloc_meta->free_track[0];
> > }
> >
> > +void kasan_alloc_pages(struct page *page, unsigned int order, gfp_t flags)
> > +{
> > + /*
> > + * This condition should match the one in post_alloc_hook() in
> > + * page_alloc.c.
> > + */
> > + bool init = !want_init_on_free() && want_init_on_alloc(flags);
>
> Now we have a comment here ...
>
> > +
> > + kasan_unpoison_pages(page, order, init);
> > +}
> > +
> > +void kasan_free_pages(struct page *page, unsigned int order)
> > +{
> > + /*
> > + * This condition should match the one in free_pages_prepare() in
> > + * page_alloc.c.
> > + */
> > + bool init = want_init_on_free();
>
> and here, ...
>
> > +
> > + kasan_poison_pages(page, order, init);
> > +}
> > +
> > #if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
> >
> > void kasan_set_tagging_report_once(bool state)
> > diff --git a/mm/mempool.c b/mm/mempool.c
> > index a258cf4de575..0b8afbec3e35 100644
> > --- a/mm/mempool.c
> > +++ b/mm/mempool.c
> > @@ -106,7 +106,8 @@ static __always_inline void kasan_poison_element(mempool_t *pool, void *element)
> > if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc)
> > kasan_slab_free_mempool(element);
> > else if (pool->alloc == mempool_alloc_pages)
> > - kasan_free_pages(element, (unsigned long)pool->pool_data, false);
> > + kasan_poison_pages(element, (unsigned long)pool->pool_data,
> > + false);
> > }
> >
> > static void kasan_unpoison_element(mempool_t *pool, void *element)
> > @@ -114,7 +115,8 @@ static void kasan_unpoison_element(mempool_t *pool, void *element)
> > if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc)
> > kasan_unpoison_range(element, __ksize(element));
> > else if (pool->alloc == mempool_alloc_pages)
> > - kasan_alloc_pages(element, (unsigned long)pool->pool_data, false);
> > + kasan_unpoison_pages(element, (unsigned long)pool->pool_data,
> > + false);
> > }
> >
> > static __always_inline void add_element(mempool_t *pool, void *element)
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index aaa1655cf682..4fddb7cac3c6 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -382,7 +382,7 @@ int page_group_by_mobility_disabled __read_mostly;
> > static DEFINE_STATIC_KEY_TRUE(deferred_pages);
> >
> > /*
> > - * Calling kasan_free_pages() only after deferred memory initialization
> > + * Calling kasan_poison_pages() only after deferred memory initialization
> > * has completed. Poisoning pages during deferred memory init will greatly
> > * lengthen the process and cause problem in large memory systems as the
> > * deferred pages initialization is done with interrupt disabled.
> > @@ -394,15 +394,11 @@ static DEFINE_STATIC_KEY_TRUE(deferred_pages);
> > * on-demand allocation and then freed again before the deferred pages
> > * initialization is done, but this is not likely to happen.
> > */
> > -static inline void kasan_free_nondeferred_pages(struct page *page, int order,
> > - bool init, fpi_t fpi_flags)
> > +static inline bool should_skip_kasan_poison(fpi_t fpi_flags)
> > {
> > - if (static_branch_unlikely(&deferred_pages))
> > - return;
> > - if (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
> > - (fpi_flags & FPI_SKIP_KASAN_POISON))
> > - return;
> > - kasan_free_pages(page, order, init);
> > + return static_branch_unlikely(&deferred_pages) ||
> > + (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
> > + (fpi_flags & FPI_SKIP_KASAN_POISON));
> > }
> >
> > /* Returns true if the struct page for the pfn is uninitialised */
> > @@ -453,13 +449,10 @@ defer_init(int nid, unsigned long pfn, unsigned long end_pfn)
> > return false;
> > }
> > #else
> > -static inline void kasan_free_nondeferred_pages(struct page *page, int order,
> > - bool init, fpi_t fpi_flags)
> > +static inline bool should_skip_kasan_poison(fpi_t fpi_flags)
> > {
> > - if (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
> > - (fpi_flags & FPI_SKIP_KASAN_POISON))
> > - return;
> > - kasan_free_pages(page, order, init);
> > + return (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
> > + (fpi_flags & FPI_SKIP_KASAN_POISON));
> > }
> >
> > static inline bool early_page_uninitialised(unsigned long pfn)
> > @@ -1245,7 +1238,7 @@ static __always_inline bool free_pages_prepare(struct page *page,
> > unsigned int order, bool check_free, fpi_t fpi_flags)
> > {
> > int bad = 0;
> > - bool init;
> > + bool skip_kasan_poison = should_skip_kasan_poison(fpi_flags);
> >
> > VM_BUG_ON_PAGE(PageTail(page), page);
> >
> > @@ -1314,10 +1307,17 @@ static __always_inline bool free_pages_prepare(struct page *page,
> > * With hardware tag-based KASAN, memory tags must be set before the
> > * page becomes unavailable via debug_pagealloc or arch_free_page.
> > */
> > - init = want_init_on_free();
> > - if (init && !kasan_has_integrated_init())
> > - kernel_init_free_pages(page, 1 << order);
> > - kasan_free_nondeferred_pages(page, order, init, fpi_flags);
> > + if (kasan_has_integrated_init()) {
> > + if (!skip_kasan_poison)
> > + kasan_free_pages(page, order);
> > + } else {
> > + bool init = want_init_on_free();
>
> ... but not here ...
>
> > +
> > + if (init)
> > + kernel_init_free_pages(page, 1 << order);
> > + if (!skip_kasan_poison)
> > + kasan_poison_pages(page, order, init);
> > + }
> >
> > /*
> > * arch_free_page() can make the page's contents inaccessible. s390
> > @@ -2324,8 +2324,6 @@ static bool check_new_pages(struct page *page, unsigned int order)
> > inline void post_alloc_hook(struct page *page, unsigned int order,
> > gfp_t gfp_flags)
> > {
> > - bool init;
> > -
> > set_page_private(page, 0);
> > set_page_refcounted(page);
> >
> > @@ -2344,10 +2342,15 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
> > * kasan_alloc_pages and kernel_init_free_pages must be
> > * kept together to avoid discrepancies in behavior.
> > */
> > - init = !want_init_on_free() && want_init_on_alloc(gfp_flags);
> > - kasan_alloc_pages(page, order, init);
> > - if (init && !kasan_has_integrated_init())
> > - kernel_init_free_pages(page, 1 << order);
> > + if (kasan_has_integrated_init()) {
> > + kasan_alloc_pages(page, order, gfp_flags);
> > + } else {
> > + bool init = !want_init_on_free() && want_init_on_alloc(gfp_flags);
>
> ... or here.
>
> So if someone updates one of these conditions, they might forget the
> ones in KASAN code.
>
> Is there a strong reason not to use a macro or static inline helper?
> If not, let's do that.
I'm not sure that it will accomplish much. It isn't much code after
all and it means that we are adding another level of indirection which
readers will need to look through in order to understand what is going
on.
We already have this comment in free_pages_prepare:
/*
* As memory initialization might be integrated into KASAN,
* kasan_free_pages and kernel_init_free_pages must be
* kept together to avoid discrepancies in behavior.
*
* With hardware tag-based KASAN, memory tags must be set before the
* page becomes unavailable via debug_pagealloc or arch_free_page.
*/
and this one in post_alloc_hook:
/*
* As memory initialization might be integrated into KASAN,
* kasan_alloc_pages and kernel_init_free_pages must be
* kept together to avoid discrepancies in behavior.
*/
Is that not enough?
Peter
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v4 2/4] kasan: use separate (un)poison implementation for integrated init
2021-06-01 19:28 ` Peter Collingbourne
@ 2021-06-02 12:19 ` Andrey Konovalov
0 siblings, 0 replies; 9+ messages in thread
From: Andrey Konovalov @ 2021-06-02 12:19 UTC (permalink / raw)
To: Peter Collingbourne
Cc: Alexander Potapenko, Catalin Marinas, Vincenzo Frascino,
Andrew Morton, Jann Horn, Evgenii Stepanov,
Linux Memory Management List, Linux ARM
On Tue, Jun 1, 2021 at 10:29 PM Peter Collingbourne <pcc@google.com> wrote:
>
> On Fri, May 28, 2021 at 3:25 AM Andrey Konovalov <andreyknvl@gmail.com> wrote:
> >
> > On Fri, May 28, 2021 at 4:04 AM Peter Collingbourne <pcc@google.com> wrote:
> > >
> > > Currently with integrated init page_alloc.c needs to know whether
> > > kasan_alloc_pages() will zero initialize memory, but this will start
> > > becoming more complicated once we start adding tag initialization
> > > support for user pages. To avoid page_alloc.c needing to know more
> > > details of what integrated init will do, move the unpoisoning logic
> > > for integrated init into the HW tags implementation. Currently the
> > > logic is identical but it will diverge in subsequent patches.
> > >
> > > For symmetry do the same for poisoning although this logic will
> > > be unaffected by subsequent patches.
> > >
> > > Signed-off-by: Peter Collingbourne <pcc@google.com>
> > > Link: https://linux-review.googlesource.com/id/I2c550234c6c4a893c48c18ff0c6ce658c7c67056
> > > ---
> > > v4:
> > > - use IS_ENABLED(CONFIG_KASAN)
> > > - add comments to kasan_alloc_pages and kasan_free_pages
> > > - remove line break
> > >
> > > v3:
> > > - use BUILD_BUG()
> > >
> > > v2:
> > > - fix build with KASAN disabled
> > >
> > > include/linux/kasan.h | 64 +++++++++++++++++++++++++------------------
> > > mm/kasan/common.c | 4 +--
> > > mm/kasan/hw_tags.c | 22 +++++++++++++++
> > > mm/mempool.c | 6 ++--
> > > mm/page_alloc.c | 55 +++++++++++++++++++------------------
> > > 5 files changed, 95 insertions(+), 56 deletions(-)
> > >
> > > diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> > > index b1678a61e6a7..a1c7ce5f3e4f 100644
> > > --- a/include/linux/kasan.h
> > > +++ b/include/linux/kasan.h
> > > @@ -2,6 +2,7 @@
> > > #ifndef _LINUX_KASAN_H
> > > #define _LINUX_KASAN_H
> > >
> > > +#include <linux/bug.h>
> > > #include <linux/static_key.h>
> > > #include <linux/types.h>
> > >
> > > @@ -79,14 +80,6 @@ static inline void kasan_disable_current(void) {}
> > >
> > > #endif /* CONFIG_KASAN_GENERIC || CONFIG_KASAN_SW_TAGS */
> > >
> > > -#ifdef CONFIG_KASAN
> > > -
> > > -struct kasan_cache {
> > > - int alloc_meta_offset;
> > > - int free_meta_offset;
> > > - bool is_kmalloc;
> > > -};
> > > -
> > > #ifdef CONFIG_KASAN_HW_TAGS
> > >
> > > DECLARE_STATIC_KEY_FALSE(kasan_flag_enabled);
> > > @@ -101,11 +94,14 @@ static inline bool kasan_has_integrated_init(void)
> > > return kasan_enabled();
> > > }
> > >
> > > +void kasan_alloc_pages(struct page *page, unsigned int order, gfp_t flags);
> > > +void kasan_free_pages(struct page *page, unsigned int order);
> > > +
> > > #else /* CONFIG_KASAN_HW_TAGS */
> > >
> > > static inline bool kasan_enabled(void)
> > > {
> > > - return true;
> > > + return IS_ENABLED(CONFIG_KASAN);
> > > }
> > >
> > > static inline bool kasan_has_integrated_init(void)
> > > @@ -113,8 +109,30 @@ static inline bool kasan_has_integrated_init(void)
> > > return false;
> > > }
> > >
> > > +static __always_inline void kasan_alloc_pages(struct page *page,
> > > + unsigned int order, gfp_t flags)
> > > +{
> > > + /* Only available for integrated init. */
> > > + BUILD_BUG();
> > > +}
> > > +
> > > +static __always_inline void kasan_free_pages(struct page *page,
> > > + unsigned int order)
> > > +{
> > > + /* Only available for integrated init. */
> > > + BUILD_BUG();
> > > +}
> > > +
> > > #endif /* CONFIG_KASAN_HW_TAGS */
> > >
> > > +#ifdef CONFIG_KASAN
> > > +
> > > +struct kasan_cache {
> > > + int alloc_meta_offset;
> > > + int free_meta_offset;
> > > + bool is_kmalloc;
> > > +};
> > > +
> > > slab_flags_t __kasan_never_merge(void);
> > > static __always_inline slab_flags_t kasan_never_merge(void)
> > > {
> > > @@ -130,20 +148,20 @@ static __always_inline void kasan_unpoison_range(const void *addr, size_t size)
> > > __kasan_unpoison_range(addr, size);
> > > }
> > >
> > > -void __kasan_alloc_pages(struct page *page, unsigned int order, bool init);
> > > -static __always_inline void kasan_alloc_pages(struct page *page,
> > > +void __kasan_poison_pages(struct page *page, unsigned int order, bool init);
> > > +static __always_inline void kasan_poison_pages(struct page *page,
> > > unsigned int order, bool init)
> > > {
> > > if (kasan_enabled())
> > > - __kasan_alloc_pages(page, order, init);
> > > + __kasan_poison_pages(page, order, init);
> > > }
> > >
> > > -void __kasan_free_pages(struct page *page, unsigned int order, bool init);
> > > -static __always_inline void kasan_free_pages(struct page *page,
> > > - unsigned int order, bool init)
> > > +void __kasan_unpoison_pages(struct page *page, unsigned int order, bool init);
> > > +static __always_inline void kasan_unpoison_pages(struct page *page,
> > > + unsigned int order, bool init)
> > > {
> > > if (kasan_enabled())
> > > - __kasan_free_pages(page, order, init);
> > > + __kasan_unpoison_pages(page, order, init);
> > > }
> > >
> > > void __kasan_cache_create(struct kmem_cache *cache, unsigned int *size,
> > > @@ -285,21 +303,15 @@ void kasan_restore_multi_shot(bool enabled);
> > >
> > > #else /* CONFIG_KASAN */
> > >
> > > -static inline bool kasan_enabled(void)
> > > -{
> > > - return false;
> > > -}
> > > -static inline bool kasan_has_integrated_init(void)
> > > -{
> > > - return false;
> > > -}
> > > static inline slab_flags_t kasan_never_merge(void)
> > > {
> > > return 0;
> > > }
> > > static inline void kasan_unpoison_range(const void *address, size_t size) {}
> > > -static inline void kasan_alloc_pages(struct page *page, unsigned int order, bool init) {}
> > > -static inline void kasan_free_pages(struct page *page, unsigned int order, bool init) {}
> > > +static inline void kasan_poison_pages(struct page *page, unsigned int order,
> > > + bool init) {}
> > > +static inline void kasan_unpoison_pages(struct page *page, unsigned int order,
> > > + bool init) {}
> > > static inline void kasan_cache_create(struct kmem_cache *cache,
> > > unsigned int *size,
> > > slab_flags_t *flags) {}
> > > diff --git a/mm/kasan/common.c b/mm/kasan/common.c
> > > index 6bb87f2acd4e..0ecd293af344 100644
> > > --- a/mm/kasan/common.c
> > > +++ b/mm/kasan/common.c
> > > @@ -97,7 +97,7 @@ slab_flags_t __kasan_never_merge(void)
> > > return 0;
> > > }
> > >
> > > -void __kasan_alloc_pages(struct page *page, unsigned int order, bool init)
> > > +void __kasan_unpoison_pages(struct page *page, unsigned int order, bool init)
> > > {
> > > u8 tag;
> > > unsigned long i;
> > > @@ -111,7 +111,7 @@ void __kasan_alloc_pages(struct page *page, unsigned int order, bool init)
> > > kasan_unpoison(page_address(page), PAGE_SIZE << order, init);
> > > }
> > >
> > > -void __kasan_free_pages(struct page *page, unsigned int order, bool init)
> > > +void __kasan_poison_pages(struct page *page, unsigned int order, bool init)
> > > {
> > > if (likely(!PageHighMem(page)))
> > > kasan_poison(page_address(page), PAGE_SIZE << order,
> > > diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
> > > index 4004388b4e4b..9d0f6f934016 100644
> > > --- a/mm/kasan/hw_tags.c
> > > +++ b/mm/kasan/hw_tags.c
> > > @@ -238,6 +238,28 @@ struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
> > > return &alloc_meta->free_track[0];
> > > }
> > >
> > > +void kasan_alloc_pages(struct page *page, unsigned int order, gfp_t flags)
> > > +{
> > > + /*
> > > + * This condition should match the one in post_alloc_hook() in
> > > + * page_alloc.c.
> > > + */
> > > + bool init = !want_init_on_free() && want_init_on_alloc(flags);
> >
> > Now we have a comment here ...
> >
> > > +
> > > + kasan_unpoison_pages(page, order, init);
> > > +}
> > > +
> > > +void kasan_free_pages(struct page *page, unsigned int order)
> > > +{
> > > + /*
> > > + * This condition should match the one in free_pages_prepare() in
> > > + * page_alloc.c.
> > > + */
> > > + bool init = want_init_on_free();
> >
> > and here, ...
> >
> > > +
> > > + kasan_poison_pages(page, order, init);
> > > +}
> > > +
> > > #if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
> > >
> > > void kasan_set_tagging_report_once(bool state)
> > > diff --git a/mm/mempool.c b/mm/mempool.c
> > > index a258cf4de575..0b8afbec3e35 100644
> > > --- a/mm/mempool.c
> > > +++ b/mm/mempool.c
> > > @@ -106,7 +106,8 @@ static __always_inline void kasan_poison_element(mempool_t *pool, void *element)
> > > if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc)
> > > kasan_slab_free_mempool(element);
> > > else if (pool->alloc == mempool_alloc_pages)
> > > - kasan_free_pages(element, (unsigned long)pool->pool_data, false);
> > > + kasan_poison_pages(element, (unsigned long)pool->pool_data,
> > > + false);
> > > }
> > >
> > > static void kasan_unpoison_element(mempool_t *pool, void *element)
> > > @@ -114,7 +115,8 @@ static void kasan_unpoison_element(mempool_t *pool, void *element)
> > > if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc)
> > > kasan_unpoison_range(element, __ksize(element));
> > > else if (pool->alloc == mempool_alloc_pages)
> > > - kasan_alloc_pages(element, (unsigned long)pool->pool_data, false);
> > > + kasan_unpoison_pages(element, (unsigned long)pool->pool_data,
> > > + false);
> > > }
> > >
> > > static __always_inline void add_element(mempool_t *pool, void *element)
> > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > > index aaa1655cf682..4fddb7cac3c6 100644
> > > --- a/mm/page_alloc.c
> > > +++ b/mm/page_alloc.c
> > > @@ -382,7 +382,7 @@ int page_group_by_mobility_disabled __read_mostly;
> > > static DEFINE_STATIC_KEY_TRUE(deferred_pages);
> > >
> > > /*
> > > - * Calling kasan_free_pages() only after deferred memory initialization
> > > + * Calling kasan_poison_pages() only after deferred memory initialization
> > > * has completed. Poisoning pages during deferred memory init will greatly
> > > * lengthen the process and cause problem in large memory systems as the
> > > * deferred pages initialization is done with interrupt disabled.
> > > @@ -394,15 +394,11 @@ static DEFINE_STATIC_KEY_TRUE(deferred_pages);
> > > * on-demand allocation and then freed again before the deferred pages
> > > * initialization is done, but this is not likely to happen.
> > > */
> > > -static inline void kasan_free_nondeferred_pages(struct page *page, int order,
> > > - bool init, fpi_t fpi_flags)
> > > +static inline bool should_skip_kasan_poison(fpi_t fpi_flags)
> > > {
> > > - if (static_branch_unlikely(&deferred_pages))
> > > - return;
> > > - if (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
> > > - (fpi_flags & FPI_SKIP_KASAN_POISON))
> > > - return;
> > > - kasan_free_pages(page, order, init);
> > > + return static_branch_unlikely(&deferred_pages) ||
> > > + (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
> > > + (fpi_flags & FPI_SKIP_KASAN_POISON));
> > > }
> > >
> > > /* Returns true if the struct page for the pfn is uninitialised */
> > > @@ -453,13 +449,10 @@ defer_init(int nid, unsigned long pfn, unsigned long end_pfn)
> > > return false;
> > > }
> > > #else
> > > -static inline void kasan_free_nondeferred_pages(struct page *page, int order,
> > > - bool init, fpi_t fpi_flags)
> > > +static inline bool should_skip_kasan_poison(fpi_t fpi_flags)
> > > {
> > > - if (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
> > > - (fpi_flags & FPI_SKIP_KASAN_POISON))
> > > - return;
> > > - kasan_free_pages(page, order, init);
> > > + return (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
> > > + (fpi_flags & FPI_SKIP_KASAN_POISON));
> > > }
> > >
> > > static inline bool early_page_uninitialised(unsigned long pfn)
> > > @@ -1245,7 +1238,7 @@ static __always_inline bool free_pages_prepare(struct page *page,
> > > unsigned int order, bool check_free, fpi_t fpi_flags)
> > > {
> > > int bad = 0;
> > > - bool init;
> > > + bool skip_kasan_poison = should_skip_kasan_poison(fpi_flags);
> > >
> > > VM_BUG_ON_PAGE(PageTail(page), page);
> > >
> > > @@ -1314,10 +1307,17 @@ static __always_inline bool free_pages_prepare(struct page *page,
> > > * With hardware tag-based KASAN, memory tags must be set before the
> > > * page becomes unavailable via debug_pagealloc or arch_free_page.
> > > */
> > > - init = want_init_on_free();
> > > - if (init && !kasan_has_integrated_init())
> > > - kernel_init_free_pages(page, 1 << order);
> > > - kasan_free_nondeferred_pages(page, order, init, fpi_flags);
> > > + if (kasan_has_integrated_init()) {
> > > + if (!skip_kasan_poison)
> > > + kasan_free_pages(page, order);
> > > + } else {
> > > + bool init = want_init_on_free();
> >
> > ... but not here ...
> >
> > > +
> > > + if (init)
> > > + kernel_init_free_pages(page, 1 << order);
> > > + if (!skip_kasan_poison)
> > > + kasan_poison_pages(page, order, init);
> > > + }
> > >
> > > /*
> > > * arch_free_page() can make the page's contents inaccessible. s390
> > > @@ -2324,8 +2324,6 @@ static bool check_new_pages(struct page *page, unsigned int order)
> > > inline void post_alloc_hook(struct page *page, unsigned int order,
> > > gfp_t gfp_flags)
> > > {
> > > - bool init;
> > > -
> > > set_page_private(page, 0);
> > > set_page_refcounted(page);
> > >
> > > @@ -2344,10 +2342,15 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
> > > * kasan_alloc_pages and kernel_init_free_pages must be
> > > * kept together to avoid discrepancies in behavior.
> > > */
> > > - init = !want_init_on_free() && want_init_on_alloc(gfp_flags);
> > > - kasan_alloc_pages(page, order, init);
> > > - if (init && !kasan_has_integrated_init())
> > > - kernel_init_free_pages(page, 1 << order);
> > > + if (kasan_has_integrated_init()) {
> > > + kasan_alloc_pages(page, order, gfp_flags);
> > > + } else {
> > > + bool init = !want_init_on_free() && want_init_on_alloc(gfp_flags);
> >
> > ... or here.
> >
> > So if someone updates one of these conditions, they might forget the
> > ones in KASAN code.
> >
> > Is there a strong reason not to use a macro or static inline helper?
> > If not, let's do that.
>
> I'm not sure that it will accomplish much. It isn't much code after
> all and it means that we are adding another level of indirection which
> readers will need to look through in order to understand what is going
> on.
>
> We already have this comment in free_pages_prepare:
>
> /*
> * As memory initialization might be integrated into KASAN,
> * kasan_free_pages and kernel_init_free_pages must be
> * kept together to avoid discrepancies in behavior.
> *
> * With hardware tag-based KASAN, memory tags must be set before the
> * page becomes unavailable via debug_pagealloc or arch_free_page.
> */
>
> and this one in post_alloc_hook:
>
> /*
> * As memory initialization might be integrated into KASAN,
> * kasan_alloc_pages and kernel_init_free_pages must be
> * kept together to avoid discrepancies in behavior.
> */
>
> Is that not enough?
Ah, forgot about those two. Alright, let's keep this version with comments then.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 3/4] arm64: mte: handle tags zeroing at page allocation time
2021-05-28 1:04 [PATCH v4 0/4] arm64: improve efficiency of setting tags for user pages Peter Collingbourne
2021-05-28 1:04 ` [PATCH v4 1/4] mm: arch: remove indirection level in alloc_zeroed_user_highpage_movable() Peter Collingbourne
2021-05-28 1:04 ` [PATCH v4 2/4] kasan: use separate (un)poison implementation for integrated init Peter Collingbourne
@ 2021-05-28 1:04 ` Peter Collingbourne
2021-05-28 1:04 ` [PATCH v4 4/4] kasan: disable freed user page poisoning with HW tags Peter Collingbourne
3 siblings, 0 replies; 9+ messages in thread
From: Peter Collingbourne @ 2021-05-28 1:04 UTC (permalink / raw)
To: Andrey Konovalov, Alexander Potapenko, Catalin Marinas,
Vincenzo Frascino, Andrew Morton, Jann Horn
Cc: Peter Collingbourne, Evgenii Stepanov, linux-mm, linux-arm-kernel
Currently, on an anonymous page fault, the kernel allocates a zeroed
page and maps it in user space. If the mapping is tagged (PROT_MTE),
set_pte_at() additionally clears the tags. It is, however, more
efficient to clear the tags at the same time as zeroing the data on
allocation. To avoid clearing the tags on any page (which may not be
mapped as tagged), only do this if the vma flags contain VM_MTE. This
requires introducing a new GFP flag that is used to determine whether
to clear the tags.
The DC GZVA instruction with a 0 top byte (and 0 tag) requires
top-byte-ignore. Set the TCR_EL1.{TBI1,TBID1} bits irrespective of
whether KASAN_HW is enabled.
Signed-off-by: Peter Collingbourne <pcc@google.com>
Co-developed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Link: https://linux-review.googlesource.com/id/Id46dc94e30fe11474f7e54f5d65e7658dbdddb26
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
---
v2:
- remove want_zero_tags_on_free()
arch/arm64/include/asm/mte.h | 4 ++++
arch/arm64/include/asm/page.h | 8 ++++++--
arch/arm64/lib/mte.S | 20 ++++++++++++++++++++
arch/arm64/mm/fault.c | 26 ++++++++++++++++++++++++++
arch/arm64/mm/proc.S | 10 +++++++---
include/linux/gfp.h | 9 +++++++--
include/linux/highmem.h | 8 ++++++++
mm/kasan/hw_tags.c | 9 ++++++++-
mm/page_alloc.c | 13 ++++++++++---
9 files changed, 96 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
index bc88a1ced0d7..67bf259ae768 100644
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -37,6 +37,7 @@ void mte_free_tag_storage(char *storage);
/* track which pages have valid allocation tags */
#define PG_mte_tagged PG_arch_2
+void mte_zero_clear_page_tags(void *addr);
void mte_sync_tags(pte_t *ptep, pte_t pte);
void mte_copy_page_tags(void *kto, const void *kfrom);
void mte_thread_init_user(void);
@@ -53,6 +54,9 @@ int mte_ptrace_copy_tags(struct task_struct *child, long request,
/* unused if !CONFIG_ARM64_MTE, silence the compiler */
#define PG_mte_tagged 0
+static inline void mte_zero_clear_page_tags(void *addr)
+{
+}
static inline void mte_sync_tags(pte_t *ptep, pte_t pte)
{
}
diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h
index 0cfe4f7e7055..ed1b9dcf12b2 100644
--- a/arch/arm64/include/asm/page.h
+++ b/arch/arm64/include/asm/page.h
@@ -13,6 +13,7 @@
#ifndef __ASSEMBLY__
#include <linux/personality.h> /* for READ_IMPLIES_EXEC */
+#include <linux/types.h> /* for gfp_t */
#include <asm/pgtable-types.h>
struct page;
@@ -28,10 +29,13 @@ void copy_user_highpage(struct page *to, struct page *from,
void copy_highpage(struct page *to, struct page *from);
#define __HAVE_ARCH_COPY_HIGHPAGE
-#define alloc_zeroed_user_highpage_movable(movableflags, vma, vaddr) \
- alloc_page_vma(GFP_HIGHUSER_MOVABLE | __GFP_ZERO, vma, vaddr)
+struct page *alloc_zeroed_user_highpage_movable(struct vm_area_struct *vma,
+ unsigned long vaddr);
#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE_MOVABLE
+void tag_clear_highpage(struct page *to);
+#define __HAVE_ARCH_TAG_CLEAR_HIGHPAGE
+
#define clear_user_page(page, vaddr, pg) clear_page(page)
#define copy_user_page(to, from, vaddr, pg) copy_page(to, from)
diff --git a/arch/arm64/lib/mte.S b/arch/arm64/lib/mte.S
index 351537c12f36..e83643b3995f 100644
--- a/arch/arm64/lib/mte.S
+++ b/arch/arm64/lib/mte.S
@@ -36,6 +36,26 @@ SYM_FUNC_START(mte_clear_page_tags)
ret
SYM_FUNC_END(mte_clear_page_tags)
+/*
+ * Zero the page and tags at the same time
+ *
+ * Parameters:
+ * x0 - address to the beginning of the page
+ */
+SYM_FUNC_START(mte_zero_clear_page_tags)
+ mrs x1, dczid_el0
+ and w1, w1, #0xf
+ mov x2, #4
+ lsl x1, x2, x1
+ and x0, x0, #(1 << MTE_TAG_SHIFT) - 1 // clear the tag
+
+1: dc gzva, x0
+ add x0, x0, x1
+ tst x0, #(PAGE_SIZE - 1)
+ b.ne 1b
+ ret
+SYM_FUNC_END(mte_zero_clear_page_tags)
+
/*
* Copy the tags from the source page to the destination one
* x0 - address of the destination page
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 871c82ab0a30..180c0343d82a 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -921,3 +921,29 @@ void do_debug_exception(unsigned long addr_if_watchpoint, unsigned int esr,
debug_exception_exit(regs);
}
NOKPROBE_SYMBOL(do_debug_exception);
+
+/*
+ * Used during anonymous page fault handling.
+ */
+struct page *alloc_zeroed_user_highpage_movable(struct vm_area_struct *vma,
+ unsigned long vaddr)
+{
+ gfp_t flags = GFP_HIGHUSER_MOVABLE | __GFP_ZERO;
+
+ /*
+ * If the page is mapped with PROT_MTE, initialise the tags at the
+ * point of allocation and page zeroing as this is usually faster than
+ * separate DC ZVA and STGM.
+ */
+ if (vma->vm_flags & VM_MTE)
+ flags |= __GFP_ZEROTAGS;
+
+ return alloc_page_vma(flags, vma, vaddr);
+}
+
+void tag_clear_highpage(struct page *page)
+{
+ mte_zero_clear_page_tags(page_address(page));
+ page_kasan_tag_reset(page);
+ set_bit(PG_mte_tagged, &page->flags);
+}
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 97d7bcd8d4f2..48fd1df3d05a 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -46,9 +46,13 @@
#endif
#ifdef CONFIG_KASAN_HW_TAGS
-#define TCR_KASAN_HW_FLAGS SYS_TCR_EL1_TCMA1 | TCR_TBI1 | TCR_TBID1
+#define TCR_MTE_FLAGS SYS_TCR_EL1_TCMA1 | TCR_TBI1 | TCR_TBID1
#else
-#define TCR_KASAN_HW_FLAGS 0
+/*
+ * The mte_zero_clear_page_tags() implementation uses DC GZVA, which relies on
+ * TBI being enabled at EL1.
+ */
+#define TCR_MTE_FLAGS TCR_TBI1 | TCR_TBID1
#endif
/*
@@ -464,7 +468,7 @@ SYM_FUNC_START(__cpu_setup)
msr_s SYS_TFSRE0_EL1, xzr
/* set the TCR_EL1 bits */
- mov_q x10, TCR_KASAN_HW_FLAGS
+ mov_q x10, TCR_MTE_FLAGS
orr tcr, tcr, x10
1:
#endif
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 11da8af06704..68ba237365dc 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -53,8 +53,9 @@ struct vm_area_struct;
#define ___GFP_HARDWALL 0x100000u
#define ___GFP_THISNODE 0x200000u
#define ___GFP_ACCOUNT 0x400000u
+#define ___GFP_ZEROTAGS 0x800000u
#ifdef CONFIG_LOCKDEP
-#define ___GFP_NOLOCKDEP 0x800000u
+#define ___GFP_NOLOCKDEP 0x1000000u
#else
#define ___GFP_NOLOCKDEP 0
#endif
@@ -229,16 +230,20 @@ struct vm_area_struct;
* %__GFP_COMP address compound page metadata.
*
* %__GFP_ZERO returns a zeroed page on success.
+ *
+ * %__GFP_ZEROTAGS returns a page with zeroed memory tags on success, if
+ * __GFP_ZERO is set.
*/
#define __GFP_NOWARN ((__force gfp_t)___GFP_NOWARN)
#define __GFP_COMP ((__force gfp_t)___GFP_COMP)
#define __GFP_ZERO ((__force gfp_t)___GFP_ZERO)
+#define __GFP_ZEROTAGS ((__force gfp_t)___GFP_ZEROTAGS)
/* Disable lockdep for GFP context tracking */
#define __GFP_NOLOCKDEP ((__force gfp_t)___GFP_NOLOCKDEP)
/* Room for N __GFP_FOO bits */
-#define __GFP_BITS_SHIFT (23 + IS_ENABLED(CONFIG_LOCKDEP))
+#define __GFP_BITS_SHIFT (24 + IS_ENABLED(CONFIG_LOCKDEP))
#define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1))
/**
diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index 54d0643b8fcf..8c6e8e996c87 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -185,6 +185,14 @@ static inline void clear_highpage(struct page *page)
kunmap_atomic(kaddr);
}
+#ifndef __HAVE_ARCH_TAG_CLEAR_HIGHPAGE
+
+static inline void tag_clear_highpage(struct page *page)
+{
+}
+
+#endif
+
/*
* If we pass in a base or tail page, we can zero up to PAGE_SIZE.
* If we pass in a head page, we can zero up to the size of the compound page.
diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
index 9d0f6f934016..41fd5326ee0a 100644
--- a/mm/kasan/hw_tags.c
+++ b/mm/kasan/hw_tags.c
@@ -246,7 +246,14 @@ void kasan_alloc_pages(struct page *page, unsigned int order, gfp_t flags)
*/
bool init = !want_init_on_free() && want_init_on_alloc(flags);
- kasan_unpoison_pages(page, order, init);
+ if (flags & __GFP_ZEROTAGS) {
+ int i;
+
+ for (i = 0; i != 1 << order; ++i)
+ tag_clear_highpage(page + i);
+ } else {
+ kasan_unpoison_pages(page, order, init);
+ }
}
void kasan_free_pages(struct page *page, unsigned int order)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4fddb7cac3c6..13937e793fda 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1219,10 +1219,16 @@ static int free_tail_pages_check(struct page *head_page, struct page *page)
return ret;
}
-static void kernel_init_free_pages(struct page *page, int numpages)
+static void kernel_init_free_pages(struct page *page, int numpages, bool zero_tags)
{
int i;
+ if (zero_tags) {
+ for (i = 0; i < numpages; i++)
+ tag_clear_highpage(page + i);
+ return;
+ }
+
/* s390's use of memset() could override KASAN redzones. */
kasan_disable_current();
for (i = 0; i < numpages; i++) {
@@ -1314,7 +1320,7 @@ static __always_inline bool free_pages_prepare(struct page *page,
bool init = want_init_on_free();
if (init)
- kernel_init_free_pages(page, 1 << order);
+ kernel_init_free_pages(page, 1 << order, false);
if (!skip_kasan_poison)
kasan_poison_pages(page, order, init);
}
@@ -2349,7 +2355,8 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
kasan_unpoison_pages(page, order, init);
if (init)
- kernel_init_free_pages(page, 1 << order);
+ kernel_init_free_pages(page, 1 << order,
+ gfp_flags & __GFP_ZEROTAGS);
}
set_page_owner(page, order, gfp_flags);
--
2.32.0.rc0.204.g9fa02ecfa5-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v4 4/4] kasan: disable freed user page poisoning with HW tags
2021-05-28 1:04 [PATCH v4 0/4] arm64: improve efficiency of setting tags for user pages Peter Collingbourne
` (2 preceding siblings ...)
2021-05-28 1:04 ` [PATCH v4 3/4] arm64: mte: handle tags zeroing at page allocation time Peter Collingbourne
@ 2021-05-28 1:04 ` Peter Collingbourne
3 siblings, 0 replies; 9+ messages in thread
From: Peter Collingbourne @ 2021-05-28 1:04 UTC (permalink / raw)
To: Andrey Konovalov, Alexander Potapenko, Catalin Marinas,
Vincenzo Frascino, Andrew Morton, Jann Horn
Cc: Peter Collingbourne, Evgenii Stepanov, linux-mm, linux-arm-kernel
Poisoning freed pages protects against kernel use-after-free. The
likelihood of such a bug involving kernel pages is significantly higher
than that for user pages. At the same time, poisoning freed pages can
impose a significant performance cost, which cannot always be justified
for user pages given the lower probability of finding a bug. Therefore,
disable freed user page poisoning when using HW tags. We identify
"user" pages via the flag set GFP_HIGHUSER_MOVABLE, which indicates
a strong likelihood of not being directly accessible to the kernel.
Signed-off-by: Peter Collingbourne <pcc@google.com>
Link: https://linux-review.googlesource.com/id/I716846e2de8ef179f44e835770df7e6307be96c9
---
v4:
- move flag to GFP_HIGHUSER_MOVABLE
- remove command line flag
include/linux/gfp.h | 13 ++++++++++---
include/linux/page-flags.h | 9 +++++++++
include/trace/events/mmflags.h | 9 ++++++++-
mm/kasan/hw_tags.c | 3 +++
mm/page_alloc.c | 12 +++++++-----
5 files changed, 37 insertions(+), 9 deletions(-)
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 68ba237365dc..e6102dfa4faa 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -54,8 +54,9 @@ struct vm_area_struct;
#define ___GFP_THISNODE 0x200000u
#define ___GFP_ACCOUNT 0x400000u
#define ___GFP_ZEROTAGS 0x800000u
+#define ___GFP_SKIP_KASAN_POISON 0x1000000u
#ifdef CONFIG_LOCKDEP
-#define ___GFP_NOLOCKDEP 0x1000000u
+#define ___GFP_NOLOCKDEP 0x2000000u
#else
#define ___GFP_NOLOCKDEP 0
#endif
@@ -233,17 +234,22 @@ struct vm_area_struct;
*
* %__GFP_ZEROTAGS returns a page with zeroed memory tags on success, if
* __GFP_ZERO is set.
+ *
+ * %__GFP_SKIP_KASAN_POISON returns a page which does not need to be poisoned
+ * on deallocation. Typically used for userspace pages. Currently only has an
+ * effect in HW tags mode.
*/
#define __GFP_NOWARN ((__force gfp_t)___GFP_NOWARN)
#define __GFP_COMP ((__force gfp_t)___GFP_COMP)
#define __GFP_ZERO ((__force gfp_t)___GFP_ZERO)
#define __GFP_ZEROTAGS ((__force gfp_t)___GFP_ZEROTAGS)
+#define __GFP_SKIP_KASAN_POISON ((__force gfp_t)___GFP_SKIP_KASAN_POISON)
/* Disable lockdep for GFP context tracking */
#define __GFP_NOLOCKDEP ((__force gfp_t)___GFP_NOLOCKDEP)
/* Room for N __GFP_FOO bits */
-#define __GFP_BITS_SHIFT (24 + IS_ENABLED(CONFIG_LOCKDEP))
+#define __GFP_BITS_SHIFT (25 + IS_ENABLED(CONFIG_LOCKDEP))
#define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1))
/**
@@ -324,7 +330,8 @@ struct vm_area_struct;
#define GFP_DMA __GFP_DMA
#define GFP_DMA32 __GFP_DMA32
#define GFP_HIGHUSER (GFP_USER | __GFP_HIGHMEM)
-#define GFP_HIGHUSER_MOVABLE (GFP_HIGHUSER | __GFP_MOVABLE)
+#define GFP_HIGHUSER_MOVABLE (GFP_HIGHUSER | __GFP_MOVABLE | \
+ __GFP_SKIP_KASAN_POISON)
#define GFP_TRANSHUGE_LIGHT ((GFP_HIGHUSER_MOVABLE | __GFP_COMP | \
__GFP_NOMEMALLOC | __GFP_NOWARN) & ~__GFP_RECLAIM)
#define GFP_TRANSHUGE (GFP_TRANSHUGE_LIGHT | __GFP_DIRECT_RECLAIM)
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 04a34c08e0a6..40e2c5000585 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -137,6 +137,9 @@ enum pageflags {
#endif
#ifdef CONFIG_64BIT
PG_arch_2,
+#endif
+#ifdef CONFIG_KASAN_HW_TAGS
+ PG_skip_kasan_poison,
#endif
__NR_PAGEFLAGS,
@@ -443,6 +446,12 @@ TESTCLEARFLAG(Young, young, PF_ANY)
PAGEFLAG(Idle, idle, PF_ANY)
#endif
+#ifdef CONFIG_KASAN_HW_TAGS
+PAGEFLAG(SkipKASanPoison, skip_kasan_poison, PF_HEAD)
+#else
+PAGEFLAG_FALSE(SkipKASanPoison)
+#endif
+
/*
* PageReported() is used to track reported free pages within the Buddy
* allocator. We can use the non-atomic version of the test and set
diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
index 629c7a0eaff2..390270e00a1d 100644
--- a/include/trace/events/mmflags.h
+++ b/include/trace/events/mmflags.h
@@ -85,6 +85,12 @@
#define IF_HAVE_PG_ARCH_2(flag,string)
#endif
+#ifdef CONFIG_KASAN_HW_TAGS
+#define IF_HAVE_PG_SKIP_KASAN_POISON(flag,string) ,{1UL << flag, string}
+#else
+#define IF_HAVE_PG_SKIP_KASAN_POISON(flag,string)
+#endif
+
#define __def_pageflag_names \
{1UL << PG_locked, "locked" }, \
{1UL << PG_waiters, "waiters" }, \
@@ -112,7 +118,8 @@ IF_HAVE_PG_UNCACHED(PG_uncached, "uncached" ) \
IF_HAVE_PG_HWPOISON(PG_hwpoison, "hwpoison" ) \
IF_HAVE_PG_IDLE(PG_young, "young" ) \
IF_HAVE_PG_IDLE(PG_idle, "idle" ) \
-IF_HAVE_PG_ARCH_2(PG_arch_2, "arch_2" )
+IF_HAVE_PG_ARCH_2(PG_arch_2, "arch_2" ) \
+IF_HAVE_PG_SKIP_KASAN_POISON(PG_skip_kasan_poison, "skip_kasan_poison")
#define show_page_flags(flags) \
(flags) ? __print_flags(flags, "|", \
diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
index 41fd5326ee0a..ed5e5b833d61 100644
--- a/mm/kasan/hw_tags.c
+++ b/mm/kasan/hw_tags.c
@@ -246,6 +246,9 @@ void kasan_alloc_pages(struct page *page, unsigned int order, gfp_t flags)
*/
bool init = !want_init_on_free() && want_init_on_alloc(flags);
+ if (flags & __GFP_SKIP_KASAN_POISON)
+ SetPageSkipKASanPoison(page);
+
if (flags & __GFP_ZEROTAGS) {
int i;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 13937e793fda..5ad76e540a22 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -394,11 +394,12 @@ static DEFINE_STATIC_KEY_TRUE(deferred_pages);
* on-demand allocation and then freed again before the deferred pages
* initialization is done, but this is not likely to happen.
*/
-static inline bool should_skip_kasan_poison(fpi_t fpi_flags)
+static inline bool should_skip_kasan_poison(struct page *page, fpi_t fpi_flags)
{
return static_branch_unlikely(&deferred_pages) ||
(!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
- (fpi_flags & FPI_SKIP_KASAN_POISON));
+ (fpi_flags & FPI_SKIP_KASAN_POISON)) ||
+ PageSkipKASanPoison(page);
}
/* Returns true if the struct page for the pfn is uninitialised */
@@ -449,10 +450,11 @@ defer_init(int nid, unsigned long pfn, unsigned long end_pfn)
return false;
}
#else
-static inline bool should_skip_kasan_poison(fpi_t fpi_flags)
+static inline bool should_skip_kasan_poison(struct page *page, fpi_t fpi_flags)
{
return (!IS_ENABLED(CONFIG_KASAN_GENERIC) &&
- (fpi_flags & FPI_SKIP_KASAN_POISON));
+ (fpi_flags & FPI_SKIP_KASAN_POISON)) ||
+ PageSkipKASanPoison(page);
}
static inline bool early_page_uninitialised(unsigned long pfn)
@@ -1244,7 +1246,7 @@ static __always_inline bool free_pages_prepare(struct page *page,
unsigned int order, bool check_free, fpi_t fpi_flags)
{
int bad = 0;
- bool skip_kasan_poison = should_skip_kasan_poison(fpi_flags);
+ bool skip_kasan_poison = should_skip_kasan_poison(page, fpi_flags);
VM_BUG_ON_PAGE(PageTail(page), page);
--
2.32.0.rc0.204.g9fa02ecfa5-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread