linux-kernel - Re: [PATCH 4/5] Hot-remove implementation for arm64

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: <3a5afc84-b920-e164-c05a-2c6a3e05bf84@redhat.com>
Date:   Mon, 24 Apr 2017 16:59:58 -0700
From:   Laura Abbott <labbott@...hat.com>
To:     Andrea Reale <ar@...ux.vnet.ibm.com>
Cc:     Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Mark Rutland <mark.rutland@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        m.bielski@...tualopensystems.com, scott.branden@...adcom.com,
        Will Deacon <will.deacon@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Xishi Qiu <qiuxishi@...wei.com>
Subject: Re: [PATCH 4/5] Hot-remove implementation for arm64

On 04/21/2017 03:05 AM, Andrea Reale wrote:
> Hi all,
> 
> thanks for taking the time to comment. Replies in-line.
> 
> On Wed, Apr 19, 2017 at 08:53:13AM -0700, Laura Abbott wrote:
>> On 04/18/2017 11:48 AM, Ard Biesheuvel wrote:
>>> On 18 April 2017 at 19:21, Mark Rutland <mark.rutland@....com> wrote:
>>>> On Fri, Apr 14, 2017 at 03:01:58PM +0100, Andrea Reale wrote:
> 
> [...]
> 
>>>>
>>>> From a quick scan, I see that it's necessary to use pgtable_page_ctor()
>>>> for pages that will be used for userspace page tables, but it's not
>>>> clear to me if it's ever necessary for pages used for kernel page
>>>> tables.
>>>>
>>>> If it is, we appear to have a bug on arm64.
>>>>
>>>> Laura, Ard, thoughts?
>>>>
>>>
>>> The generic apply_to_page_range() will expect the PTE lock to be
>>> initialized for page table pages that are not part of init_mm. For
>>> arm64, that is precisely efi_mm as far as I am aware. For EFI, the
>>> locking is unnecessary but does no harm (the permissions are set once
>>> via apply_to_page_range() at boot), so I added this call when adding
>>> support for strict permissions in EFI rt services mappings.
>>>
>>> So I think it is appropriate for create_pgd_mapping() to be in charge
>>> of calling the ctor(). We simply have no destroy_pgd_mapping()
>>> counterpart that would be the place for the dtor() call, given that we
>>> never take down EFI rt services mappi >
>>> Whether it makes sense or not to lock/unlock in apply_to_page_range()
>>> is something I did not spend any brain cycles on at the time.
>>>
>>
>> Agreed there shouldn't be a problem right now. I do think the locking is
>> appropriate in apply_to_page_range given what other functions also get
>> locked.
>>
>> I really wish this were less asymmetrical though since it get hard
>> to reason about. It looks like hotplug_paging will call the ctor,
>> so is there an issue with calling hot-remove on memory that was once
>> hot-added or is that not a concern?
>>
>> Thanks,
>> Laura
> 
> I think the confusion comes from the fact that, in hotplug_paging, we are
> passing pgd_pgtable_alloc as the page allocator for __create_pgd_mapping,
> which always calls the ctor.
> 
> If I got things right (but, please, correct me if I am wrong), we don't
> need to get the pte_lock that the ctor gets since - in hotplug - we are
> adding to init_mm.
> 
> Moreover, I am just realizing that calling the dtor while hot-removing
> might create problems when removing memory that *was not* previously
> hotplugged, as we are calling a dtor on something that was never
> ctor'ed. Is that what you were hinting at, Laura?
> 
> Thanks and best regards,
> Andrea
> 

Yes, that was what I was thinking.

Thanks,
Laura