[PATCH 4/5] powerpc/64: Add VIRTUAL_BUG_ON checks for __va and __pa addresses

[Nicholas Piggin]

Excerpts from Christophe Leroy's message of December 24, 2021 11:24 pm:
> Hi Nic,
> 
> Le 24/07/2019 à 10:46, Nicholas Piggin a écrit :
>> Ensure __va is given a physical address below PAGE_OFFSET, and __pa is
>> given a virtual address above PAGE_OFFSET.
>> 
>> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
>> ---
>>   arch/powerpc/include/asm/page.h | 14 ++++++++++++--
>>   1 file changed, 12 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
>> index 0d52f57fca04..c8bb14ff4713 100644
>> --- a/arch/powerpc/include/asm/page.h
>> +++ b/arch/powerpc/include/asm/page.h
>> @@ -215,9 +215,19 @@ static inline bool pfn_valid(unsigned long pfn)
>>   /*
>>    * gcc miscompiles (unsigned long)(&static_var) - PAGE_OFFSET
>>    * with -mcmodel=medium, so we use & and | instead of - and + on 64-bit.
>> + * This also results in better code generation.
>>    */
>> -#define __va(x) ((void *)(unsigned long)((phys_addr_t)(x) | PAGE_OFFSET))
>> -#define __pa(x) ((unsigned long)(x) & 0x0fffffffffffffffUL)
>> +#define __va(x)								\
>> +({									\
>> +	VIRTUAL_BUG_ON((unsigned long)(x) >= PAGE_OFFSET);		\
>> +	(void *)(unsigned long)((phys_addr_t)(x) | PAGE_OFFSET);	\
>> +})
>> +
>> +#define __pa(x)								\
>> +({									\
>> +	VIRTUAL_BUG_ON((unsigned long)(x) < PAGE_OFFSET);		\
> 
> With this, it is likely that virt_addr_valid() BUGs on a non valid address.
> 
> I think the purpose of virt_addr_valid() is to check addresses 
> seamlessly, see check_heap_object()

Looks like you're right. How did you catch that?

We could change virt_addr_valid() to make that test first. x86 and arm64
both do checking rather than relying on !pfn_valid for < PAGE_OFFSET
addresses.

Thanks,
Nick