Re: [PATCH 2/2] mm: soft_dirty: userfaultfd: introduce wrprotect_tlb_flush_pending

[Andrea Arcangeli <aarcange@redhat.com>]

On Thu, Jan 07, 2021 at 02:42:17PM -0800, Linus Torvalds wrote:
> On Thu, Jan 7, 2021 at 2:31 PM Andrea Arcangeli <aarcange@redhat.com> wrote:
> >
> > Random memory corruption will still silently materialize as result of
> > the speculative lookups in the above scenario.
> 
> Explain.
> 
> Yes, you'll get random memory corruption if you keep doing wrprotect()
> without mmap_sem held for writing.

I didn't meant that.

> But I thought we agreed earlier that that is a bug. And I thought the
> softdirty code already got it for writing.

softdirty used mmap_read_lock too but this again isn't relevant here
and for the sake of discussion we can safely assume mmap_read_lock
doesn't exist in the kernel, and everything takes the mmap_write_lock
whenever a mmap_lock is taken at all.

I mean something bad will happen if a write happens, but soft dirty
cannot register it because we didn't wrprotect the pte? Some dirty
page won't be transferred to destination and it will be assumed there
was no softy dirty event for such page? Otherwise it would mean that
wrprotecting is simply optional for all pages under clear_refs?

Not doing the final TLB flush in softdirty caused some issue even when
there was no COW and the deferred flush only would delay the wrprotect
fault:

       https://lore.kernel.org/linux-mm/CA+32v5zzFYJQ7eHfJP-2OHeR+6p5PZsX=RDJNU6vGF3hLO+j-g@mail.gmail.com/
       https://lore.kernel.org/linux-mm/20210105221628.GA12854@willie-the-truck/

Skipping the wrprotection of the pte because of a speculative
pagecache lookup elevating a random page_count, from the userland
point of view, I guessed would behave as missing the final TLB flush
before clear_refs returns to userland, just worse.

Thanks,
Andrea