[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <017744c1-1252-a25c-3dcc-057ee18d0769@csgroup.eu>
Date: Thu, 20 Jan 2022 07:31:43 +0000
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Kefeng Wang <wangkefeng.wang@...wei.com>,
Nicholas Piggin <npiggin@...il.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Kees Cook <keescook@...omium.org>,
Laura Abbott <labbott@...hat.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
Mark Rutland <mark.rutland@....com>,
Michael Ellerman <mpe@...erman.id.au>,
Paul Mackerras <paulus@...ba.org>
Subject: Re: [PATCH v2 1/2] powerpc: Fix virt_addr_valid() check
Le 19/01/2022 à 02:15, Kefeng Wang a écrit :
>
> 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,
Why ? Have you experimented any problem at all on PPC32 with the way it
is done at the moment ?
I don't think we have to change PPC32 at all unless we have a real
reason to do it.
>
> eg, addr >= PAGE_OFFSET && addr < high_memory
Isn't it exactly what pfn_valid() already do today ?
Why change that at all ?
Christophe
>
> 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
>>>
>>>
Powered by blists - more mailing lists