linux-kernel - Re: [PATCH v2 2/3] userfaultfd: 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:   Mon, 2 Oct 2023 09:49:38 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Peter Xu <peterx@...hat.com>
Cc:     Suren Baghdasaryan <surenb@...gle.com>,
        Jann Horn <jannh@...gle.com>, akpm@...ux-foundation.org,
        viro@...iv.linux.org.uk, brauner@...nel.org, shuah@...nel.org,
        aarcange@...hat.com, lokeshgidra@...gle.com, hughd@...gle.com,
        mhocko@...e.com, axelrasmussen@...gle.com, rppt@...nel.org,
        willy@...radead.org, Liam.Howlett@...cle.com,
        zhangpeng362@...wei.com, bgeffon@...gle.com,
        kaleshsingh@...gle.com, ngeoffray@...gle.com, jdduke@...gle.com,
        linux-mm@...ck.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        kernel-team@...roid.com
Subject: Re: [PATCH v2 2/3] userfaultfd: UFFDIO_REMAP uABI

On 28.09.23 21:00, Peter Xu wrote:
> On Thu, Sep 28, 2023 at 07:15:13PM +0200, David Hildenbrand wrote:
>> There are some interesting questions to ask here:
>>
>> 1) What happens if the old VMA has VM_SOFTDIRTY set but the new one not? You
>> most probably have to mark the PTE softdirty and not make it writable.
> 
> I don't know whether anyone would care about soft-dirty used with uffd
> remap, but if to think about it..
> 
> Logically if the dst vma has !SOFTDIRTY (means, soft-dirty tracking
> enabled), then IIUC the right thing to do is to assume this page is
> modified, hence mark softdirty and perhaps proceed with other checks (where
> write bit can be set if all check pass)?

I think so, yes.

> 
> Because from a soft-dirty monitor POV on dst_vma I see this REMAP the same
> as writting data onto the missing page and got a page fault
> (e.g. UFFDIO_COPY); we just avoided the allocation and copy.
> 
> The src vma seems also fine in this regard: soft-dirty should ignore holes
> always anyway (e.g. DONTNEED on a page should report !soft-dirty later even
> if tracking).

Sounds good to me.

> 
>>
>> 2) VM_UFFD_WP requires similar care I assume? Peter might know.
> 
> UFFD_WP shouldn't be affected, iiuc.
> 
> Let's first discuss dst vma side.
> 
> WP_UNPOPULATED made it slightly complicated but not so much.  The core
> should be that REMAP only installs pages if it's exactly pte_none():
> 
> +       if (!pte_none(orig_dst_pte)) {
> +               err = -EEXIST;
> +               goto out;
> +       }
> 
> Then it already covers things like pte markers, and any marker currently
> will fail the REMAP ioctl already.  May not be always wanted, but no risk
> of losing wp notifications.  If that'll be a valid use case we can work it
> out.

Agreed.

> 
> On src vma, REMAP ioctl should behave the same as DONTNEED.  Now we drop
> the src pte along with the uffd-wp bit even if set, which is the correct
> behavior from that regard.
> 
> Again, I don't know whether anyone cares on any of those, though..

If it's easy to handle, we should just handle it or instead spell it out 
why we believe we can break other features. Seems to be very easy to handle.

-- 
Cheers,

David / dhildenb