linux-kernel - Re: [PATCH] mm: Fix __wp_page_copy_user fallback path for remote mm

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: <50578e9d-6562-4f0f-af7c-cabfb875bbb7@redhat.com>
Date: Tue, 12 Nov 2024 11:00:27 +0100
From: David Hildenbrand <david@...hat.com>
To: Asahi Lina <lina@...hilina.net>, Alistair Popple <apopple@...dia.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
 Sergio Lopez Pascual <slp@...hat.com>, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, asahi@...ts.linux.dev,
 Dan Williams <dan.j.williams@...el.com>
Subject: Re: [PATCH] mm: Fix __wp_page_copy_user fallback path for remote mm

On 12.11.24 10:48, Asahi Lina wrote:
> 
> 
> On 11/11/24 8:24 AM, Alistair Popple wrote:
>>
>> David Hildenbrand <david@...hat.com> writes:
>>
>>> On 07.11.24 18:32, Asahi Lina wrote:
>>>> On 11/8/24 2:14 AM, David Hildenbrand wrote:
>>>>> I recall that there is still a problem with false-positives on
>>>>> folio_test_anon() with ZONE_DEVICE pages, so it's maybe not that
>>>>> easy ... and the whole get_dev_pagemap() stuff is nasty.
>>
>> Specifically FS DAX reuses PAGE_MAPPING_ANON in
>> include/linux/page-flags.h
>>
>>      /*
>>       * Different with flags above, this flag is used only for fsdax mode.  It
>>       * indicates that this page->mapping is now under reflink case.
>>       */
>>      #define PAGE_MAPPING_DAX_SHARED	((void *)0x1)
>>
>> FS DAX pages are never anon though, so you could probably test for
>> !vma_is_dax() and/or add an implementation of is_fsdax_page().
>>
>>>>> Likely we would have to do what GUP does, and temporarily grab a pgmap
>>>>> reference. Gah.
>>>>>
>>>>>
>>>>> So if we sort out the pagemap stuff and the possibly wrong
>>>>> folio_test_anon() on some ZONE_DEVICE pages (but not all, because IIRC
>>>>> DEVICE_PRIVATE can be anon ...), it might be doable.
>>
>> Correct, DEVICE_PRIVATE and DEVICE_COHERENT pages are always anon (at
>> least for now).
>>
>>>>> But it sounds ugly, especially because that code might change soon and
>>>>> not require messing with ZONE_DEVICE pages on that level.
>>
>> Yes, I'm hopoing to get the next version of that series posted this
>> week. I found a couple of other FS DAX bugs that slowed me down.
>>
>>   - Alistair
>>
>>>>> And then, we'd not be able to handle VM_PFNMAP cleanly ...
>>>>>
> 
> If this is all going to be fixed another way soon then I think there's
> no rush to get a workaround in earlier than that, I just don't want it
> to fall by the wayside.
> 
> We have my original patch downstream in libkrunfw (which despite the
> lockdep complaints does work in practice)

I assume it's sufficient to deadlock when a writer pops up after you 
succeeded with the first read-locking, and before you start the second 
read-locking. IIRC, rwsem is a fair lock, so read-locking when 
already-read-locked is not guaranteed to work.

That's why lockdep complains.

-- 
Cheers,

David / dhildenb