linux-kernel - Re: [PATCH v1 4/7] mm/ksm: fix KSM COW breaking with userfaultfd-wp via FAULT_FLAG

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:   Thu, 20 Oct 2022 12:05:29 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Peter Xu <peterx@...hat.com>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        linux-kselftest@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Shuah Khan <shuah@...nel.org>, Hugh Dickins <hughd@...gle.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Andrea Arcangeli <aarcange@...hat.com>,
        "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        Jason Gunthorpe <jgg@...dia.com>,
        John Hubbard <jhubbard@...dia.com>
Subject: Re: [PATCH v1 4/7] mm/ksm: fix KSM COW breaking with userfaultfd-wp
 via FAULT_FLAG_UNSHARE

Hi Peter,

sorry for replying so late, I only managed now to get back to this patch 
set.

>> Yes, I can give it a try. What I dislike about ksm_test is that it's a
>> mixture of benchmarks and test cases that have to explicitly triggered by
>> parameters. It's not a simple "run all available test cases" tests as we
>> know it. So maybe something separate (or having it as part of the uffd
>> tests) makes more sense.
> 
> We can add an entry into run_vmtests.sh.  That's also what current ksm_test
> does.

Right, but I kind-of don't like that way at all and would much rather do 
it cleaner...

> 
> Yes adding into uffd test would work too, but I do have a plan that we
> should move functional tests out of userfaultfd.c, leaving that with the
> stress test only.  Not really a big deal, though.

... similar to what you want to do with userfaultfd.c

So maybe I'll just add a new test for ksm functional tests.

> 
>>
>>>
>>>>
>>>> Consequently, we will no longer trigger a fake write fault and break COW
>>>> without any such side-effects.
>>>>
>>>> This is primarily a fix for KSM+userfaultfd-wp, however, the fake write
>>>> fault was always questionable. As this fix is not easy to backport and it's
>>>> not very critical, let's not cc stable.
>>>
>>> A patch to cc most of the stable would probably need to still go with the
>>> old write approach but attaching ALLOW_RETRY.  But I agree maybe that may
>>> not need to bother, or a report should have arrived earlier..  The unshare
>>> approach looks much cleaner indeed.
>>
>> A fix without FAULT_FLAG_UNSHARE is not straight forward. We really don't
>> want to notify user space about write events here (because there is none).
>> And there is no way around the uffd handling in WP code without that.
>>
>> FAULT_FLAG_ALLOW_RETRY would rely on userfaultfd triggering and having to
>> resolve the WP event.
> 
> Right it'll be very much a false positive, but the userspace should be fine
> with it e.g. for live snapshot we need to copy page earlier; it still won't
> stop the process from running along the way.  But I agree that's not ideal.

At least the test case at hand will wait until infinitely, because there 
is no handler that would allow break_cow to make progress (well, we 
don't expect write events in the test :) ).

Anyhow, I don't think messing with that for stable kernels is worth the 
pain / complexity / possible issues.

-- 
Thanks,

David / dhildenb