Re: [PATCH v8] mm,kfence: decouple kfence from page granularity mapping judgement

[Pavan Kondeti <quic_pkondeti@quicinc.com>]

On Tue, Mar 14, 2023 at 03:05:02PM +0800, Zhenhua Huang 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/include/asm/kfence.h |  2 ++
>  arch/arm64/mm/mmu.c             | 44 +++++++++++++++++++++++++++++++++++++++++
>  arch/arm64/mm/pageattr.c        |  9 +++++++--
>  include/linux/kfence.h          |  8 ++++++++
>  mm/kfence/core.c                |  9 +++++++++
>  5 files changed, 70 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kfence.h b/arch/arm64/include/asm/kfence.h
> index aa855c6..f1f9ca2d 100644
> --- a/arch/arm64/include/asm/kfence.h
> +++ b/arch/arm64/include/asm/kfence.h
> @@ -10,6 +10,8 @@
>  
>  #include <asm/set_memory.h>
>  
> +extern phys_addr_t early_kfence_pool;
> +
>  static inline bool arch_kfence_init_pool(void) { return true; }
>  
>  static inline bool kfence_protect_page(unsigned long addr, bool protect)
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 6f9d889..7fbf2ed 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>
> @@ -38,6 +39,7 @@
>  #include <asm/ptdump.h>
>  #include <asm/tlbflush.h>
>  #include <asm/pgalloc.h>
> +#include <asm/kfence.h>
>  
>  #define NO_BLOCK_MAPPINGS	BIT(0)
>  #define NO_CONT_MAPPINGS	BIT(1)
> @@ -525,6 +527,33 @@ 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;
> +

Are you sure that kernel commandline param are processed this early?
AFAICS, start_kernel()->parse_args() process the kernel arguments. We
are here before that. without your patch, mm_init() which takes care of
allocating kfence memory is called after parse_args().

Can you check your patch with kfence.sample_interval=0 appended to
kernel commandline?

> +	kfence_pool = memblock_phys_alloc(KFENCE_POOL_SIZE, PAGE_SIZE);
> +	if (!kfence_pool)
> +		pr_err("failed to allocate kfence pool\n");
> +
For whatever reason, if this allocation fails, what should be done? We
end up not calling kfence_set_pool(). kfence_alloc_pool() is going to
attempt allocation again but we did not setup page granularity. That
means, we are enabling KFENCE without meeting pre-conditions. Can you
check this?

> +	return kfence_pool;
> +}
> +

Thanks,
Pavan