Catalin Marinas <catalin.marinas@xxxxxxx> writes:
> On Thu, Mar 21, 2019 at 12:15:46AM +1100, Michael Ellerman wrote:
>> Catalin Marinas <catalin.marinas@xxxxxxx> writes:
>> > On Wed, Mar 13, 2019 at 10:57:17AM -0400, Qian Cai wrote:
>> >> @@ -1531,7 +1547,14 @@ static void kmemleak_scan(void)
>> >>
>> >> /* data/bss scanning */
>> >> scan_large_block(_sdata, _edata);
>> >> - scan_large_block(__bss_start, __bss_stop);
>> >> +
>> >> + if (bss_hole_start) {
>> >> + scan_large_block(__bss_start, bss_hole_start);
>> >> + scan_large_block(bss_hole_stop, __bss_stop);
>> >> + } else {
>> >> + scan_large_block(__bss_start, __bss_stop);
>> >> + }
>> >> +
>> >> scan_large_block(__start_ro_after_init, __end_ro_after_init);
>> >
>> > I'm not a fan of this approach but I couldn't come up with anything
>> > better. I was hoping we could check for PageReserved() in scan_block()
>> > but on arm64 it ends up not scanning the .bss at all.
>> >
>> > Until another user appears, I'm ok with this patch.
>> >
>> > Acked-by: Catalin Marinas <catalin.marinas@xxxxxxx>
>>
>> I actually would like to rework this kvm_tmp thing to not be in bss at
>> all. It's a bit of a hack and is incompatible with strict RWX.
>>
>> If we size it a bit more conservatively we can hopefully just reserve
>> some space in the text section for it.
>>
>> I'm not going to have time to work on that immediately though, so if
>> people want this fixed now then this patch could go in as a temporary
>> solution.
>
> I think I have a simpler idea. Kmemleak allows punching holes in
> allocated objects, so just turn the data/bss sections into dedicated
> kmemleak objects. This happens when kmemleak is initialised, before the
> initcalls are invoked. The kvm_free_tmp() would just free the
> corresponding part of the bss.
>
> Patch below, only tested briefly on arm64. Qian, could you give it a try
> on powerpc? Thanks.
>
> --------8<------------------------------
> diff --git a/arch/powerpc/kernel/kvm.c b/arch/powerpc/kernel/kvm.c
> index 683b5b3805bd..c4b8cb3c298d 100644
> --- a/arch/powerpc/kernel/kvm.c
> +++ b/arch/powerpc/kernel/kvm.c
> @@ -712,6 +712,8 @@ static void kvm_use_magic_page(void)
>
> static __init void kvm_free_tmp(void)
> {
> + kmemleak_free_part(&kvm_tmp[kvm_tmp_index],
> + ARRAY_SIZE(kvm_tmp) - kvm_tmp_index);
> free_reserved_area(&kvm_tmp[kvm_tmp_index],
> &kvm_tmp[ARRAY_SIZE(kvm_tmp)], -1, NULL);
> }
Fine by me as long as it works (sounds like it does).
Acked-by: Michael Ellerman <mpe@xxxxxxxxxxxxxx> (powerpc)
cheers
