linux-kernel - Re: [PATCH] mm, hugetlb: fix resv_huge_pages underflow on UFFDIO

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:   Fri, 14 May 2021 10:56:36 -0700
From:   Mike Kravetz <mike.kravetz@...cle.com>
To:     Peter Xu <peterx@...hat.com>
Cc:     Mina Almasry <almasrymina@...gle.com>,
        Axel Rasmussen <axelrasmussen@...gle.com>,
        Linux-MM <linux-mm@...ck.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm, hugetlb: fix resv_huge_pages underflow on UFFDIO_COPY

On 5/14/21 5:31 AM, Peter Xu wrote:
> Hi, Mike,
> 
> On Thu, May 13, 2021 at 09:02:15PM -0700, Mike Kravetz wrote:
> 
> [...]
> 
>> I am also concerned with the semantics of this approach and what happens
>> when a fault races with the userfaultfd copy.  Previously I asked Peter
>> if we could/should use a page found in the cache for the copy.  His
>> answer was as follows:
>>
>>  AFAICT that's the expected behavior, and it need to be like that so as to avoid
>>  silent data corruption (if the page cache existed, it means the page is not
>>  "missing" at all, then it does not suite for a UFFDIO_COPY as it's only used
>>  for uffd page missing case).
> 
> I didn't follow the rest discussion in depth yet... but just to mention that
> the above answer was for the question whether we can "update the page in the
> page cache", rather than "use a page found in the page cache".
> 
> I think reuse the page should be fine, however it'll definitely break existing
> user interface (as it'll expect -EEXIST for now - we have kselftest covers
> that), meanwhile I don't see why the -EEXIST bothers a lot: it still tells the
> user that this page was filled in already.  Normally it was filled in by
> another UFFDIO_COPY (as we could have multiple uffd service threads) along with
> a valid pte, then this userspace thread can simply skip this message as it
> means the event has been handled by some other servicing thread.
> 
> (This also reminded me that there won't be a chance of UFFDIO_COPY race on page
>  no page fault at least, since no page fault will always go into the uffd
>  missing handling rather than filling in the page cache for a VM_UFFD_MISSING
>  vma; while mmap read lock should guarantee VM_UFFD_MISSING be persistent)

Perhaps I am missing something.

Since this is a shared mapping, can we not have a 'regular' mapping to
the same range that is uffd registered?  And, that regular mappings could
fault and race with the uffd copy code?

-- 
Mike Kravetz