linux-kernel - Re: [PATCH v3 2/3] mm/khugepaged: Fix GUP-fast interaction by sending IPI

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]

Date:   Mon, 28 Nov 2022 18:00:49 +0100
From:   David Hildenbrand <david@...hat.com>
To:     Jann Horn <jannh@...gle.com>
Cc:     security@...nel.org, Andrew Morton <akpm@...ux-foundation.org>,
        Yang Shi <shy828301@...il.com>, Peter Xu <peterx@...hat.com>,
        John Hubbard <jhubbard@...dia.com>,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH v3 2/3] mm/khugepaged: Fix GUP-fast interaction by sending
 IPI

On 28.11.22 17:58, Jann Horn wrote:
> On Mon, Nov 28, 2022 at 2:46 PM David Hildenbrand <david@...hat.com> wrote:
>> On 25.11.22 22:37, Jann Horn wrote:
>>> Since commit 70cbc3cc78a99 ("mm: gup: fix the fast GUP race against THP
>>> collapse"), the lockless_pages_from_mm() fastpath rechecks the pmd_t to
>>> ensure that the page table was not removed by khugepaged in between.
>>>
>>> However, lockless_pages_from_mm() still requires that the page table is not
>>> concurrently freed.
>>
>> That's an interesting point. For anon THPs, the page table won't get
>> immediately freed, but instead will be deposited in the "pgtable list"
>> stored alongside the THP.
>>
>>   From there, it might get withdrawn (pgtable_trans_huge_withdraw()) and
>>
>> a) Reused as a page table when splitting the THP. That should be fine,
>> no garbage in it, simply a page table again.
> 
> Depends on the definition of "fine" - it will be a page table again,
> but deposited page tables are not associated with a specific address,
> so it might be reused at a different address. If GUP-fast on address A
> races with a page table from address A being deposited and reused at
> address B, and then GUP-fast returns something from address B, that's
> not exactly great either.

The "PMD changed" check should catch that. We only care about not 
dereferencing something that's garbage and not a page/folio if I 
remember the previous discussions on that correctly.

Anyhow, feel free to add my

Acked-by: David Hildenbrand <david@...hat.com>

-- 
Thanks,

David / dhildenb