Re: [PATCH] x86/mm/mem_encrypt: fix a crash with kmemleak_scan

[Qian Cai]

On 4/16/19 1:11 PM, Borislav Petkov wrote:
>> +	/*
>> +	 * Inform kmemleak about the hole in the .bss section since the
>> +	 * corresponding pages will be unmapped with DEBUG_PAGEALLOC=y.
>> +	 */
>> +	kmemleak_free_part((void *)vaddr, vaddr_end - vaddr);
>>  	free_init_pages("unused decrypted", vaddr, vaddr_end);
> 
> I don't understand what the logic here is: we have a couple of other
> free_init_pages() calls but they don't have kmemleak_free_part() in
> front.
> 
> Now, if kmemleak needs to be told that memory is getting freed, why
> isn't kmemleak_free_part() called in free_init_pages() ?
> 
> This needs more explanation.

kmemleak_init() will register the data/bss sections (only register
.data..ro_after_init if not within .data) and then kmemleak_scan() will scan
those address and dereference them looking for pointer referencing. If
free_init_pages() free and unmap pages in those sections, kmemleak_scan() will
trigger a crash if referencing one of those addresses.

I checked other x86 free_init_pages() call sites and don't see anything obvious
where another place to free an address in those sections.

__smp_locks[]: .smp_locks
__initramfs_start[]: .init
__init_begin: .init
from text_end to rodata_start: contains .notes, __ex_table
from rodata_end to _sdata: .pci_fixup, __ksymtab, __ksymtab_gpl etc

So, I don't think it need to add kmemleak_free_part() in every free_init_pages()
calls.