linux-kernel - Re: [PATCH] x86/mm/mem_encrypt: fix a crash with kmemleak

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <26957df8-f1b0-c000-2edf-1368ce22bfce@lca.pw>
Date:   Tue, 16 Apr 2019 20:38:30 -0400
From:   Qian Cai <cai@....pw>
To:     Borislav Petkov <bp@...en8.de>
Cc:     dave.hansen@...ux.intel.com, luto@...nel.org, peterz@...radead.org,
        catalin.marinas@....com, tglx@...utronix.de, mingo@...hat.com,
        x86@...nel.org, linux-kernel@...r.kernel.org,
        Brijesh Singh <brijesh.singh@....com>
Subject: Re: [PATCH] x86/mm/mem_encrypt: fix a crash with kmemleak_scan

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.