Messages in this thread | | | Date | Wed, 19 Jan 2022 09:15:55 +0800 | Subject | Re: [PATCH v2 1/2] powerpc: Fix virt_addr_valid() check | From | Kefeng Wang <> |
| |
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 >> >>
| |