All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6] mm,kfence: decouple kfence from page granularity mapping judgement
@ 2023-03-13 11:57 ` Zhenhua Huang
  0 siblings, 0 replies; 8+ messages in thread
From: Zhenhua Huang @ 2023-03-13 11:57 UTC (permalink / raw)
  To: catalin.marinas, will, glider, elver, dvyukov, akpm,
	robin.murphy, mark.rutland, jianyong.wu, james.morse,
	wangkefeng.wang
  Cc: Zhenhua Huang, linux-arm-kernel, kasan-dev, linux-mm,
	quic_pkondeti, quic_guptap, quic_tingweiz

Kfence only needs its pool to be mapped as page granularity, if it is
inited early. Previous judgement was a bit over protected. From [1], Mark
suggested to "just map the KFENCE region a page granularity". So I
decouple it from judgement and do page granularity mapping for kfence
pool only. Need to be noticed that late init of kfence pool still requires
page granularity mapping.

Page granularity mapping in theory cost more(2M per 1GB) memory on arm64
platform. Like what I've tested on QEMU(emulated 1GB RAM) with
gki_defconfig, also turning off rodata protection:
Before:
[root@liebao ]# cat /proc/meminfo
MemTotal:         999484 kB
After:
[root@liebao ]# cat /proc/meminfo
MemTotal:        1001480 kB

To implement this, also relocate the kfence pool allocation before the
linear mapping setting up, arm64_kfence_alloc_pool is to allocate phys
addr, __kfence_pool is to be set after linear mapping set up.

LINK: [1] https://lore.kernel.org/linux-arm-kernel/Y+IsdrvDNILA59UN@FVFF77S0Q05N/
Suggested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com>
---
 arch/arm64/mm/mmu.c      | 42 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm64/mm/pageattr.c |  8 ++++++--
 include/linux/kfence.h   | 10 ++++++++++
 mm/kfence/core.c         |  9 +++++++++
 4 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 6f9d889..ca5c932 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -24,6 +24,7 @@
 #include <linux/mm.h>
 #include <linux/vmalloc.h>
 #include <linux/set_memory.h>
+#include <linux/kfence.h>
 
 #include <asm/barrier.h>
 #include <asm/cputype.h>
@@ -525,6 +526,31 @@ static int __init enable_crash_mem_map(char *arg)
 }
 early_param("crashkernel", enable_crash_mem_map);
 
+#ifdef CONFIG_KFENCE
+
+static phys_addr_t arm64_kfence_alloc_pool(void)
+{
+	phys_addr_t kfence_pool;
+
+	if (!kfence_sample_interval)
+		return 0;
+
+	kfence_pool = memblock_phys_alloc(KFENCE_POOL_SIZE, PAGE_SIZE);
+	if (!kfence_pool)
+		pr_err("failed to allocate kfence pool\n");
+
+	return kfence_pool;
+}
+
+#else
+
+static phys_addr_t arm64_kfence_alloc_pool(void)
+{
+	return 0;
+}
+
+#endif
+
 static void __init map_mem(pgd_t *pgdp)
 {
 	static const u64 direct_map_end = _PAGE_END(VA_BITS_MIN);
@@ -532,6 +558,7 @@ static void __init map_mem(pgd_t *pgdp)
 	phys_addr_t kernel_end = __pa_symbol(__init_begin);
 	phys_addr_t start, end;
 	int flags = NO_EXEC_MAPPINGS;
+	phys_addr_t kfence_pool;
 	u64 i;
 
 	/*
@@ -564,6 +591,10 @@ static void __init map_mem(pgd_t *pgdp)
 	}
 #endif
 
+	kfence_pool = arm64_kfence_alloc_pool();
+	if (kfence_pool)
+		memblock_mark_nomap(kfence_pool, KFENCE_POOL_SIZE);
+
 	/* map all the memory banks */
 	for_each_mem_range(i, &start, &end) {
 		if (start >= end)
@@ -608,6 +639,17 @@ static void __init map_mem(pgd_t *pgdp)
 		}
 	}
 #endif
+
+	/* Kfence pool needs page-level mapping */
+	if (kfence_pool) {
+		__map_memblock(pgdp, kfence_pool,
+			kfence_pool + KFENCE_POOL_SIZE,
+			pgprot_tagged(PAGE_KERNEL),
+			NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS);
+		memblock_clear_nomap(kfence_pool, KFENCE_POOL_SIZE);
+		/* kfence_pool really mapped now */
+		kfence_set_pool(kfence_pool);
+	}
 }
 
 void mark_rodata_ro(void)
diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
index 79dd201..25e4a983 100644
--- a/arch/arm64/mm/pageattr.c
+++ b/arch/arm64/mm/pageattr.c
@@ -7,6 +7,7 @@
 #include <linux/module.h>
 #include <linux/sched.h>
 #include <linux/vmalloc.h>
+#include <linux/kfence.h>
 
 #include <asm/cacheflush.h>
 #include <asm/set_memory.h>
@@ -22,12 +23,15 @@ bool rodata_full __ro_after_init = IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED
 bool can_set_direct_map(void)
 {
 	/*
-	 * rodata_full, DEBUG_PAGEALLOC and KFENCE require linear map to be
+	 * rodata_full and DEBUG_PAGEALLOC require linear map to be
 	 * mapped at page granularity, so that it is possible to
 	 * protect/unprotect single pages.
+	 *
+	 * Kfence pool requires page granularity mapping also if we init it
+	 * late.
 	 */
 	return (rodata_enabled && rodata_full) || debug_pagealloc_enabled() ||
-		IS_ENABLED(CONFIG_KFENCE);
+	    (IS_ENABLED(CONFIG_KFENCE) && !kfence_sample_interval);
 }
 
 static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
diff --git a/include/linux/kfence.h b/include/linux/kfence.h
index 726857a..2b77eee 100644
--- a/include/linux/kfence.h
+++ b/include/linux/kfence.h
@@ -64,6 +64,12 @@ static __always_inline bool is_kfence_address(const void *addr)
 void __init kfence_alloc_pool(void);
 
 /**
+ * kfence_set_pool() - allows an arch to set the
+ * KFENCE pool during early init
+ */
+void __init kfence_set_pool(phys_addr_t addr);
+
+/**
  * kfence_init() - perform KFENCE initialization at boot time
  *
  * Requires that kfence_alloc_pool() was called before. This sets up the
@@ -222,8 +228,12 @@ bool __kfence_obj_info(struct kmem_obj_info *kpp, void *object, struct slab *sla
 
 #else /* CONFIG_KFENCE */
 
+extern unsigned long kfence_sample_interval;
+
+#define KFENCE_POOL_SIZE 0
 static inline bool is_kfence_address(const void *addr) { return false; }
 static inline void kfence_alloc_pool(void) { }
+static inline void kfence_set_pool(phys_addr_t addr) { }
 static inline void kfence_init(void) { }
 static inline void kfence_shutdown_cache(struct kmem_cache *s) { }
 static inline void *kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags) { return NULL; }
diff --git a/mm/kfence/core.c b/mm/kfence/core.c
index 5349c37..0765395 100644
--- a/mm/kfence/core.c
+++ b/mm/kfence/core.c
@@ -814,12 +814,21 @@ void __init kfence_alloc_pool(void)
 	if (!kfence_sample_interval)
 		return;
 
+	/* if the pool has already been initialized by arch, skip the below */
+	if (__kfence_pool)
+		return;
+
 	__kfence_pool = memblock_alloc(KFENCE_POOL_SIZE, PAGE_SIZE);
 
 	if (!__kfence_pool)
 		pr_err("failed to allocate pool\n");
 }
 
+void __init kfence_set_pool(phys_addr_t addr)
+{
+	__kfence_pool = phys_to_virt(addr);
+}
+
 static void kfence_init_enable(void)
 {
 	if (!IS_ENABLED(CONFIG_KFENCE_STATIC_KEYS))
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH v6] mm,kfence: decouple kfence from page granularity mapping judgement
@ 2023-03-13 11:57 ` Zhenhua Huang
  0 siblings, 0 replies; 8+ messages in thread
From: Zhenhua Huang @ 2023-03-13 11:57 UTC (permalink / raw)
  To: catalin.marinas, will, glider, elver, dvyukov, akpm,
	robin.murphy, mark.rutland, jianyong.wu, james.morse,
	wangkefeng.wang
  Cc: Zhenhua Huang, linux-arm-kernel, kasan-dev, linux-mm,
	quic_pkondeti, quic_guptap, quic_tingweiz

Kfence only needs its pool to be mapped as page granularity, if it is
inited early. Previous judgement was a bit over protected. From [1], Mark
suggested to "just map the KFENCE region a page granularity". So I
decouple it from judgement and do page granularity mapping for kfence
pool only. Need to be noticed that late init of kfence pool still requires
page granularity mapping.

Page granularity mapping in theory cost more(2M per 1GB) memory on arm64
platform. Like what I've tested on QEMU(emulated 1GB RAM) with
gki_defconfig, also turning off rodata protection:
Before:
[root@liebao ]# cat /proc/meminfo
MemTotal:         999484 kB
After:
[root@liebao ]# cat /proc/meminfo
MemTotal:        1001480 kB

To implement this, also relocate the kfence pool allocation before the
linear mapping setting up, arm64_kfence_alloc_pool is to allocate phys
addr, __kfence_pool is to be set after linear mapping set up.

LINK: [1] https://lore.kernel.org/linux-arm-kernel/Y+IsdrvDNILA59UN@FVFF77S0Q05N/
Suggested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com>
---
 arch/arm64/mm/mmu.c      | 42 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm64/mm/pageattr.c |  8 ++++++--
 include/linux/kfence.h   | 10 ++++++++++
 mm/kfence/core.c         |  9 +++++++++
 4 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 6f9d889..ca5c932 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -24,6 +24,7 @@
 #include <linux/mm.h>
 #include <linux/vmalloc.h>
 #include <linux/set_memory.h>
+#include <linux/kfence.h>
 
 #include <asm/barrier.h>
 #include <asm/cputype.h>
@@ -525,6 +526,31 @@ static int __init enable_crash_mem_map(char *arg)
 }
 early_param("crashkernel", enable_crash_mem_map);
 
+#ifdef CONFIG_KFENCE
+
+static phys_addr_t arm64_kfence_alloc_pool(void)
+{
+	phys_addr_t kfence_pool;
+
+	if (!kfence_sample_interval)
+		return 0;
+
+	kfence_pool = memblock_phys_alloc(KFENCE_POOL_SIZE, PAGE_SIZE);
+	if (!kfence_pool)
+		pr_err("failed to allocate kfence pool\n");
+
+	return kfence_pool;
+}
+
+#else
+
+static phys_addr_t arm64_kfence_alloc_pool(void)
+{
+	return 0;
+}
+
+#endif
+
 static void __init map_mem(pgd_t *pgdp)
 {
 	static const u64 direct_map_end = _PAGE_END(VA_BITS_MIN);
@@ -532,6 +558,7 @@ static void __init map_mem(pgd_t *pgdp)
 	phys_addr_t kernel_end = __pa_symbol(__init_begin);
 	phys_addr_t start, end;
 	int flags = NO_EXEC_MAPPINGS;
+	phys_addr_t kfence_pool;
 	u64 i;
 
 	/*
@@ -564,6 +591,10 @@ static void __init map_mem(pgd_t *pgdp)
 	}
 #endif
 
+	kfence_pool = arm64_kfence_alloc_pool();
+	if (kfence_pool)
+		memblock_mark_nomap(kfence_pool, KFENCE_POOL_SIZE);
+
 	/* map all the memory banks */
 	for_each_mem_range(i, &start, &end) {
 		if (start >= end)
@@ -608,6 +639,17 @@ static void __init map_mem(pgd_t *pgdp)
 		}
 	}
 #endif
+
+	/* Kfence pool needs page-level mapping */
+	if (kfence_pool) {
+		__map_memblock(pgdp, kfence_pool,
+			kfence_pool + KFENCE_POOL_SIZE,
+			pgprot_tagged(PAGE_KERNEL),
+			NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS);
+		memblock_clear_nomap(kfence_pool, KFENCE_POOL_SIZE);
+		/* kfence_pool really mapped now */
+		kfence_set_pool(kfence_pool);
+	}
 }
 
 void mark_rodata_ro(void)
diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
index 79dd201..25e4a983 100644
--- a/arch/arm64/mm/pageattr.c
+++ b/arch/arm64/mm/pageattr.c
@@ -7,6 +7,7 @@
 #include <linux/module.h>
 #include <linux/sched.h>
 #include <linux/vmalloc.h>
+#include <linux/kfence.h>
 
 #include <asm/cacheflush.h>
 #include <asm/set_memory.h>
@@ -22,12 +23,15 @@ bool rodata_full __ro_after_init = IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED
 bool can_set_direct_map(void)
 {
 	/*
-	 * rodata_full, DEBUG_PAGEALLOC and KFENCE require linear map to be
+	 * rodata_full and DEBUG_PAGEALLOC require linear map to be
 	 * mapped at page granularity, so that it is possible to
 	 * protect/unprotect single pages.
+	 *
+	 * Kfence pool requires page granularity mapping also if we init it
+	 * late.
 	 */
 	return (rodata_enabled && rodata_full) || debug_pagealloc_enabled() ||
-		IS_ENABLED(CONFIG_KFENCE);
+	    (IS_ENABLED(CONFIG_KFENCE) && !kfence_sample_interval);
 }
 
 static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
diff --git a/include/linux/kfence.h b/include/linux/kfence.h
index 726857a..2b77eee 100644
--- a/include/linux/kfence.h
+++ b/include/linux/kfence.h
@@ -64,6 +64,12 @@ static __always_inline bool is_kfence_address(const void *addr)
 void __init kfence_alloc_pool(void);
 
 /**
+ * kfence_set_pool() - allows an arch to set the
+ * KFENCE pool during early init
+ */
+void __init kfence_set_pool(phys_addr_t addr);
+
+/**
  * kfence_init() - perform KFENCE initialization at boot time
  *
  * Requires that kfence_alloc_pool() was called before. This sets up the
@@ -222,8 +228,12 @@ bool __kfence_obj_info(struct kmem_obj_info *kpp, void *object, struct slab *sla
 
 #else /* CONFIG_KFENCE */
 
+extern unsigned long kfence_sample_interval;
+
+#define KFENCE_POOL_SIZE 0
 static inline bool is_kfence_address(const void *addr) { return false; }
 static inline void kfence_alloc_pool(void) { }
+static inline void kfence_set_pool(phys_addr_t addr) { }
 static inline void kfence_init(void) { }
 static inline void kfence_shutdown_cache(struct kmem_cache *s) { }
 static inline void *kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags) { return NULL; }
diff --git a/mm/kfence/core.c b/mm/kfence/core.c
index 5349c37..0765395 100644
--- a/mm/kfence/core.c
+++ b/mm/kfence/core.c
@@ -814,12 +814,21 @@ void __init kfence_alloc_pool(void)
 	if (!kfence_sample_interval)
 		return;
 
+	/* if the pool has already been initialized by arch, skip the below */
+	if (__kfence_pool)
+		return;
+
 	__kfence_pool = memblock_alloc(KFENCE_POOL_SIZE, PAGE_SIZE);
 
 	if (!__kfence_pool)
 		pr_err("failed to allocate pool\n");
 }
 
+void __init kfence_set_pool(phys_addr_t addr)
+{
+	__kfence_pool = phys_to_virt(addr);
+}
+
 static void kfence_init_enable(void)
 {
 	if (!IS_ENABLED(CONFIG_KFENCE_STATIC_KEYS))
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH v6] mm,kfence: decouple kfence from page granularity mapping judgement
  2023-03-13 11:57 ` Zhenhua Huang
@ 2023-03-13 13:00   ` Marco Elver
  -1 siblings, 0 replies; 8+ messages in thread
From: Marco Elver @ 2023-03-13 13:00 UTC (permalink / raw)
  To: Zhenhua Huang
  Cc: catalin.marinas, will, glider, dvyukov, akpm, robin.murphy,
	mark.rutland, jianyong.wu, james.morse, wangkefeng.wang,
	linux-arm-kernel, kasan-dev, linux-mm, quic_pkondeti,
	quic_guptap, quic_tingweiz

On Mon, 13 Mar 2023 at 12:57, Zhenhua Huang <quic_zhenhuah@quicinc.com> wrote:
>
> Kfence only needs its pool to be mapped as page granularity, if it is
> inited early. Previous judgement was a bit over protected. From [1], Mark
> suggested to "just map the KFENCE region a page granularity". So I
> decouple it from judgement and do page granularity mapping for kfence
> pool only. Need to be noticed that late init of kfence pool still requires
> page granularity mapping.
>
> Page granularity mapping in theory cost more(2M per 1GB) memory on arm64
> platform. Like what I've tested on QEMU(emulated 1GB RAM) with
> gki_defconfig, also turning off rodata protection:
> Before:
> [root@liebao ]# cat /proc/meminfo
> MemTotal:         999484 kB
> After:
> [root@liebao ]# cat /proc/meminfo
> MemTotal:        1001480 kB
>
> To implement this, also relocate the kfence pool allocation before the
> linear mapping setting up, arm64_kfence_alloc_pool is to allocate phys
> addr, __kfence_pool is to be set after linear mapping set up.
>
> LINK: [1] https://lore.kernel.org/linux-arm-kernel/Y+IsdrvDNILA59UN@FVFF77S0Q05N/
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com>
> ---
>  arch/arm64/mm/mmu.c      | 42 ++++++++++++++++++++++++++++++++++++++++++
>  arch/arm64/mm/pageattr.c |  8 ++++++--
>  include/linux/kfence.h   | 10 ++++++++++
>  mm/kfence/core.c         |  9 +++++++++
>  4 files changed, 67 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 6f9d889..ca5c932 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -24,6 +24,7 @@
>  #include <linux/mm.h>
>  #include <linux/vmalloc.h>
>  #include <linux/set_memory.h>
> +#include <linux/kfence.h>
>
>  #include <asm/barrier.h>
>  #include <asm/cputype.h>
> @@ -525,6 +526,31 @@ static int __init enable_crash_mem_map(char *arg)
>  }
>  early_param("crashkernel", enable_crash_mem_map);
>
> +#ifdef CONFIG_KFENCE
> +
> +static phys_addr_t arm64_kfence_alloc_pool(void)
> +{
> +       phys_addr_t kfence_pool;
> +
> +       if (!kfence_sample_interval)
> +               return 0;
> +
> +       kfence_pool = memblock_phys_alloc(KFENCE_POOL_SIZE, PAGE_SIZE);
> +       if (!kfence_pool)
> +               pr_err("failed to allocate kfence pool\n");
> +
> +       return kfence_pool;
> +}
> +
> +#else
> +
> +static phys_addr_t arm64_kfence_alloc_pool(void)
> +{
> +       return 0;
> +}
> +
> +#endif
> +
>  static void __init map_mem(pgd_t *pgdp)
>  {
>         static const u64 direct_map_end = _PAGE_END(VA_BITS_MIN);
> @@ -532,6 +558,7 @@ static void __init map_mem(pgd_t *pgdp)
>         phys_addr_t kernel_end = __pa_symbol(__init_begin);
>         phys_addr_t start, end;
>         int flags = NO_EXEC_MAPPINGS;
> +       phys_addr_t kfence_pool;
>         u64 i;
>
>         /*
> @@ -564,6 +591,10 @@ static void __init map_mem(pgd_t *pgdp)
>         }
>  #endif
>
> +       kfence_pool = arm64_kfence_alloc_pool();
> +       if (kfence_pool)
> +               memblock_mark_nomap(kfence_pool, KFENCE_POOL_SIZE);
> +
>         /* map all the memory banks */
>         for_each_mem_range(i, &start, &end) {
>                 if (start >= end)
> @@ -608,6 +639,17 @@ static void __init map_mem(pgd_t *pgdp)
>                 }
>         }
>  #endif
> +
> +       /* Kfence pool needs page-level mapping */
> +       if (kfence_pool) {
> +               __map_memblock(pgdp, kfence_pool,
> +                       kfence_pool + KFENCE_POOL_SIZE,
> +                       pgprot_tagged(PAGE_KERNEL),
> +                       NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS);
> +               memblock_clear_nomap(kfence_pool, KFENCE_POOL_SIZE);
> +               /* kfence_pool really mapped now */
> +               kfence_set_pool(kfence_pool);
> +       }
>  }
>
>  void mark_rodata_ro(void)
> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
> index 79dd201..25e4a983 100644
> --- a/arch/arm64/mm/pageattr.c
> +++ b/arch/arm64/mm/pageattr.c
> @@ -7,6 +7,7 @@
>  #include <linux/module.h>
>  #include <linux/sched.h>
>  #include <linux/vmalloc.h>
> +#include <linux/kfence.h>
>
>  #include <asm/cacheflush.h>
>  #include <asm/set_memory.h>
> @@ -22,12 +23,15 @@ bool rodata_full __ro_after_init = IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED
>  bool can_set_direct_map(void)
>  {
>         /*
> -        * rodata_full, DEBUG_PAGEALLOC and KFENCE require linear map to be
> +        * rodata_full and DEBUG_PAGEALLOC require linear map to be
>          * mapped at page granularity, so that it is possible to
>          * protect/unprotect single pages.
> +        *
> +        * Kfence pool requires page granularity mapping also if we init it
> +        * late.
>          */
>         return (rodata_enabled && rodata_full) || debug_pagealloc_enabled() ||
> -               IS_ENABLED(CONFIG_KFENCE);
> +           (IS_ENABLED(CONFIG_KFENCE) && !kfence_sample_interval);

If you're struggling with kfence_sample_interval not existing if
!CONFIG_KFENCE, this is one of the occasions where it'd be perfectly
fine to write:

bool can_set_direct_map(void) {
#ifdef CONFIG_KFENCE
    /* ... your comment here ...*/
    if (!kfence_sample_interval)
        return true;
}
#endif
     return .........
}

>  }
>
>  static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
> diff --git a/include/linux/kfence.h b/include/linux/kfence.h
> index 726857a..2b77eee 100644
> --- a/include/linux/kfence.h
> +++ b/include/linux/kfence.h
> @@ -64,6 +64,12 @@ static __always_inline bool is_kfence_address(const void *addr)
>  void __init kfence_alloc_pool(void);
>
>  /**
> + * kfence_set_pool() - allows an arch to set the
> + * KFENCE pool during early init
> + */
> +void __init kfence_set_pool(phys_addr_t addr);
> +
> +/**
>   * kfence_init() - perform KFENCE initialization at boot time
>   *
>   * Requires that kfence_alloc_pool() was called before. This sets up the
> @@ -222,8 +228,12 @@ bool __kfence_obj_info(struct kmem_obj_info *kpp, void *object, struct slab *sla
>
>  #else /* CONFIG_KFENCE */
>
> +extern unsigned long kfence_sample_interval;

This variable does not exist if !CONFIG_KFENCE, please remove. See
suggestion above.

> +#define KFENCE_POOL_SIZE 0
>  static inline bool is_kfence_address(const void *addr) { return false; }
>  static inline void kfence_alloc_pool(void) { }
> +static inline void kfence_set_pool(phys_addr_t addr) { }
>  static inline void kfence_init(void) { }
>  static inline void kfence_shutdown_cache(struct kmem_cache *s) { }
>  static inline void *kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags) { return NULL; }
> diff --git a/mm/kfence/core.c b/mm/kfence/core.c
> index 5349c37..0765395 100644
> --- a/mm/kfence/core.c
> +++ b/mm/kfence/core.c
> @@ -814,12 +814,21 @@ void __init kfence_alloc_pool(void)
>         if (!kfence_sample_interval)
>                 return;
>
> +       /* if the pool has already been initialized by arch, skip the below */
> +       if (__kfence_pool)
> +               return;
> +
>         __kfence_pool = memblock_alloc(KFENCE_POOL_SIZE, PAGE_SIZE);
>
>         if (!__kfence_pool)
>                 pr_err("failed to allocate pool\n");
>  }
>
> +void __init kfence_set_pool(phys_addr_t addr)
> +{
> +       __kfence_pool = phys_to_virt(addr);
> +}
> +
>  static void kfence_init_enable(void)
>  {
>         if (!IS_ENABLED(CONFIG_KFENCE_STATIC_KEYS))
> --
> 2.7.4
>


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v6] mm,kfence: decouple kfence from page granularity mapping judgement
@ 2023-03-13 13:00   ` Marco Elver
  0 siblings, 0 replies; 8+ messages in thread
From: Marco Elver @ 2023-03-13 13:00 UTC (permalink / raw)
  To: Zhenhua Huang
  Cc: catalin.marinas, will, glider, dvyukov, akpm, robin.murphy,
	mark.rutland, jianyong.wu, james.morse, wangkefeng.wang,
	linux-arm-kernel, kasan-dev, linux-mm, quic_pkondeti,
	quic_guptap, quic_tingweiz

On Mon, 13 Mar 2023 at 12:57, Zhenhua Huang <quic_zhenhuah@quicinc.com> wrote:
>
> Kfence only needs its pool to be mapped as page granularity, if it is
> inited early. Previous judgement was a bit over protected. From [1], Mark
> suggested to "just map the KFENCE region a page granularity". So I
> decouple it from judgement and do page granularity mapping for kfence
> pool only. Need to be noticed that late init of kfence pool still requires
> page granularity mapping.
>
> Page granularity mapping in theory cost more(2M per 1GB) memory on arm64
> platform. Like what I've tested on QEMU(emulated 1GB RAM) with
> gki_defconfig, also turning off rodata protection:
> Before:
> [root@liebao ]# cat /proc/meminfo
> MemTotal:         999484 kB
> After:
> [root@liebao ]# cat /proc/meminfo
> MemTotal:        1001480 kB
>
> To implement this, also relocate the kfence pool allocation before the
> linear mapping setting up, arm64_kfence_alloc_pool is to allocate phys
> addr, __kfence_pool is to be set after linear mapping set up.
>
> LINK: [1] https://lore.kernel.org/linux-arm-kernel/Y+IsdrvDNILA59UN@FVFF77S0Q05N/
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com>
> ---
>  arch/arm64/mm/mmu.c      | 42 ++++++++++++++++++++++++++++++++++++++++++
>  arch/arm64/mm/pageattr.c |  8 ++++++--
>  include/linux/kfence.h   | 10 ++++++++++
>  mm/kfence/core.c         |  9 +++++++++
>  4 files changed, 67 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 6f9d889..ca5c932 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -24,6 +24,7 @@
>  #include <linux/mm.h>
>  #include <linux/vmalloc.h>
>  #include <linux/set_memory.h>
> +#include <linux/kfence.h>
>
>  #include <asm/barrier.h>
>  #include <asm/cputype.h>
> @@ -525,6 +526,31 @@ static int __init enable_crash_mem_map(char *arg)
>  }
>  early_param("crashkernel", enable_crash_mem_map);
>
> +#ifdef CONFIG_KFENCE
> +
> +static phys_addr_t arm64_kfence_alloc_pool(void)
> +{
> +       phys_addr_t kfence_pool;
> +
> +       if (!kfence_sample_interval)
> +               return 0;
> +
> +       kfence_pool = memblock_phys_alloc(KFENCE_POOL_SIZE, PAGE_SIZE);
> +       if (!kfence_pool)
> +               pr_err("failed to allocate kfence pool\n");
> +
> +       return kfence_pool;
> +}
> +
> +#else
> +
> +static phys_addr_t arm64_kfence_alloc_pool(void)
> +{
> +       return 0;
> +}
> +
> +#endif
> +
>  static void __init map_mem(pgd_t *pgdp)
>  {
>         static const u64 direct_map_end = _PAGE_END(VA_BITS_MIN);
> @@ -532,6 +558,7 @@ static void __init map_mem(pgd_t *pgdp)
>         phys_addr_t kernel_end = __pa_symbol(__init_begin);
>         phys_addr_t start, end;
>         int flags = NO_EXEC_MAPPINGS;
> +       phys_addr_t kfence_pool;
>         u64 i;
>
>         /*
> @@ -564,6 +591,10 @@ static void __init map_mem(pgd_t *pgdp)
>         }
>  #endif
>
> +       kfence_pool = arm64_kfence_alloc_pool();
> +       if (kfence_pool)
> +               memblock_mark_nomap(kfence_pool, KFENCE_POOL_SIZE);
> +
>         /* map all the memory banks */
>         for_each_mem_range(i, &start, &end) {
>                 if (start >= end)
> @@ -608,6 +639,17 @@ static void __init map_mem(pgd_t *pgdp)
>                 }
>         }
>  #endif
> +
> +       /* Kfence pool needs page-level mapping */
> +       if (kfence_pool) {
> +               __map_memblock(pgdp, kfence_pool,
> +                       kfence_pool + KFENCE_POOL_SIZE,
> +                       pgprot_tagged(PAGE_KERNEL),
> +                       NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS);
> +               memblock_clear_nomap(kfence_pool, KFENCE_POOL_SIZE);
> +               /* kfence_pool really mapped now */
> +               kfence_set_pool(kfence_pool);
> +       }
>  }
>
>  void mark_rodata_ro(void)
> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
> index 79dd201..25e4a983 100644
> --- a/arch/arm64/mm/pageattr.c
> +++ b/arch/arm64/mm/pageattr.c
> @@ -7,6 +7,7 @@
>  #include <linux/module.h>
>  #include <linux/sched.h>
>  #include <linux/vmalloc.h>
> +#include <linux/kfence.h>
>
>  #include <asm/cacheflush.h>
>  #include <asm/set_memory.h>
> @@ -22,12 +23,15 @@ bool rodata_full __ro_after_init = IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED
>  bool can_set_direct_map(void)
>  {
>         /*
> -        * rodata_full, DEBUG_PAGEALLOC and KFENCE require linear map to be
> +        * rodata_full and DEBUG_PAGEALLOC require linear map to be
>          * mapped at page granularity, so that it is possible to
>          * protect/unprotect single pages.
> +        *
> +        * Kfence pool requires page granularity mapping also if we init it
> +        * late.
>          */
>         return (rodata_enabled && rodata_full) || debug_pagealloc_enabled() ||
> -               IS_ENABLED(CONFIG_KFENCE);
> +           (IS_ENABLED(CONFIG_KFENCE) && !kfence_sample_interval);

If you're struggling with kfence_sample_interval not existing if
!CONFIG_KFENCE, this is one of the occasions where it'd be perfectly
fine to write:

bool can_set_direct_map(void) {
#ifdef CONFIG_KFENCE
    /* ... your comment here ...*/
    if (!kfence_sample_interval)
        return true;
}
#endif
     return .........
}

>  }
>
>  static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
> diff --git a/include/linux/kfence.h b/include/linux/kfence.h
> index 726857a..2b77eee 100644
> --- a/include/linux/kfence.h
> +++ b/include/linux/kfence.h
> @@ -64,6 +64,12 @@ static __always_inline bool is_kfence_address(const void *addr)
>  void __init kfence_alloc_pool(void);
>
>  /**
> + * kfence_set_pool() - allows an arch to set the
> + * KFENCE pool during early init
> + */
> +void __init kfence_set_pool(phys_addr_t addr);
> +
> +/**
>   * kfence_init() - perform KFENCE initialization at boot time
>   *
>   * Requires that kfence_alloc_pool() was called before. This sets up the
> @@ -222,8 +228,12 @@ bool __kfence_obj_info(struct kmem_obj_info *kpp, void *object, struct slab *sla
>
>  #else /* CONFIG_KFENCE */
>
> +extern unsigned long kfence_sample_interval;

This variable does not exist if !CONFIG_KFENCE, please remove. See
suggestion above.

> +#define KFENCE_POOL_SIZE 0
>  static inline bool is_kfence_address(const void *addr) { return false; }
>  static inline void kfence_alloc_pool(void) { }
> +static inline void kfence_set_pool(phys_addr_t addr) { }
>  static inline void kfence_init(void) { }
>  static inline void kfence_shutdown_cache(struct kmem_cache *s) { }
>  static inline void *kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags) { return NULL; }
> diff --git a/mm/kfence/core.c b/mm/kfence/core.c
> index 5349c37..0765395 100644
> --- a/mm/kfence/core.c
> +++ b/mm/kfence/core.c
> @@ -814,12 +814,21 @@ void __init kfence_alloc_pool(void)
>         if (!kfence_sample_interval)
>                 return;
>
> +       /* if the pool has already been initialized by arch, skip the below */
> +       if (__kfence_pool)
> +               return;
> +
>         __kfence_pool = memblock_alloc(KFENCE_POOL_SIZE, PAGE_SIZE);
>
>         if (!__kfence_pool)
>                 pr_err("failed to allocate pool\n");
>  }
>
> +void __init kfence_set_pool(phys_addr_t addr)
> +{
> +       __kfence_pool = phys_to_virt(addr);
> +}
> +
>  static void kfence_init_enable(void)
>  {
>         if (!IS_ENABLED(CONFIG_KFENCE_STATIC_KEYS))
> --
> 2.7.4
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v6] mm,kfence: decouple kfence from page granularity mapping judgement
  2023-03-13 13:00   ` Marco Elver
@ 2023-03-13 14:42     ` Kefeng Wang
  -1 siblings, 0 replies; 8+ messages in thread
From: Kefeng Wang @ 2023-03-13 14:42 UTC (permalink / raw)
  To: Marco Elver, Zhenhua Huang
  Cc: catalin.marinas, will, glider, dvyukov, akpm, robin.murphy,
	mark.rutland, jianyong.wu, james.morse, linux-arm-kernel,
	kasan-dev, linux-mm, quic_pkondeti, quic_guptap, quic_tingweiz



On 2023/3/13 21:00, Marco Elver wrote:
> On Mon, 13 Mar 2023 at 12:57, Zhenhua Huang <quic_zhenhuah@quicinc.com> wrote:
>>
>> Kfence only needs its pool to be mapped as page granularity, if it is
>> inited early. Previous judgement was a bit over protected. From [1], Mark
>> suggested to "just map the KFENCE region a page granularity". So I
>> decouple it from judgement and do page granularity mapping for kfence
>> pool only. Need to be noticed that late init of kfence pool still requires
>> page granularity mapping.
>>
>> Page granularity mapping in theory cost more(2M per 1GB) memory on arm64
>> platform. Like what I've tested on QEMU(emulated 1GB RAM) with
>> gki_defconfig, also turning off rodata protection:
>> Before:
>> [root@liebao ]# cat /proc/meminfo
>> MemTotal:         999484 kB
>> After:
>> [root@liebao ]# cat /proc/meminfo
>> MemTotal:        1001480 kB
>>
>> To implement this, also relocate the kfence pool allocation before the
>> linear mapping setting up, arm64_kfence_alloc_pool is to allocate phys
>> addr, __kfence_pool is to be set after linear mapping set up.
>>
>> LINK: [1] https://lore.kernel.org/linux-arm-kernel/Y+IsdrvDNILA59UN@FVFF77S0Q05N/
>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>> Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com>
>> ---
>>   arch/arm64/mm/mmu.c      | 42 ++++++++++++++++++++++++++++++++++++++++++
>>   arch/arm64/mm/pageattr.c |  8 ++++++--
>>   include/linux/kfence.h   | 10 ++++++++++
>>   mm/kfence/core.c         |  9 +++++++++
>>   4 files changed, 67 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index 6f9d889..ca5c932 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -24,6 +24,7 @@
>>   #include <linux/mm.h>
>>   #include <linux/vmalloc.h>
>>   #include <linux/set_memory.h>
>> +#include <linux/kfence.h>
>>
>>   #include <asm/barrier.h>
>>   #include <asm/cputype.h>
>> @@ -525,6 +526,31 @@ static int __init enable_crash_mem_map(char *arg)
>>   }
>>   early_param("crashkernel", enable_crash_mem_map);
>>
>> +#ifdef CONFIG_KFENCE
>> +
>> +static phys_addr_t arm64_kfence_alloc_pool(void)
>> +{
>> +       phys_addr_t kfence_pool;
>> +
>> +       if (!kfence_sample_interval)
>> +               return 0;
>> +
>> +       kfence_pool = memblock_phys_alloc(KFENCE_POOL_SIZE, PAGE_SIZE);
>> +       if (!kfence_pool)
>> +               pr_err("failed to allocate kfence pool\n");
>> +
>> +       return kfence_pool;
>> +}
>> +
>> +#else
>> +
>> +static phys_addr_t arm64_kfence_alloc_pool(void)
>> +{
>> +       return 0;
>> +}
>> +
>> +#endif
>> +
>>   static void __init map_mem(pgd_t *pgdp)
>>   {
>>          static const u64 direct_map_end = _PAGE_END(VA_BITS_MIN);
>> @@ -532,6 +558,7 @@ static void __init map_mem(pgd_t *pgdp)
>>          phys_addr_t kernel_end = __pa_symbol(__init_begin);
>>          phys_addr_t start, end;
>>          int flags = NO_EXEC_MAPPINGS;
>> +       phys_addr_t kfence_pool;
>>          u64 i;
>>
>>          /*
>> @@ -564,6 +591,10 @@ static void __init map_mem(pgd_t *pgdp)
>>          }
>>   #endif
>>
>> +       kfence_pool = arm64_kfence_alloc_pool();
>> +       if (kfence_pool)
>> +               memblock_mark_nomap(kfence_pool, KFENCE_POOL_SIZE);
>> +
>>          /* map all the memory banks */
>>          for_each_mem_range(i, &start, &end) {
>>                  if (start >= end)
>> @@ -608,6 +639,17 @@ static void __init map_mem(pgd_t *pgdp)
>>                  }
>>          }
>>   #endif
>> +
>> +       /* Kfence pool needs page-level mapping */
>> +       if (kfence_pool) {
>> +               __map_memblock(pgdp, kfence_pool,
>> +                       kfence_pool + KFENCE_POOL_SIZE,
>> +                       pgprot_tagged(PAGE_KERNEL),
>> +                       NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS);
>> +               memblock_clear_nomap(kfence_pool, KFENCE_POOL_SIZE);
>> +               /* kfence_pool really mapped now */
>> +               kfence_set_pool(kfence_pool);
>> +       }
>>   }
>>
>>   void mark_rodata_ro(void)
>> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
>> index 79dd201..25e4a983 100644
>> --- a/arch/arm64/mm/pageattr.c
>> +++ b/arch/arm64/mm/pageattr.c
>> @@ -7,6 +7,7 @@
>>   #include <linux/module.h>
>>   #include <linux/sched.h>
>>   #include <linux/vmalloc.h>
>> +#include <linux/kfence.h>
>>
>>   #include <asm/cacheflush.h>
>>   #include <asm/set_memory.h>
>> @@ -22,12 +23,15 @@ bool rodata_full __ro_after_init = IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED
>>   bool can_set_direct_map(void)
>>   {
>>          /*
>> -        * rodata_full, DEBUG_PAGEALLOC and KFENCE require linear map to be
>> +        * rodata_full and DEBUG_PAGEALLOC require linear map to be
>>           * mapped at page granularity, so that it is possible to
>>           * protect/unprotect single pages.
>> +        *
>> +        * Kfence pool requires page granularity mapping also if we init it
>> +        * late.
>>           */
>>          return (rodata_enabled && rodata_full) || debug_pagealloc_enabled() ||
>> -               IS_ENABLED(CONFIG_KFENCE);
>> +           (IS_ENABLED(CONFIG_KFENCE) && !kfence_sample_interval);
> 
> If you're struggling with kfence_sample_interval not existing if
> !CONFIG_KFENCE, this is one of the occasions where it'd be perfectly
> fine to write:
> 
> bool can_set_direct_map(void) {
> #ifdef CONFIG_KFENCE
>      /* ... your comment here ...*/
>      if (!kfence_sample_interval)
>          return true;
> }
> #endif
>       return .........
> }
> 
>>   }
>>
The can_set_direct_map() could be called anytime, eg, memory add,
vmalloc, and this will make different state of can_set_direct_map()
if kfence is re-enabled, I think that we need a new value to check 
whether or not the early kfence_pool is initialized.




^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v6] mm,kfence: decouple kfence from page granularity mapping judgement
@ 2023-03-13 14:42     ` Kefeng Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Kefeng Wang @ 2023-03-13 14:42 UTC (permalink / raw)
  To: Marco Elver, Zhenhua Huang
  Cc: catalin.marinas, will, glider, dvyukov, akpm, robin.murphy,
	mark.rutland, jianyong.wu, james.morse, linux-arm-kernel,
	kasan-dev, linux-mm, quic_pkondeti, quic_guptap, quic_tingweiz



On 2023/3/13 21:00, Marco Elver wrote:
> On Mon, 13 Mar 2023 at 12:57, Zhenhua Huang <quic_zhenhuah@quicinc.com> wrote:
>>
>> Kfence only needs its pool to be mapped as page granularity, if it is
>> inited early. Previous judgement was a bit over protected. From [1], Mark
>> suggested to "just map the KFENCE region a page granularity". So I
>> decouple it from judgement and do page granularity mapping for kfence
>> pool only. Need to be noticed that late init of kfence pool still requires
>> page granularity mapping.
>>
>> Page granularity mapping in theory cost more(2M per 1GB) memory on arm64
>> platform. Like what I've tested on QEMU(emulated 1GB RAM) with
>> gki_defconfig, also turning off rodata protection:
>> Before:
>> [root@liebao ]# cat /proc/meminfo
>> MemTotal:         999484 kB
>> After:
>> [root@liebao ]# cat /proc/meminfo
>> MemTotal:        1001480 kB
>>
>> To implement this, also relocate the kfence pool allocation before the
>> linear mapping setting up, arm64_kfence_alloc_pool is to allocate phys
>> addr, __kfence_pool is to be set after linear mapping set up.
>>
>> LINK: [1] https://lore.kernel.org/linux-arm-kernel/Y+IsdrvDNILA59UN@FVFF77S0Q05N/
>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>> Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com>
>> ---
>>   arch/arm64/mm/mmu.c      | 42 ++++++++++++++++++++++++++++++++++++++++++
>>   arch/arm64/mm/pageattr.c |  8 ++++++--
>>   include/linux/kfence.h   | 10 ++++++++++
>>   mm/kfence/core.c         |  9 +++++++++
>>   4 files changed, 67 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index 6f9d889..ca5c932 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -24,6 +24,7 @@
>>   #include <linux/mm.h>
>>   #include <linux/vmalloc.h>
>>   #include <linux/set_memory.h>
>> +#include <linux/kfence.h>
>>
>>   #include <asm/barrier.h>
>>   #include <asm/cputype.h>
>> @@ -525,6 +526,31 @@ static int __init enable_crash_mem_map(char *arg)
>>   }
>>   early_param("crashkernel", enable_crash_mem_map);
>>
>> +#ifdef CONFIG_KFENCE
>> +
>> +static phys_addr_t arm64_kfence_alloc_pool(void)
>> +{
>> +       phys_addr_t kfence_pool;
>> +
>> +       if (!kfence_sample_interval)
>> +               return 0;
>> +
>> +       kfence_pool = memblock_phys_alloc(KFENCE_POOL_SIZE, PAGE_SIZE);
>> +       if (!kfence_pool)
>> +               pr_err("failed to allocate kfence pool\n");
>> +
>> +       return kfence_pool;
>> +}
>> +
>> +#else
>> +
>> +static phys_addr_t arm64_kfence_alloc_pool(void)
>> +{
>> +       return 0;
>> +}
>> +
>> +#endif
>> +
>>   static void __init map_mem(pgd_t *pgdp)
>>   {
>>          static const u64 direct_map_end = _PAGE_END(VA_BITS_MIN);
>> @@ -532,6 +558,7 @@ static void __init map_mem(pgd_t *pgdp)
>>          phys_addr_t kernel_end = __pa_symbol(__init_begin);
>>          phys_addr_t start, end;
>>          int flags = NO_EXEC_MAPPINGS;
>> +       phys_addr_t kfence_pool;
>>          u64 i;
>>
>>          /*
>> @@ -564,6 +591,10 @@ static void __init map_mem(pgd_t *pgdp)
>>          }
>>   #endif
>>
>> +       kfence_pool = arm64_kfence_alloc_pool();
>> +       if (kfence_pool)
>> +               memblock_mark_nomap(kfence_pool, KFENCE_POOL_SIZE);
>> +
>>          /* map all the memory banks */
>>          for_each_mem_range(i, &start, &end) {
>>                  if (start >= end)
>> @@ -608,6 +639,17 @@ static void __init map_mem(pgd_t *pgdp)
>>                  }
>>          }
>>   #endif
>> +
>> +       /* Kfence pool needs page-level mapping */
>> +       if (kfence_pool) {
>> +               __map_memblock(pgdp, kfence_pool,
>> +                       kfence_pool + KFENCE_POOL_SIZE,
>> +                       pgprot_tagged(PAGE_KERNEL),
>> +                       NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS);
>> +               memblock_clear_nomap(kfence_pool, KFENCE_POOL_SIZE);
>> +               /* kfence_pool really mapped now */
>> +               kfence_set_pool(kfence_pool);
>> +       }
>>   }
>>
>>   void mark_rodata_ro(void)
>> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
>> index 79dd201..25e4a983 100644
>> --- a/arch/arm64/mm/pageattr.c
>> +++ b/arch/arm64/mm/pageattr.c
>> @@ -7,6 +7,7 @@
>>   #include <linux/module.h>
>>   #include <linux/sched.h>
>>   #include <linux/vmalloc.h>
>> +#include <linux/kfence.h>
>>
>>   #include <asm/cacheflush.h>
>>   #include <asm/set_memory.h>
>> @@ -22,12 +23,15 @@ bool rodata_full __ro_after_init = IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED
>>   bool can_set_direct_map(void)
>>   {
>>          /*
>> -        * rodata_full, DEBUG_PAGEALLOC and KFENCE require linear map to be
>> +        * rodata_full and DEBUG_PAGEALLOC require linear map to be
>>           * mapped at page granularity, so that it is possible to
>>           * protect/unprotect single pages.
>> +        *
>> +        * Kfence pool requires page granularity mapping also if we init it
>> +        * late.
>>           */
>>          return (rodata_enabled && rodata_full) || debug_pagealloc_enabled() ||
>> -               IS_ENABLED(CONFIG_KFENCE);
>> +           (IS_ENABLED(CONFIG_KFENCE) && !kfence_sample_interval);
> 
> If you're struggling with kfence_sample_interval not existing if
> !CONFIG_KFENCE, this is one of the occasions where it'd be perfectly
> fine to write:
> 
> bool can_set_direct_map(void) {
> #ifdef CONFIG_KFENCE
>      /* ... your comment here ...*/
>      if (!kfence_sample_interval)
>          return true;
> }
> #endif
>       return .........
> }
> 
>>   }
>>
The can_set_direct_map() could be called anytime, eg, memory add,
vmalloc, and this will make different state of can_set_direct_map()
if kfence is re-enabled, I think that we need a new value to check 
whether or not the early kfence_pool is initialized.



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v6] mm,kfence: decouple kfence from page granularity mapping judgement
  2023-03-13 14:42     ` Kefeng Wang
@ 2023-03-14  5:26       ` Zhenhua Huang
  -1 siblings, 0 replies; 8+ messages in thread
From: Zhenhua Huang @ 2023-03-14  5:26 UTC (permalink / raw)
  To: Kefeng Wang, Marco Elver
  Cc: catalin.marinas, will, glider, dvyukov, akpm, robin.murphy,
	mark.rutland, jianyong.wu, james.morse, linux-arm-kernel,
	kasan-dev, linux-mm, quic_pkondeti, quic_guptap, quic_tingweiz



On 2023/3/13 22:42, Kefeng Wang wrote:
> 
> 
> On 2023/3/13 21:00, Marco Elver wrote:
>> On Mon, 13 Mar 2023 at 12:57, Zhenhua Huang 
>> <quic_zhenhuah@quicinc.com> wrote:
>>>
>>> Kfence only needs its pool to be mapped as page granularity, if it is
>>> inited early. Previous judgement was a bit over protected. From [1], 
>>> Mark
>>> suggested to "just map the KFENCE region a page granularity". So I
>>> decouple it from judgement and do page granularity mapping for kfence
>>> pool only. Need to be noticed that late init of kfence pool still 
>>> requires
>>> page granularity mapping.
>>>
>>> Page granularity mapping in theory cost more(2M per 1GB) memory on arm64
>>> platform. Like what I've tested on QEMU(emulated 1GB RAM) with
>>> gki_defconfig, also turning off rodata protection:
>>> Before:
>>> [root@liebao ]# cat /proc/meminfo
>>> MemTotal:         999484 kB
>>> After:
>>> [root@liebao ]# cat /proc/meminfo
>>> MemTotal:        1001480 kB
>>>
>>> To implement this, also relocate the kfence pool allocation before the
>>> linear mapping setting up, arm64_kfence_alloc_pool is to allocate phys
>>> addr, __kfence_pool is to be set after linear mapping set up.
>>>
>>> LINK: [1] 
>>> https://lore.kernel.org/linux-arm-kernel/Y+IsdrvDNILA59UN@FVFF77S0Q05N/
>>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>>> Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com>
>>> ---
>>>   arch/arm64/mm/mmu.c      | 42 
>>> ++++++++++++++++++++++++++++++++++++++++++
>>>   arch/arm64/mm/pageattr.c |  8 ++++++--
>>>   include/linux/kfence.h   | 10 ++++++++++
>>>   mm/kfence/core.c         |  9 +++++++++
>>>   4 files changed, 67 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>>> index 6f9d889..ca5c932 100644
>>> --- a/arch/arm64/mm/mmu.c
>>> +++ b/arch/arm64/mm/mmu.c
>>> @@ -24,6 +24,7 @@
>>>   #include <linux/mm.h>
>>>   #include <linux/vmalloc.h>
>>>   #include <linux/set_memory.h>
>>> +#include <linux/kfence.h>
>>>
>>>   #include <asm/barrier.h>
>>>   #include <asm/cputype.h>
>>> @@ -525,6 +526,31 @@ static int __init enable_crash_mem_map(char *arg)
>>>   }
>>>   early_param("crashkernel", enable_crash_mem_map);
>>>
>>> +#ifdef CONFIG_KFENCE
>>> +
>>> +static phys_addr_t arm64_kfence_alloc_pool(void)
>>> +{
>>> +       phys_addr_t kfence_pool;
>>> +
>>> +       if (!kfence_sample_interval)
>>> +               return 0;
>>> +
>>> +       kfence_pool = memblock_phys_alloc(KFENCE_POOL_SIZE, PAGE_SIZE);
>>> +       if (!kfence_pool)
>>> +               pr_err("failed to allocate kfence pool\n");
>>> +
>>> +       return kfence_pool;
>>> +}
>>> +
>>> +#else
>>> +
>>> +static phys_addr_t arm64_kfence_alloc_pool(void)
>>> +{
>>> +       return 0;
>>> +}
>>> +
>>> +#endif
>>> +
>>>   static void __init map_mem(pgd_t *pgdp)
>>>   {
>>>          static const u64 direct_map_end = _PAGE_END(VA_BITS_MIN);
>>> @@ -532,6 +558,7 @@ static void __init map_mem(pgd_t *pgdp)
>>>          phys_addr_t kernel_end = __pa_symbol(__init_begin);
>>>          phys_addr_t start, end;
>>>          int flags = NO_EXEC_MAPPINGS;
>>> +       phys_addr_t kfence_pool;
>>>          u64 i;
>>>
>>>          /*
>>> @@ -564,6 +591,10 @@ static void __init map_mem(pgd_t *pgdp)
>>>          }
>>>   #endif
>>>
>>> +       kfence_pool = arm64_kfence_alloc_pool();
>>> +       if (kfence_pool)
>>> +               memblock_mark_nomap(kfence_pool, KFENCE_POOL_SIZE);
>>> +
>>>          /* map all the memory banks */
>>>          for_each_mem_range(i, &start, &end) {
>>>                  if (start >= end)
>>> @@ -608,6 +639,17 @@ static void __init map_mem(pgd_t *pgdp)
>>>                  }
>>>          }
>>>   #endif
>>> +
>>> +       /* Kfence pool needs page-level mapping */
>>> +       if (kfence_pool) {
>>> +               __map_memblock(pgdp, kfence_pool,
>>> +                       kfence_pool + KFENCE_POOL_SIZE,
>>> +                       pgprot_tagged(PAGE_KERNEL),
>>> +                       NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS);
>>> +               memblock_clear_nomap(kfence_pool, KFENCE_POOL_SIZE);
>>> +               /* kfence_pool really mapped now */
>>> +               kfence_set_pool(kfence_pool);
>>> +       }
>>>   }
>>>
>>>   void mark_rodata_ro(void)
>>> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
>>> index 79dd201..25e4a983 100644
>>> --- a/arch/arm64/mm/pageattr.c
>>> +++ b/arch/arm64/mm/pageattr.c
>>> @@ -7,6 +7,7 @@
>>>   #include <linux/module.h>
>>>   #include <linux/sched.h>
>>>   #include <linux/vmalloc.h>
>>> +#include <linux/kfence.h>
>>>
>>>   #include <asm/cacheflush.h>
>>>   #include <asm/set_memory.h>
>>> @@ -22,12 +23,15 @@ bool rodata_full __ro_after_init = 
>>> IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED
>>>   bool can_set_direct_map(void)
>>>   {
>>>          /*
>>> -        * rodata_full, DEBUG_PAGEALLOC and KFENCE require linear map 
>>> to be
>>> +        * rodata_full and DEBUG_PAGEALLOC require linear map to be
>>>           * mapped at page granularity, so that it is possible to
>>>           * protect/unprotect single pages.
>>> +        *
>>> +        * Kfence pool requires page granularity mapping also if we 
>>> init it
>>> +        * late.
>>>           */
>>>          return (rodata_enabled && rodata_full) || 
>>> debug_pagealloc_enabled() ||
>>> -               IS_ENABLED(CONFIG_KFENCE);
>>> +           (IS_ENABLED(CONFIG_KFENCE) && !kfence_sample_interval);
>>
>> If you're struggling with kfence_sample_interval not existing if
>> !CONFIG_KFENCE, this is one of the occasions where it'd be perfectly
>> fine to write:
>>
>> bool can_set_direct_map(void) {
>> #ifdef CONFIG_KFENCE
>>      /* ... your comment here ...*/
>>      if (!kfence_sample_interval)
>>          return true;
>> }
>> #endif
>>       return .........
>> }
>>
>>>   }
>>>
> The can_set_direct_map() could be called anytime, eg, memory add,
> vmalloc, and this will make different state of can_set_direct_map()
> if kfence is re-enabled, I think that we need a new value to check 
> whether or not the early kfence_pool is initialized.

Many thanks, Kefeng and Marco for your careful review. Agree, 
kfence_sample_interval can be modified in a few ways and we can't use it 
in can_set_direct_map().

To be honest, previously I wanted to allocate kfence pool early always 
but it seems breaks the flexibility that b33f778bba5e ("kfence: alloc 
kfence_pool after system startup") introduced.

Now I prefer to introduce one global variable early_kfence_pool to 
indicate if kfence_pool is initialized early, then can_set_direct_map() 
should be easy and clear to handle: just add "(IS_ENABLED(CONFIG_KFENCE) 
&& !early_kfence_pool)" for the case of possibility we may init kfence 
pool later. The naming of early_kfence_pool also can well expressed what 
we're doing :)

How about your idea? I will update a new patchset.

> 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v6] mm,kfence: decouple kfence from page granularity mapping judgement
@ 2023-03-14  5:26       ` Zhenhua Huang
  0 siblings, 0 replies; 8+ messages in thread
From: Zhenhua Huang @ 2023-03-14  5:26 UTC (permalink / raw)
  To: Kefeng Wang, Marco Elver
  Cc: catalin.marinas, will, glider, dvyukov, akpm, robin.murphy,
	mark.rutland, jianyong.wu, james.morse, linux-arm-kernel,
	kasan-dev, linux-mm, quic_pkondeti, quic_guptap, quic_tingweiz



On 2023/3/13 22:42, Kefeng Wang wrote:
> 
> 
> On 2023/3/13 21:00, Marco Elver wrote:
>> On Mon, 13 Mar 2023 at 12:57, Zhenhua Huang 
>> <quic_zhenhuah@quicinc.com> wrote:
>>>
>>> Kfence only needs its pool to be mapped as page granularity, if it is
>>> inited early. Previous judgement was a bit over protected. From [1], 
>>> Mark
>>> suggested to "just map the KFENCE region a page granularity". So I
>>> decouple it from judgement and do page granularity mapping for kfence
>>> pool only. Need to be noticed that late init of kfence pool still 
>>> requires
>>> page granularity mapping.
>>>
>>> Page granularity mapping in theory cost more(2M per 1GB) memory on arm64
>>> platform. Like what I've tested on QEMU(emulated 1GB RAM) with
>>> gki_defconfig, also turning off rodata protection:
>>> Before:
>>> [root@liebao ]# cat /proc/meminfo
>>> MemTotal:         999484 kB
>>> After:
>>> [root@liebao ]# cat /proc/meminfo
>>> MemTotal:        1001480 kB
>>>
>>> To implement this, also relocate the kfence pool allocation before the
>>> linear mapping setting up, arm64_kfence_alloc_pool is to allocate phys
>>> addr, __kfence_pool is to be set after linear mapping set up.
>>>
>>> LINK: [1] 
>>> https://lore.kernel.org/linux-arm-kernel/Y+IsdrvDNILA59UN@FVFF77S0Q05N/
>>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>>> Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com>
>>> ---
>>>   arch/arm64/mm/mmu.c      | 42 
>>> ++++++++++++++++++++++++++++++++++++++++++
>>>   arch/arm64/mm/pageattr.c |  8 ++++++--
>>>   include/linux/kfence.h   | 10 ++++++++++
>>>   mm/kfence/core.c         |  9 +++++++++
>>>   4 files changed, 67 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>>> index 6f9d889..ca5c932 100644
>>> --- a/arch/arm64/mm/mmu.c
>>> +++ b/arch/arm64/mm/mmu.c
>>> @@ -24,6 +24,7 @@
>>>   #include <linux/mm.h>
>>>   #include <linux/vmalloc.h>
>>>   #include <linux/set_memory.h>
>>> +#include <linux/kfence.h>
>>>
>>>   #include <asm/barrier.h>
>>>   #include <asm/cputype.h>
>>> @@ -525,6 +526,31 @@ static int __init enable_crash_mem_map(char *arg)
>>>   }
>>>   early_param("crashkernel", enable_crash_mem_map);
>>>
>>> +#ifdef CONFIG_KFENCE
>>> +
>>> +static phys_addr_t arm64_kfence_alloc_pool(void)
>>> +{
>>> +       phys_addr_t kfence_pool;
>>> +
>>> +       if (!kfence_sample_interval)
>>> +               return 0;
>>> +
>>> +       kfence_pool = memblock_phys_alloc(KFENCE_POOL_SIZE, PAGE_SIZE);
>>> +       if (!kfence_pool)
>>> +               pr_err("failed to allocate kfence pool\n");
>>> +
>>> +       return kfence_pool;
>>> +}
>>> +
>>> +#else
>>> +
>>> +static phys_addr_t arm64_kfence_alloc_pool(void)
>>> +{
>>> +       return 0;
>>> +}
>>> +
>>> +#endif
>>> +
>>>   static void __init map_mem(pgd_t *pgdp)
>>>   {
>>>          static const u64 direct_map_end = _PAGE_END(VA_BITS_MIN);
>>> @@ -532,6 +558,7 @@ static void __init map_mem(pgd_t *pgdp)
>>>          phys_addr_t kernel_end = __pa_symbol(__init_begin);
>>>          phys_addr_t start, end;
>>>          int flags = NO_EXEC_MAPPINGS;
>>> +       phys_addr_t kfence_pool;
>>>          u64 i;
>>>
>>>          /*
>>> @@ -564,6 +591,10 @@ static void __init map_mem(pgd_t *pgdp)
>>>          }
>>>   #endif
>>>
>>> +       kfence_pool = arm64_kfence_alloc_pool();
>>> +       if (kfence_pool)
>>> +               memblock_mark_nomap(kfence_pool, KFENCE_POOL_SIZE);
>>> +
>>>          /* map all the memory banks */
>>>          for_each_mem_range(i, &start, &end) {
>>>                  if (start >= end)
>>> @@ -608,6 +639,17 @@ static void __init map_mem(pgd_t *pgdp)
>>>                  }
>>>          }
>>>   #endif
>>> +
>>> +       /* Kfence pool needs page-level mapping */
>>> +       if (kfence_pool) {
>>> +               __map_memblock(pgdp, kfence_pool,
>>> +                       kfence_pool + KFENCE_POOL_SIZE,
>>> +                       pgprot_tagged(PAGE_KERNEL),
>>> +                       NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS);
>>> +               memblock_clear_nomap(kfence_pool, KFENCE_POOL_SIZE);
>>> +               /* kfence_pool really mapped now */
>>> +               kfence_set_pool(kfence_pool);
>>> +       }
>>>   }
>>>
>>>   void mark_rodata_ro(void)
>>> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
>>> index 79dd201..25e4a983 100644
>>> --- a/arch/arm64/mm/pageattr.c
>>> +++ b/arch/arm64/mm/pageattr.c
>>> @@ -7,6 +7,7 @@
>>>   #include <linux/module.h>
>>>   #include <linux/sched.h>
>>>   #include <linux/vmalloc.h>
>>> +#include <linux/kfence.h>
>>>
>>>   #include <asm/cacheflush.h>
>>>   #include <asm/set_memory.h>
>>> @@ -22,12 +23,15 @@ bool rodata_full __ro_after_init = 
>>> IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED
>>>   bool can_set_direct_map(void)
>>>   {
>>>          /*
>>> -        * rodata_full, DEBUG_PAGEALLOC and KFENCE require linear map 
>>> to be
>>> +        * rodata_full and DEBUG_PAGEALLOC require linear map to be
>>>           * mapped at page granularity, so that it is possible to
>>>           * protect/unprotect single pages.
>>> +        *
>>> +        * Kfence pool requires page granularity mapping also if we 
>>> init it
>>> +        * late.
>>>           */
>>>          return (rodata_enabled && rodata_full) || 
>>> debug_pagealloc_enabled() ||
>>> -               IS_ENABLED(CONFIG_KFENCE);
>>> +           (IS_ENABLED(CONFIG_KFENCE) && !kfence_sample_interval);
>>
>> If you're struggling with kfence_sample_interval not existing if
>> !CONFIG_KFENCE, this is one of the occasions where it'd be perfectly
>> fine to write:
>>
>> bool can_set_direct_map(void) {
>> #ifdef CONFIG_KFENCE
>>      /* ... your comment here ...*/
>>      if (!kfence_sample_interval)
>>          return true;
>> }
>> #endif
>>       return .........
>> }
>>
>>>   }
>>>
> The can_set_direct_map() could be called anytime, eg, memory add,
> vmalloc, and this will make different state of can_set_direct_map()
> if kfence is re-enabled, I think that we need a new value to check 
> whether or not the early kfence_pool is initialized.

Many thanks, Kefeng and Marco for your careful review. Agree, 
kfence_sample_interval can be modified in a few ways and we can't use it 
in can_set_direct_map().

To be honest, previously I wanted to allocate kfence pool early always 
but it seems breaks the flexibility that b33f778bba5e ("kfence: alloc 
kfence_pool after system startup") introduced.

Now I prefer to introduce one global variable early_kfence_pool to 
indicate if kfence_pool is initialized early, then can_set_direct_map() 
should be easy and clear to handle: just add "(IS_ENABLED(CONFIG_KFENCE) 
&& !early_kfence_pool)" for the case of possibility we may init kfence 
pool later. The naming of early_kfence_pool also can well expressed what 
we're doing :)

How about your idea? I will update a new patchset.

> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-03-14  5:28 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-13 11:57 [PATCH v6] mm,kfence: decouple kfence from page granularity mapping judgement Zhenhua Huang
2023-03-13 11:57 ` Zhenhua Huang
2023-03-13 13:00 ` Marco Elver
2023-03-13 13:00   ` Marco Elver
2023-03-13 14:42   ` Kefeng Wang
2023-03-13 14:42     ` Kefeng Wang
2023-03-14  5:26     ` Zhenhua Huang
2023-03-14  5:26       ` Zhenhua Huang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.