linux-kernel - Re: Race condition observed between page migration and page fault handling on arm64 machines

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <92df0ee1-d3c9-41e2-834c-284127ae2c4c@arm.com>
Date: Thu, 1 Aug 2024 15:35:49 +0530
From: Dev Jain <dev.jain@....com>
To: David Hildenbrand <david@...hat.com>, akpm@...ux-foundation.org,
 willy@...radead.org
Cc: ryan.roberts@....com, anshuman.khandual@....com, catalin.marinas@....com,
 cl@...two.org, vbabka@...e.cz, mhocko@...e.com, apopple@...dia.com,
 osalvador@...e.de, baolin.wang@...ux.alibaba.com,
 dave.hansen@...ux.intel.com, will@...nel.org, baohua@...nel.org,
 ioworker0@...il.com, gshan@...hat.com, mark.rutland@....com,
 kirill.shutemov@...ux.intel.com, hughd@...gle.com, aneesh.kumar@...nel.org,
 yang@...amperecomputing.com, peterx@...hat.com, broonie@...nel.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-mm@...ck.org
Subject: Re: Race condition observed between page migration and page fault
 handling on arm64 machines


On 8/1/24 15:11, David Hildenbrand wrote:
> On 01.08.24 11:38, Dev Jain wrote:
>>
>> On 8/1/24 14:12, David Hildenbrand wrote:
>>> On 01.08.24 10:16, Dev Jain wrote:
>>>> I and Ryan had a discussion and we thought it would be best to get
>>>> feedback
>>>> from the community.
>>>>
>>>> The migration mm selftest currently fails on arm64 for shared anon
>>>> mappings,
>>>> due to the following race:
>>>
>>> Do you mean MAP_SHARED|MAP_ANON or MAP_PRIVATE|MAP_ANON_fork? Because
>>> you note shmem below, I assume you mean MAP_SHARED|MAP_ANON
>>
>>
>> Yes.
>>
>>>
>>>>
>>>> Migration:                        Page fault:
>>>> try_to_migrate_one():                    handle_pte_fault():
>>>> 1. Nuke the PTE                        PTE has been deleted =>
>>>> do_pte_missing()
>>>> 2. Mark the PTE for migration                PTE has not been deleted
>>>> but is just not "present" => do_swap_page()
>>>>
>>>
>>> In filemap_fault_recheck_pte_none() we recheck under PTL to make sure
>>> that a temporary pte_none() really was persistent pte_none() and not a
>>> temporary pte_none() under PTL.
>>>
>>> Should we do something similar in do_fault()? I see that we already do
>>> something like that on the "!vma->vm_ops->fault" path.
>>>
>>> But of course, there is a tradeoff between letting migration
>>> (temporarily) fail and grabbing the PTL during page faults.
>>
>>
>> To dampen the tradeoff, we could do this in shmem_fault() instead? But
>> then, this would mean that we do this in all
>>
>> kinds of vma->vm_ops->fault, only when we discover another reference
>> count race condition :) Doing this in do_fault()
>>
>> should solve this once and for all. In fact, do_pte_missing() may call
>> do_anonymous_page() or do_fault(), and I just
>>
>> noticed that the former already checks this using vmf_pte_changed().
>
> What I am still missing is why this is (a) arm64 only; and (b) if this 
> is something we should really worry about. There are other reasons 
> (e.g., speculative references) why migration could temporarily fail, 
> does it happen that often that it is really something we have to worry 
> about?


(a) See discussion at [1]; I guess it passes on x86, which is quite 
strange since the race is clearly arch-independent.

(b) On my machine, on an average in under 10 iterations of move_pages(), 
it fails, which seems problematic to

me assuming it is passing on other arches, since 10 iterations means 
this is failing very quickly.


[1]: 
https://lore.kernel.org/all/9de42ace-dab1-5f60-af8a-26045ada27b9@arm.com/