Re: [PATCH] x86/speculation/l1tf: fix overflow on l1tf_pfn_limit() on 32bit

[Vlastimil Babka <vbabka@suse.cz>]

On 08/20/2018 12:49 PM, Michal Hocko wrote:
> On Mon 20-08-18 11:58:35, Vlastimil Babka wrote:
>> On 32bit PAE kernels on 64bit hardware with enough physical bits,
>> l1tf_pfn_limit() will overflow unsigned long. This in turn affects
>> max_swapfile_size() and can lead to swapon returning -EINVAL. This has been
>> observed in a 32bit guest with 42 bits physical address size, where
>> max_swapfile_size() overflows exactly to 1 << 32, thus zero, and produces the
>> following warning to dmesg:
>>
>> [    6.396845] Truncating oversized swap area, only using 0k out of 2047996k
>>
>> Fix this by using unsigned long long instead.
>>
>> Reported-by: Dominique Leuenberger <dimstar@suse.de>
>> Reported-by: Adrian Schroeter <adrian@suse.de>
>> Fixes: 17dbca119312 ("x86/speculation/l1tf: Add sysfs reporting for l1tf")
>> Fixes: 377eeaa8e11f ("x86/speculation/l1tf: Limit swap file size to MAX_PA/2")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> 
> Looks good to me. I would probably use phys_addr_t which would be more
> descriptive but this is just minor thing.

Hmm phys_addr_t is still 32bit on !PAE so there the overflow could still
happen. I guess max_swapfile_size() should skip the whole L1TF part for
!PAE since there is no pte inverting done anyway.

Also the value is "number of pages" which is not the same as "physical
address" so the phys_addr_t could be misleading anyway?

> Acked-by: Michal Hocko <mhocko@suse.com>

Thanks!