linux-kernel - Re: [PATCH 1/1] x86/ioremap: Use is_vmalloc

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: <87frra3qlr.fsf@nvdebian.thelocal>
Date: Mon, 12 Aug 2024 21:46:40 +1000
From: Alistair Popple <apopple@...dia.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Dan Williams <dan.j.williams@...el.com>, dave.hansen@...ux.intel.com,
 luto@...nel.org, peterz@...radead.org, max8rr8@...il.com,
 linux-kernel@...r.kernel.org, x86@...nel.org, jhubbard@...dia.com, Kees
  Cook <keescook@...omium.org>
Subject: Re: [PATCH 1/1] x86/ioremap: Use is_vmalloc_addr in iounmap


Thomas Gleixner <tglx@...utronix.de> writes:

> On Mon, Aug 12 2024 at 17:41, Alistair Popple wrote:
>> Thomas Gleixner <tglx@...utronix.de> writes:
>>> ---
>>> --- a/arch/x86/include/asm/pgtable_64_types.h
>>> +++ b/arch/x86/include/asm/pgtable_64_types.h
>>> @@ -187,6 +187,8 @@ extern unsigned int ptrs_per_p4d;
>>>  #define KMSAN_MODULES_ORIGIN_START	(KMSAN_MODULES_SHADOW_START + MODULES_LEN)
>>>  #endif /* CONFIG_KMSAN */
>>>  
>>> +#define DIRECT_MAP_END		(VMALLOC_START - 1)
>>
>> If I'm understanding the KASLR implementation correctly then this
>> doesn't seem quite right - the entropy means there is a hole from the
>> end of the direct map (ie. the range PAGE_OFFSET to
>> PAGE_OFFSET+kaslr_regions[0].size_tb) and VMALLOC_START which shouldn't
>> be used.
>>
>> In practice hotplugging memory into that range probably works, but it
>> seems like it would set us up for future bugs. It also means memory
>> hotplug could fail intermittently based on the per-boot entropy.
>>
>> For example with CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING set to 10T one
>> could imagine hotplugging 16T would mostly work except on some system
>> boots if KASLR happens to randomly place VMALLOC_START close to the end
>> of the direct map.
>>
>> Therefore to keep memory hotplug deterministic I think it would be
>> better to define DIRECT_MAP_END as a variable that KASLR sets/updates.
>
> Right. I forgot about the hole.
>
>> This does not look right to me - isn't DIRECT_MAP_END a virtual address
>> where as the resource ranges are physical addresses? Ie. I think this
>> should be:
>>
>> +		end = min_t(resource_size_t, base->end, __pa(DIRECT_MAP_END));
>>
>> The same applies to the rest of the DIRECT_MAP_END users here. Perhaps
>> it would be better to define this as DIRECT_MAP_SIZE and calculate this
>> based off PAGE_OFFSET instead?
>
> Duh, yes. I shouldn't try to write patches at 30C :)

It's a much more plesent 7C in the upside down half of the world atm :)

Anyway I will put together a proper patch to do the above.