Re: [PATCH v4 2/6] riscv: Rework kasan population functions

[Björn Töpel]

Alexandre Ghiti <alexghiti@xxxxxxxxxxxx> writes:

> Our previous kasan population implementation used to have the final kasan
> shadow region mapped with kasan_early_shadow_page, because we did not clean
> the early mapping and then we had to populate the kasan region "in-place"
> which made the code cumbersome.
>
> So now we clear the early mapping, establish a temporary mapping while we
> populate the kasan shadow region with just the kernel regions that will
> be used.
>
> This new version uses the "generic" way of going through a page table
> that may be folded at runtime (avoid the XXX_next macros).
>
> It was tested with outline instrumentation on an Ubuntu kernel
> configuration successfully.
>
> Signed-off-by: Alexandre Ghiti <alexghiti@xxxxxxxxxxxx>

(One minor nit, that can be addressed later.)

Reviewed-by: Björn Töpel <bjorn@xxxxxxxxxxxx>

>  arch/riscv/mm/kasan_init.c | 361 +++++++++++++++++++------------------
>  1 file changed, 183 insertions(+), 178 deletions(-)


> @@ -482,7 +437,37 @@ static void __init kasan_shallow_populate(void *start, void *end)
>  	unsigned long vend = PAGE_ALIGN((unsigned long)end);
>  
>  	kasan_shallow_populate_pgd(vaddr, vend);
> -	local_flush_tlb_all();
> +}
> +
> +static void create_tmp_mapping(void)
> +{
> +	void *ptr;
> +	p4d_t *base_p4d;
> +
> +	/*
> +	 * We need to clean the early mapping: this is hard to achieve "in-place",
> +	 * so install a temporary mapping like arm64 and x86 do.
> +	 */
> +	memcpy(tmp_pg_dir, swapper_pg_dir, sizeof(pgd_t) * PTRS_PER_PGD);
> +
> +	/* Copy the last p4d since it is shared with the kernel mapping. */
> +	if (pgtable_l5_enabled) {
> +		ptr = (p4d_t *)pgd_page_vaddr(*pgd_offset_k(KASAN_SHADOW_END));
> +		memcpy(tmp_p4d, ptr, sizeof(p4d_t) * PTRS_PER_P4D);
> +		set_pgd(&tmp_pg_dir[pgd_index(KASAN_SHADOW_END)],
> +			pfn_pgd(PFN_DOWN(__pa(tmp_p4d)), PAGE_TABLE));
> +		base_p4d = tmp_p4d;
> +	} else {
> +		base_p4d = (p4d_t *)tmp_pg_dir;
> +	}
> +
> +	/* Copy the last pud since it is shared with the kernel mapping. */
> +	if (pgtable_l4_enabled) {
> +		ptr = (pud_t *)p4d_page_vaddr(*(base_p4d + p4d_index(KASAN_SHADOW_END)));
> +		memcpy(tmp_pud, ptr, sizeof(pud_t) * PTRS_PER_PUD);
> +		set_p4d(&base_p4d[p4d_index(KASAN_SHADOW_END)],
> +			pfn_p4d(PFN_DOWN(__pa(tmp_pud)), PAGE_TABLE));
> +	}
>  }
>  
>  void __init kasan_init(void)
> @@ -490,10 +475,27 @@ void __init kasan_init(void)
>  	phys_addr_t p_start, p_end;
>  	u64 i;
>  
> -	if (IS_ENABLED(CONFIG_KASAN_VMALLOC))
> +	create_tmp_mapping();
> +	csr_write(CSR_SATP, PFN_DOWN(__pa(tmp_pg_dir)) | satp_mode);

Nit: Maybe add a comment, why the sfence.vma is *not* required here. I
tripped over it.


Björn