linux-kernel - Re: [PATCH v2 1/2] powerpc: Fix virt_addr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e1c7a290-45ac-25ab-018e-2289fad72413@huawei.com>
Date:   Thu, 20 Jan 2022 19:09:29 +0800
From:   Kefeng Wang <wangkefeng.wang@...wei.com>
To:     Christophe Leroy <christophe.leroy@...roup.eu>,
        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


On 2022/1/20 15:31, Christophe Leroy wrote:
>
> 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.

yes, I missed this commit in old kernel(lts5.10), you have fixed the 
upper limit.

commit 602946ec2f90d5bd965857753880db29d2d9a1e9
Author: Christophe Leroy <christophe.leroy@...roup.eu>
Date:   Tue Oct 12 12:40:37 2021 +0200

     powerpc: Set max_mapnr correctly


>
>> 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
>>>>
>>> >