Re: [PATCH v2 1/2] powerpc: Fix virt_addr_valid() check

[Kefeng Wang <wangkefeng.wang@huawei.com>]

On 2022/1/11 14:04, Christophe Leroy wrote:
>
> Le 11/01/2022 à 05:37, Nicholas Piggin a écrit :
>> Excerpts from Kefeng Wang's message of January 8, 2022 9:58 pm:
>>> Hi PPC maintainers， ping..
>> Hmm. I might have confused myself about this. I'm going back and
>> trying to work out what I was thinking when I suggested it. This
>> works on 64e because vmalloc space is below the kernel linear map,
>> right?
>>
>> On 64s it is the other way around and it is still possible to enable
>> flatmem on 64s. Altough we might just not hit the problem there because
>> __pa() will not mask away the vmalloc offset for 64s so it will still
>> return something that's outside the pfn_valid range for flatmem. That's
>> very subtle though.
> That's the way it works on PPC32 at least, so for me it's not chocking
> to have it work the same way on PPC64s.
>
> The main issue here is the way __pa() works. On PPC32 __pa = va -
> PAGE_OFFSET, so it works correctly for any address.
> On PPC64, __pa() works by masking out the 2 top bits instead of
> substracting PAGE_OFFSET, so the test must add a verification that we
> really have the 2 top bits set at first. This is what (addr >=
> PAGE_OFFSET) does. Once this first test is done, we can perfectly rely
> on pfn_valid() just like PPC32, I see absolutely no point in an
> additionnal test checking the addr is below KERN_VIRT_START.


Hi Christophe and Nicholas， for ppc32， I think we need check the upper 
limit,

eg,  addr >= PAGE_OFFSET && addr < high_memory

arch/powerpc/mm/mem.c:  high_memory = (void *) __va(max_low_pfn * 
PAGE_SIZE);

for ppc32 max_low_pfn is the upper low memory pfn,  and For ppc64, 
high_memory is

the max memory pfn, it looks good too, correct me if I'm wrong, if the 
above check

is ok, I will send a new v3,  thanks.




>
>
>> The checks added to __pa actually don't prevent vmalloc memory from
>> being passed to it either on 64s, only a more basic test.
> That's correct. It is the role of pfn_valid() to check that.
>
> Christophe
>
>> I think 64s wants (addr >= PAGE_OFFSET && addr < KERN_VIRT_START) as
>> the condition.  Could possibly add that check to __pa as well to
>> catch vmalloc addresses.
>>
>> Thanks,
>> Nick
>>
>>