Re: [PATCH 2/2] arm/xen: Add misuse warning to virt_to_gfn

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.08.2020 01:48, Stefano Stabellini wrote:
> On Tue, 25 Aug 2020, Jan Beulich wrote:
>> On 25.08.2020 11:31, Simon Leiner wrote:
>>> --- a/include/xen/arm/page.h
>>> +++ b/include/xen/arm/page.h
>>> @@ -76,7 +76,11 @@ static inline unsigned long bfn_to_pfn(unsigned long bfn)
>>>  #define bfn_to_local_pfn(bfn)      bfn_to_pfn(bfn)
>>>  
>>>  /* VIRT <-> GUEST conversion */
>>> -#define virt_to_gfn(v)             (pfn_to_gfn(virt_to_phys(v) >> 
>>> XEN_PAGE_SHIFT))
>>> +#define virt_to_gfn(v)                                                     
>>>     \
>>> +   ({                                                                     \
>>> +           WARN_ON_ONCE(is_vmalloc_addr(v));                              \
>>> +           pfn_to_gfn(virt_to_phys(v) >> XEN_PAGE_SHIFT);                 \
>>> +   })
>>
>> Shouldn't such a check cover more than just the vmalloc range,
>> i.e. everything outside of what __va() can validly return?
> 
> Keep in mind that this patch is meant as sister patch to
> https://marc.info/?l=xen-devel&m=159834800203817
> so it makes sense to check for vmalloc addresses specifically.

I had seen that patch before writing the reply, and I had assumed
precisely what you say. It was for this reason that I did point
out the limitation: While there's a specific issue for the vmalloc
range, any other addresses outside the direct mapping area are as
problematic (aiui).

> That said, I am not aware of a good way to implement the __va test you
> are suggesting.

Hmm, to me __phys_to_virt() and __virt_to_phys_nodebug() give
the impression that they're just linear transformations of the
address, which would seem to suggest there's a pre-determined
address range used for the direct map.

Jan