diff mbox series

[-next,3/3] kasan: arm64: Fix pcpu_page_first_chunk crash with KASAN_VMALLOC

Message ID 20210705111453.164230-4-wangkefeng.wang@huawei.com (mailing list archive)
State New
Headers show
Series arm64: support page mapping percpu first chunk allocator | expand

Commit Message

Kefeng Wang July 5, 2021, 11:14 a.m. UTC
With KASAN_VMALLOC and NEED_PER_CPU_PAGE_FIRST_CHUNK, it crashs,

Unable to handle kernel paging request at virtual address ffff7000028f2000
...
swapper pgtable: 64k pages, 48-bit VAs, pgdp=0000000042440000
[ffff7000028f2000] pgd=000000063e7c0003, p4d=000000063e7c0003, pud=000000063e7c0003, pmd=000000063e7b0003, pte=0000000000000000
Internal error: Oops: 96000007 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted 5.13.0-rc4-00003-gc6e6e28f3f30-dirty #62
Hardware name: linux,dummy-virt (DT)
pstate: 200000c5 (nzCv daIF -PAN -UAO -TCO BTYPE=--)
pc : kasan_check_range+0x90/0x1a0
lr : memcpy+0x88/0xf4
sp : ffff80001378fe20
...
Call trace:
 kasan_check_range+0x90/0x1a0
 pcpu_page_first_chunk+0x3f0/0x568
 setup_per_cpu_areas+0xb8/0x184
 start_kernel+0x8c/0x328

The vm area used in vm_area_register_early() has no kasan shadow memory,
Let's add a new kasan_populate_early_vm_area_shadow() function to populate
the vm area shadow memory to fix the issue.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 arch/arm64/mm/kasan_init.c | 18 ++++++++++++++++++
 include/linux/kasan.h      |  2 ++
 mm/kasan/init.c            |  5 +++++
 mm/vmalloc.c               |  1 +
 4 files changed, 26 insertions(+)

Comments

kernel test robot July 5, 2021, 2:10 p.m. UTC | #1
Hi Kefeng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20210701]

url:    https://github.com/0day-ci/linux/commits/Kefeng-Wang/arm64-support-page-mapping-percpu-first-chunk-allocator/20210705-190907
base:    fb0ca446157a86b75502c1636b0d81e642fe6bf1
config: i386-randconfig-a015-20210705 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/5f6b5a402ed3e390563ddbddf12973470fd4886d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Kefeng-Wang/arm64-support-page-mapping-percpu-first-chunk-allocator/20210705-190907
        git checkout 5f6b5a402ed3e390563ddbddf12973470fd4886d
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   mm/vmalloc.c: In function 'vm_area_register_early':
>> mm/vmalloc.c:2252:2: error: implicit declaration of function 'kasan_populate_early_vm_area_shadow' [-Werror=implicit-function-declaration]
    2252 |  kasan_populate_early_vm_area_shadow(vm->addr, vm->size);
         |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/kasan_populate_early_vm_area_shadow +2252 mm/vmalloc.c

  2226	
  2227	/**
  2228	 * vm_area_register_early - register vmap area early during boot
  2229	 * @vm: vm_struct to register
  2230	 * @align: requested alignment
  2231	 *
  2232	 * This function is used to register kernel vm area before
  2233	 * vmalloc_init() is called.  @vm->size and @vm->flags should contain
  2234	 * proper values on entry and other fields should be zero.  On return,
  2235	 * vm->addr contains the allocated address.
  2236	 *
  2237	 * DO NOT USE THIS FUNCTION UNLESS YOU KNOW WHAT YOU'RE DOING.
  2238	 */
  2239	void __init vm_area_register_early(struct vm_struct *vm, size_t align)
  2240	{
  2241		unsigned long vm_start = VMALLOC_START;
  2242		struct vm_struct *tmp;
  2243		unsigned long addr;
  2244	
  2245		for (tmp = vmlist; tmp; tmp = tmp->next)
  2246			vm_start = (unsigned long)tmp->addr + tmp->size;
  2247	
  2248		addr = ALIGN(vm_start, align);
  2249		vm->addr = (void *)addr;
  2250	
  2251		vm_area_add_early(vm);
> 2252		kasan_populate_early_vm_area_shadow(vm->addr, vm->size);
  2253	}
  2254	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Marco Elver July 5, 2021, 3:04 p.m. UTC | #2
On Mon, Jul 05, 2021 at 07:14PM +0800, Kefeng Wang wrote:
[...]
> +#ifdef CONFIG_KASAN_VMALLOC
> +void __init __weak kasan_populate_early_vm_area_shadow(void *start,
> +						       unsigned long size)

This should probably not be __weak, otherwise you now have 2 __weak
functions.

> +{
> +	unsigned long shadow_start, shadow_end;
> +
> +	if (!is_vmalloc_or_module_addr(start))
> +		return;
> +
> +	shadow_start = (unsigned long)kasan_mem_to_shadow(start);
> +	shadow_start = ALIGN_DOWN(shadow_start, PAGE_SIZE);
> +	shadow_end = (unsigned long)kasan_mem_to_shadow(start + size);
> +	shadow_end = ALIGN(shadow_end, PAGE_SIZE);
> +	kasan_map_populate(shadow_start, shadow_end,
> +			   early_pfn_to_nid(virt_to_pfn(start)));
> +}
> +#endif

This function looks quite generic -- would any of this also apply to
other architectures? I see that ppc and sparc at least also define
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK.

>  void __init kasan_init(void)
>  {
>  	kasan_init_shadow();
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index 5310e217bd74..79d3895b0240 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -49,6 +49,8 @@ extern p4d_t kasan_early_shadow_p4d[MAX_PTRS_PER_P4D];
>  int kasan_populate_early_shadow(const void *shadow_start,
>  				const void *shadow_end);
>  
> +void kasan_populate_early_vm_area_shadow(void *start, unsigned long size);
> +
>  static inline void *kasan_mem_to_shadow(const void *addr)
>  {
>  	return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
> diff --git a/mm/kasan/init.c b/mm/kasan/init.c
> index cc64ed6858c6..d39577d088a1 100644
> --- a/mm/kasan/init.c
> +++ b/mm/kasan/init.c
> @@ -279,6 +279,11 @@ int __ref kasan_populate_early_shadow(const void *shadow_start,
>  	return 0;
>  }
>  
> +void __init __weak kasan_populate_early_vm_area_shadow(void *start,
> +						       unsigned long size)
> +{
> +}

I'm just wondering if this could be a generic function, perhaps with an
appropriate IS_ENABLED() check of a generic Kconfig option
(CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK ?) to short-circuit it, if it's
not only an arm64 problem.

But I haven't looked much further, so would appeal to you to either
confirm or reject this idea.

Thanks,
-- Marco
kernel test robot July 5, 2021, 5:15 p.m. UTC | #3
Hi Kefeng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20210701]

url:    https://github.com/0day-ci/linux/commits/Kefeng-Wang/arm64-support-page-mapping-percpu-first-chunk-allocator/20210705-190907
base:    fb0ca446157a86b75502c1636b0d81e642fe6bf1
config: powerpc-randconfig-r011-20210705 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 3f9bf9f42a9043e20c6d2a74dd4f47a90a7e2b41)
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 powerpc cross compiling tool for clang build
        # apt-get install binutils-powerpc-linux-gnu
        # https://github.com/0day-ci/linux/commit/5f6b5a402ed3e390563ddbddf12973470fd4886d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Kefeng-Wang/arm64-support-page-mapping-percpu-first-chunk-allocator/20210705-190907
        git checkout 5f6b5a402ed3e390563ddbddf12973470fd4886d
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> mm/vmalloc.c:2252:2: error: implicit declaration of function 'kasan_populate_early_vm_area_shadow' [-Werror,-Wimplicit-function-declaration]
           kasan_populate_early_vm_area_shadow(vm->addr, vm->size);
           ^
   1 error generated.


vim +/kasan_populate_early_vm_area_shadow +2252 mm/vmalloc.c

  2226	
  2227	/**
  2228	 * vm_area_register_early - register vmap area early during boot
  2229	 * @vm: vm_struct to register
  2230	 * @align: requested alignment
  2231	 *
  2232	 * This function is used to register kernel vm area before
  2233	 * vmalloc_init() is called.  @vm->size and @vm->flags should contain
  2234	 * proper values on entry and other fields should be zero.  On return,
  2235	 * vm->addr contains the allocated address.
  2236	 *
  2237	 * DO NOT USE THIS FUNCTION UNLESS YOU KNOW WHAT YOU'RE DOING.
  2238	 */
  2239	void __init vm_area_register_early(struct vm_struct *vm, size_t align)
  2240	{
  2241		unsigned long vm_start = VMALLOC_START;
  2242		struct vm_struct *tmp;
  2243		unsigned long addr;
  2244	
  2245		for (tmp = vmlist; tmp; tmp = tmp->next)
  2246			vm_start = (unsigned long)tmp->addr + tmp->size;
  2247	
  2248		addr = ALIGN(vm_start, align);
  2249		vm->addr = (void *)addr;
  2250	
  2251		vm_area_add_early(vm);
> 2252		kasan_populate_early_vm_area_shadow(vm->addr, vm->size);
  2253	}
  2254	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Daniel Axtens July 6, 2021, 12:04 a.m. UTC | #4
Hi,

Marco Elver <elver@google.com> writes:

> On Mon, Jul 05, 2021 at 07:14PM +0800, Kefeng Wang wrote:
> [...]
>> +#ifdef CONFIG_KASAN_VMALLOC
>> +void __init __weak kasan_populate_early_vm_area_shadow(void *start,
>> +						       unsigned long size)
>
> This should probably not be __weak, otherwise you now have 2 __weak
> functions.
>
>> +{
>> +	unsigned long shadow_start, shadow_end;
>> +
>> +	if (!is_vmalloc_or_module_addr(start))
>> +		return;
>> +
>> +	shadow_start = (unsigned long)kasan_mem_to_shadow(start);
>> +	shadow_start = ALIGN_DOWN(shadow_start, PAGE_SIZE);
>> +	shadow_end = (unsigned long)kasan_mem_to_shadow(start + size);
>> +	shadow_end = ALIGN(shadow_end, PAGE_SIZE);
>> +	kasan_map_populate(shadow_start, shadow_end,
>> +			   early_pfn_to_nid(virt_to_pfn(start)));
>> +}
>> +#endif
>
> This function looks quite generic -- would any of this also apply to
> other architectures? I see that ppc and sparc at least also define
> CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK.

So I checked with my latest KASAN ppc64 series and my code also breaks
in a very similar way if you boot with percpu_alloc=page. It's not
something I knew about or tested with before!

Unfortunately kasan_map_populate - despite having a very
generic-sounding name - is actually arm64 specific. I don't know if
kasan_populate_early_shadow (which is generic) would be able to fill the
role or not. If we could keep it generic that would be better.

It looks like arm64 does indeed populate the kasan_early_shadow_p{te,md..}
values, but I don't really understand what it's doing - is it possible
to use the generic kasan_populate_early_shadow on arm64?

If so, should we put the call inside of vm_area_register_early?

Kind regards,
Daniel

>
>>  void __init kasan_init(void)
>>  {
>>  	kasan_init_shadow();
>> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
>> index 5310e217bd74..79d3895b0240 100644
>> --- a/include/linux/kasan.h
>> +++ b/include/linux/kasan.h
>> @@ -49,6 +49,8 @@ extern p4d_t kasan_early_shadow_p4d[MAX_PTRS_PER_P4D];
>>  int kasan_populate_early_shadow(const void *shadow_start,
>>  				const void *shadow_end);
>>  
>> +void kasan_populate_early_vm_area_shadow(void *start, unsigned long size);
>> +
>>  static inline void *kasan_mem_to_shadow(const void *addr)
>>  {
>>  	return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
>> diff --git a/mm/kasan/init.c b/mm/kasan/init.c
>> index cc64ed6858c6..d39577d088a1 100644
>> --- a/mm/kasan/init.c
>> +++ b/mm/kasan/init.c
>> @@ -279,6 +279,11 @@ int __ref kasan_populate_early_shadow(const void *shadow_start,
>>  	return 0;
>>  }
>>  
>> +void __init __weak kasan_populate_early_vm_area_shadow(void *start,
>> +						       unsigned long size)
>> +{
>> +}
>
> I'm just wondering if this could be a generic function, perhaps with an
> appropriate IS_ENABLED() check of a generic Kconfig option
> (CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK ?) to short-circuit it, if it's
> not only an arm64 problem.
>
> But I haven't looked much further, so would appeal to you to either
> confirm or reject this idea.
>
> Thanks,
> -- Marco
Daniel Axtens July 6, 2021, 12:05 a.m. UTC | #5
> If so, should we put the call inside of vm_area_register_early?
Ah, we already do this. Sorry. My other questions remain.

Kind regards,
Daniel

>
> Kind regards,
> Daniel
>
>>
>>>  void __init kasan_init(void)
>>>  {
>>>  	kasan_init_shadow();
>>> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
>>> index 5310e217bd74..79d3895b0240 100644
>>> --- a/include/linux/kasan.h
>>> +++ b/include/linux/kasan.h
>>> @@ -49,6 +49,8 @@ extern p4d_t kasan_early_shadow_p4d[MAX_PTRS_PER_P4D];
>>>  int kasan_populate_early_shadow(const void *shadow_start,
>>>  				const void *shadow_end);
>>>  
>>> +void kasan_populate_early_vm_area_shadow(void *start, unsigned long size);
>>> +
>>>  static inline void *kasan_mem_to_shadow(const void *addr)
>>>  {
>>>  	return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
>>> diff --git a/mm/kasan/init.c b/mm/kasan/init.c
>>> index cc64ed6858c6..d39577d088a1 100644
>>> --- a/mm/kasan/init.c
>>> +++ b/mm/kasan/init.c
>>> @@ -279,6 +279,11 @@ int __ref kasan_populate_early_shadow(const void *shadow_start,
>>>  	return 0;
>>>  }
>>>  
>>> +void __init __weak kasan_populate_early_vm_area_shadow(void *start,
>>> +						       unsigned long size)
>>> +{
>>> +}
>>
>> I'm just wondering if this could be a generic function, perhaps with an
>> appropriate IS_ENABLED() check of a generic Kconfig option
>> (CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK ?) to short-circuit it, if it's
>> not only an arm64 problem.
>>
>> But I haven't looked much further, so would appeal to you to either
>> confirm or reject this idea.
>>
>> Thanks,
>> -- Marco
Kefeng Wang July 6, 2021, 4:07 a.m. UTC | #6
Hi Marco and Dmitry,

On 2021/7/5 23:04, Marco Elver wrote:
> On Mon, Jul 05, 2021 at 07:14PM +0800, Kefeng Wang wrote:
> [...]
>> +#ifdef CONFIG_KASAN_VMALLOC
>> +void __init __weak kasan_populate_early_vm_area_shadow(void *start,
>> +						       unsigned long size)
> This should probably not be __weak, otherwise you now have 2 __weak
> functions.
Indeed, forget it.
>
>> +{
>> +	unsigned long shadow_start, shadow_end;
>> +
>> +	if (!is_vmalloc_or_module_addr(start))
>> +		return;
>> +
>> +	shadow_start = (unsigned long)kasan_mem_to_shadow(start);
>> +	shadow_start = ALIGN_DOWN(shadow_start, PAGE_SIZE);
>> +	shadow_end = (unsigned long)kasan_mem_to_shadow(start + size);
>> +	shadow_end = ALIGN(shadow_end, PAGE_SIZE);
>> +	kasan_map_populate(shadow_start, shadow_end,
>> +			   early_pfn_to_nid(virt_to_pfn(start)));
>> +}
>> +#endif
> This function looks quite generic -- would any of this also apply to
> other architectures? I see that ppc and sparc at least also define
> CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK.

I can't try ppc/sparc, but only ppc support KASAN_VMALLOC,

I check the x86, it supports CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK,

looks this issue is existing on x86 and ppc.

>
>>   void __init kasan_init(void)
>>   {
>>   	kasan_init_shadow();
>> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
>> index 5310e217bd74..79d3895b0240 100644
>> --- a/include/linux/kasan.h
>> +++ b/include/linux/kasan.h
>> @@ -49,6 +49,8 @@ extern p4d_t kasan_early_shadow_p4d[MAX_PTRS_PER_P4D];
>>   int kasan_populate_early_shadow(const void *shadow_start,
>>   				const void *shadow_end);
>>   
>> +void kasan_populate_early_vm_area_shadow(void *start, unsigned long size);
>> +
>>   static inline void *kasan_mem_to_shadow(const void *addr)
>>   {
>>   	return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
>> diff --git a/mm/kasan/init.c b/mm/kasan/init.c
>> index cc64ed6858c6..d39577d088a1 100644
>> --- a/mm/kasan/init.c
>> +++ b/mm/kasan/init.c
>> @@ -279,6 +279,11 @@ int __ref kasan_populate_early_shadow(const void *shadow_start,
>>   	return 0;
>>   }
>>   
>> +void __init __weak kasan_populate_early_vm_area_shadow(void *start,
>> +						       unsigned long size)
>> +{
>> +}
> I'm just wondering if this could be a generic function, perhaps with an
> appropriate IS_ENABLED() check of a generic Kconfig option
> (CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK ?) to short-circuit it, if it's
> not only an arm64 problem.

kasan_map_populate() is arm64 special function, and the x86 has kasan_shallow_populate_pgds(),
ppc has kasan_init_shadow_page_tables(), so look those ARCHs should do the same way like ARM64,

Here we can't use kasan_populate_early_shadow(), this functions will make the early shadow maps
everything to a single page of zeroes(kasan_early_shadow_page), and set it pte_wrprotect, see
zero_pte_populate(), right?

Also I try this, it crashs on ARM64 when change kasan_map_populate() to kasan_populate_early_shadow(),

Unable to handle kernel write to read-only memory at virtual address ffff700002938000
...
Call trace:
  __memset+0x16c/0x1c0
  kasan_unpoison+0x34/0x6c
  kasan_unpoison_vmalloc+0x2c/0x3c
  __get_vm_area_node.constprop.0+0x13c/0x240
  __vmalloc_node_range+0xf4/0x4f0
  __vmalloc_node+0x80/0x9c
  init_IRQ+0xe8/0x130
  start_kernel+0x188/0x360
  __primary_switched+0xc0/0xc8


>
> But I haven't looked much further, so would appeal to you to either
> confirm or reject this idea.
>
> Thanks,
> -- Marco
> .
>
Kefeng Wang July 6, 2021, 4:12 a.m. UTC | #7
On 2021/7/5 22:10, kernel test robot wrote:
> Hi Kefeng,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on next-20210701]
>
> url:    https://github.com/0day-ci/linux/commits/Kefeng-Wang/arm64-support-page-mapping-percpu-first-chunk-allocator/20210705-190907
> base:    fb0ca446157a86b75502c1636b0d81e642fe6bf1
> config: i386-randconfig-a015-20210705 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build):
>          # https://github.com/0day-ci/linux/commit/5f6b5a402ed3e390563ddbddf12973470fd4886d
>          git remote add linux-review https://github.com/0day-ci/linux
>          git fetch --no-tags linux-review Kefeng-Wang/arm64-support-page-mapping-percpu-first-chunk-allocator/20210705-190907
>          git checkout 5f6b5a402ed3e390563ddbddf12973470fd4886d
>          # save the attached .config to linux build tree
>          make W=1 ARCH=i386
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>     mm/vmalloc.c: In function 'vm_area_register_early':
>>> mm/vmalloc.c:2252:2: error: implicit declaration of function 'kasan_populate_early_vm_area_shadow' [-Werror=implicit-function-declaration]
should add  a stub function when KASAN is not enabled, thanks.
>      2252 |  kasan_populate_early_vm_area_shadow(vm->addr, vm->size);
>           |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     cc1: some warnings being treated as errors
>
>
> vim +/kasan_populate_early_vm_area_shadow +2252 mm/vmalloc.c
>
Kefeng Wang July 16, 2021, 5:06 a.m. UTC | #8
Hi Marco and Dmitry, any comments about the following replay, thanks.

On 2021/7/6 12:07, Kefeng Wang wrote:
>
> Hi Marco and Dmitry,
>
> On 2021/7/5 23:04, Marco Elver wrote:
>> On Mon, Jul 05, 2021 at 07:14PM +0800, Kefeng Wang wrote:
>> [...]
>>> +#ifdef CONFIG_KASAN_VMALLOC
>>> +void __init __weak kasan_populate_early_vm_area_shadow(void *start,
>>> +						       unsigned long size)
>> This should probably not be __weak, otherwise you now have 2 __weak
>> functions.
> Indeed, forget it.
>>> +{
>>> +	unsigned long shadow_start, shadow_end;
>>> +
>>> +	if (!is_vmalloc_or_module_addr(start))
>>> +		return;
>>> +
>>> +	shadow_start = (unsigned long)kasan_mem_to_shadow(start);
>>> +	shadow_start = ALIGN_DOWN(shadow_start, PAGE_SIZE);
>>> +	shadow_end = (unsigned long)kasan_mem_to_shadow(start + size);
>>> +	shadow_end = ALIGN(shadow_end, PAGE_SIZE);
>>> +	kasan_map_populate(shadow_start, shadow_end,
>>> +			   early_pfn_to_nid(virt_to_pfn(start)));
>>> +}
>>> +#endif
>> This function looks quite generic -- would any of this also apply to
>> other architectures? I see that ppc and sparc at least also define
>> CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK.
>
> I can't try ppc/sparc, but only ppc support KASAN_VMALLOC,
>
> I check the x86, it supports CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK,
>
> looks this issue is existing on x86 and ppc.
>
>>>   void __init kasan_init(void)
>>>   {
>>>   	kasan_init_shadow();
>>> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
>>> index 5310e217bd74..79d3895b0240 100644
>>> --- a/include/linux/kasan.h
>>> +++ b/include/linux/kasan.h
>>> @@ -49,6 +49,8 @@ extern p4d_t kasan_early_shadow_p4d[MAX_PTRS_PER_P4D];
>>>   int kasan_populate_early_shadow(const void *shadow_start,
>>>   				const void *shadow_end);
>>>   
>>> +void kasan_populate_early_vm_area_shadow(void *start, unsigned long size);
>>> +
>>>   static inline void *kasan_mem_to_shadow(const void *addr)
>>>   {
>>>   	return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
>>> diff --git a/mm/kasan/init.c b/mm/kasan/init.c
>>> index cc64ed6858c6..d39577d088a1 100644
>>> --- a/mm/kasan/init.c
>>> +++ b/mm/kasan/init.c
>>> @@ -279,6 +279,11 @@ int __ref kasan_populate_early_shadow(const void *shadow_start,
>>>   	return 0;
>>>   }
>>>   
>>> +void __init __weak kasan_populate_early_vm_area_shadow(void *start,
>>> +						       unsigned long size)
>>> +{
>>> +}
>> I'm just wondering if this could be a generic function, perhaps with an
>> appropriate IS_ENABLED() check of a generic Kconfig option
>> (CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK ?) to short-circuit it, if it's
>> not only an arm64 problem.
>
> kasan_map_populate() is arm64 special function, and the x86 has kasan_shallow_populate_pgds(),
> ppc has kasan_init_shadow_page_tables(), so look those ARCHs should do the same way like ARM64,
>
> Here we can't use kasan_populate_early_shadow(), this functions will make the early shadow maps
> everything to a single page of zeroes(kasan_early_shadow_page), and set it pte_wrprotect, see
> zero_pte_populate(), right?
>
> Also I try this, it crashs on ARM64 when change kasan_map_populate() to kasan_populate_early_shadow(),
>
> Unable to handle kernel write to read-only memory at virtual address ffff700002938000
> ...
> Call trace:
>   __memset+0x16c/0x1c0
>   kasan_unpoison+0x34/0x6c
>   kasan_unpoison_vmalloc+0x2c/0x3c
>   __get_vm_area_node.constprop.0+0x13c/0x240
>   __vmalloc_node_range+0xf4/0x4f0
>   __vmalloc_node+0x80/0x9c
>   init_IRQ+0xe8/0x130
>   start_kernel+0x188/0x360
>   __primary_switched+0xc0/0xc8
>
>
>> But I haven't looked much further, so would appeal to you to either
>> confirm or reject this idea.
>>
>> Thanks,
>> -- Marco
>> .
>>
Marco Elver July 16, 2021, 7:41 a.m. UTC | #9
On Fri, 16 Jul 2021 at 07:06, Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> Hi Marco and Dmitry, any comments about the following replay, thanks.

Can you clarify the question? I've been waiting for v2.

I think you said that this will remain arm64 specific and the existing
generic kasan_populate_early_shadow() doesn't work.

If there's nothing else that needs resolving, please go ahead and send
v2 (the __weak comment still needs resolving).

Thanks,
-- Marco
Kefeng Wang July 17, 2021, 2:40 a.m. UTC | #10
On 2021/7/16 15:41, Marco Elver wrote:
> On Fri, 16 Jul 2021 at 07:06, Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>> Hi Marco and Dmitry, any comments about the following replay, thanks.
> Can you clarify the question? I've been waiting for v2.
>
> I think you said that this will remain arm64 specific and the existing
> generic kasan_populate_early_shadow() doesn't work.

Yes, I can't find a generic way to solve the issue, if there is no 
better way, I

will send a new version(fix the build error and the wrong __weak comment)

>
> If there's nothing else that needs resolving, please go ahead and send
> v2 (the __weak comment still needs resolving).
Thanks. will do.
>
> Thanks,
> -- Marco
> .
>
diff mbox series

Patch

diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
index 61b52a92b8b6..c295a256c573 100644
--- a/arch/arm64/mm/kasan_init.c
+++ b/arch/arm64/mm/kasan_init.c
@@ -287,6 +287,24 @@  static void __init kasan_init_depth(void)
 	init_task.kasan_depth = 0;
 }
 
+#ifdef CONFIG_KASAN_VMALLOC
+void __init __weak kasan_populate_early_vm_area_shadow(void *start,
+						       unsigned long size)
+{
+	unsigned long shadow_start, shadow_end;
+
+	if (!is_vmalloc_or_module_addr(start))
+		return;
+
+	shadow_start = (unsigned long)kasan_mem_to_shadow(start);
+	shadow_start = ALIGN_DOWN(shadow_start, PAGE_SIZE);
+	shadow_end = (unsigned long)kasan_mem_to_shadow(start + size);
+	shadow_end = ALIGN(shadow_end, PAGE_SIZE);
+	kasan_map_populate(shadow_start, shadow_end,
+			   early_pfn_to_nid(virt_to_pfn(start)));
+}
+#endif
+
 void __init kasan_init(void)
 {
 	kasan_init_shadow();
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 5310e217bd74..79d3895b0240 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -49,6 +49,8 @@  extern p4d_t kasan_early_shadow_p4d[MAX_PTRS_PER_P4D];
 int kasan_populate_early_shadow(const void *shadow_start,
 				const void *shadow_end);
 
+void kasan_populate_early_vm_area_shadow(void *start, unsigned long size);
+
 static inline void *kasan_mem_to_shadow(const void *addr)
 {
 	return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
diff --git a/mm/kasan/init.c b/mm/kasan/init.c
index cc64ed6858c6..d39577d088a1 100644
--- a/mm/kasan/init.c
+++ b/mm/kasan/init.c
@@ -279,6 +279,11 @@  int __ref kasan_populate_early_shadow(const void *shadow_start,
 	return 0;
 }
 
+void __init __weak kasan_populate_early_vm_area_shadow(void *start,
+						       unsigned long size)
+{
+}
+
 static void kasan_free_pte(pte_t *pte_start, pmd_t *pmd)
 {
 	pte_t *pte;
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index a98cf97f032f..f19e07314ee5 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2249,6 +2249,7 @@  void __init vm_area_register_early(struct vm_struct *vm, size_t align)
 	vm->addr = (void *)addr;
 
 	vm_area_add_early(vm);
+	kasan_populate_early_vm_area_shadow(vm->addr, vm->size);
 }
 
 static void vmap_init_free_space(void)