linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Xu <peterx@redhat.com>, Nadav Amit <nadav.amit@gmail.com>,
	Yu Zhao <yuzhao@google.com>,
	 Andrea Arcangeli <aarcange@redhat.com>,
	linux-mm <linux-mm@kvack.org>,
	 lkml <linux-kernel@vger.kernel.org>,
	Pavel Emelyanov <xemul@openvz.org>,
	 Mike Kravetz <mike.kravetz@oracle.com>,
	Mike Rapoport <rppt@linux.vnet.ibm.com>,
	 stable <stable@vger.kernel.org>,
	Minchan Kim <minchan@kernel.org>,
	 Andy Lutomirski <luto@kernel.org>, Will Deacon <will@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH] mm/userfaultfd: fix memory corruption due to writeprotect
Date: Mon, 21 Dec 2020 19:19:35 -0800	[thread overview]
Message-ID: <CALCETrV=8tY7h=aaudWBEn-MJnNkm2wz5qjH49SYqwkjYTpOaA@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=wh-bG4thjXUekLtrCg8FRrdWjtT40ibXXLSm_hzQG8eOw@mail.gmail.com>

On Mon, Dec 21, 2020 at 3:22 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Mon, Dec 21, 2020 at 2:30 PM Peter Xu <peterx@redhat.com> wrote:
> >
> > AFAIU mprotect() is the only one who modifies the pte using the mmap write
> > lock.  NUMA balancing is also using read mmap lock when changing pte
> > protections, while my understanding is mprotect() used write lock only because
> > it manipulates the address space itself (aka. vma layout) rather than modifying
> > the ptes, so it needs to.
>
> So it's ok to change the pte holding only the PTE lock, if it's a
> *one*way* conversion.
>
> That doesn't break the "re-check the PTE contents" model (which
> predates _all_ of the rest: NUMA, userfaultfd, everything - it's
> pretty much the original model for our page table operations, and goes
> back to the dark ages even before SMP and the existence of a page
> table lock).
>
> So for example, a COW will always create a different pte (not just
> because the page number itself changes - you could imagine a page
> getting re-used and changing back - but because it's always a RO->RW
> transition).
>
> So two COW operations cannot "undo" each other and fool us into
> thinking nothing changed.
>
> Anything that changes RW->RO - like fork(), for example - needs to
> take the mmap_lock.

Ugh, this is unpleasantly complicated.  I will admit that any API that
takes an address and more-or-less-blindly marks it RO makes me quite
nervous even assuming all the relevant locks are held.  At least
userfaultfd refuses to operate on VM_SHARED VMAs, but we have another
instance of this (with mmap_sem held for write) in x86:
mark_screen_rdonly().  Dare I ask how broken this is?  We could likely
get away with deleting it entirely.

--Andy


  reply	other threads:[~2020-12-22  3:19 UTC|newest]

Thread overview: 120+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-19  4:30 [PATCH] mm/userfaultfd: fix memory corruption due to writeprotect Nadav Amit
2020-12-19 19:15 ` Andrea Arcangeli
     [not found]   ` <EDC00345-B46E-4396-8379-98E943723809@gmail.com>
2020-12-19 22:06     ` Nadav Amit
2020-12-20  2:20       ` Andrea Arcangeli
2020-12-21  4:36         ` Nadav Amit
2020-12-21  5:12           ` Yu Zhao
2020-12-21  5:25             ` Nadav Amit
2020-12-21  5:39               ` Nadav Amit
2020-12-21  7:29                 ` Yu Zhao
2020-12-22 20:34       ` Andy Lutomirski
2020-12-22 20:58         ` Nadav Amit
2020-12-22 21:34           ` Andrea Arcangeli
2020-12-20  2:01     ` Andy Lutomirski
2020-12-20  2:49       ` Andrea Arcangeli
2020-12-20  5:08         ` Andy Lutomirski
2020-12-21 18:03           ` Andrea Arcangeli
2020-12-21 18:22             ` Andy Lutomirski
2020-12-20  6:05     ` Yu Zhao
2020-12-20  8:06       ` Nadav Amit
2020-12-20  9:54         ` Yu Zhao
2020-12-21  3:33           ` Nadav Amit
2020-12-21  4:44             ` Yu Zhao
2020-12-21 17:27         ` Peter Xu
2020-12-21 18:31           ` Nadav Amit
2020-12-21 19:16             ` Yu Zhao
2020-12-21 19:55               ` Linus Torvalds
2020-12-21 20:21                 ` Yu Zhao
2020-12-21 20:25                   ` Linus Torvalds
2020-12-21 20:23                 ` Nadav Amit
2020-12-21 20:26                   ` Linus Torvalds
2020-12-21 21:24                     ` Yu Zhao
2020-12-21 21:49                       ` Nadav Amit
2020-12-21 22:30                         ` Peter Xu
2020-12-21 22:55                           ` Nadav Amit
2020-12-21 23:30                             ` Linus Torvalds
2020-12-21 23:46                               ` Nadav Amit
2020-12-22 19:44                             ` Andrea Arcangeli
2020-12-22 20:19                               ` Nadav Amit
2020-12-22 21:17                                 ` Andrea Arcangeli
2020-12-21 23:12                           ` Yu Zhao
2020-12-21 23:33                             ` Linus Torvalds
2020-12-22  0:00                               ` Yu Zhao
2020-12-22  0:11                                 ` Linus Torvalds
2020-12-22  0:24                                   ` Yu Zhao
2020-12-21 23:22                           ` Linus Torvalds
2020-12-22  3:19                             ` Andy Lutomirski [this message]
2020-12-22  4:16                               ` Linus Torvalds
2020-12-22 20:19                                 ` Andy Lutomirski
2021-01-05 15:37                                 ` Peter Zijlstra
2021-01-05 18:03                                   ` Andrea Arcangeli
2021-01-12 16:20                                     ` Peter Zijlstra
2021-01-12 11:43                                   ` Vinayak Menon
2021-01-12 15:47                                     ` Laurent Dufour
2021-01-12 16:57                                       ` Peter Zijlstra
2021-01-12 19:02                                         ` Laurent Dufour
2021-01-12 19:15                                           ` Nadav Amit
2021-01-12 19:56                                             ` Yu Zhao
2021-01-12 20:38                                               ` Nadav Amit
2021-01-12 20:49                                                 ` Yu Zhao
2021-01-12 21:43                                                 ` Will Deacon
2021-01-12 22:29                                                   ` Nadav Amit
2021-01-12 22:46                                                     ` Will Deacon
2021-01-13  0:31                                                     ` Andy Lutomirski
2021-01-17  4:41                                                   ` Yu Zhao
2021-01-17  7:32                                                     ` Nadav Amit
2021-01-17  9:16                                                       ` Yu Zhao
2021-01-17 10:13                                                         ` Nadav Amit
2021-01-17 19:25                                                           ` Yu Zhao
2021-01-18  2:49                                                             ` Nadav Amit
2020-12-22  9:38                               ` Nadav Amit
2020-12-22 19:31                               ` Andrea Arcangeli
2020-12-22 20:15                                 ` Matthew Wilcox
2020-12-22 20:26                                   ` Andrea Arcangeli
2020-12-22 21:14                                 ` Yu Zhao
2020-12-22 22:02                                   ` Andrea Arcangeli
2020-12-22 23:39                                     ` Yu Zhao
2020-12-22 23:50                                       ` Linus Torvalds
2020-12-23  0:01                                         ` Linus Torvalds
2020-12-23  0:23                                           ` Yu Zhao
2020-12-23  2:17                                             ` Andrea Arcangeli
2020-12-23  9:44                                           ` Linus Torvalds
2020-12-23 10:06                                             ` Yu Zhao
2020-12-23 16:24                                               ` Peter Xu
2020-12-23 18:51                                                 ` Andrea Arcangeli
2020-12-23 18:55                                                   ` Andrea Arcangeli
2020-12-23 19:12                                                 ` Yu Zhao
2020-12-23 19:32                                                   ` Peter Xu
2020-12-23  0:20                                         ` Linus Torvalds
2020-12-23  2:56                                       ` Andrea Arcangeli
2020-12-23  3:36                                         ` Yu Zhao
2020-12-23 15:52                                           ` Peter Xu
2020-12-23 21:07                                             ` Andrea Arcangeli
2020-12-23 21:39                                           ` Andrea Arcangeli
2020-12-23 22:29                                             ` Yu Zhao
2020-12-23 23:04                                               ` Andrea Arcangeli
2020-12-24  1:21                                               ` Andy Lutomirski
2020-12-24  2:00                                                 ` Andrea Arcangeli
2020-12-24  3:09                                                   ` Nadav Amit
2020-12-24  3:30                                                     ` Nadav Amit
2020-12-24  3:34                                                     ` Yu Zhao
2020-12-24  4:01                                                       ` Andrea Arcangeli
2020-12-24  5:18                                                         ` Nadav Amit
2020-12-24 18:49                                                           ` Andrea Arcangeli
2020-12-24 19:16                                                             ` Andrea Arcangeli
2020-12-24  4:37                                                       ` Nadav Amit
2020-12-24  3:31                                                   ` Andrea Arcangeli
2020-12-23 23:39                                             ` Linus Torvalds
2020-12-24  1:01                                               ` Andrea Arcangeli
2020-12-22 21:14                                 ` Nadav Amit
2020-12-22 12:40                       ` Nadav Amit
2020-12-22 18:30                         ` Yu Zhao
2020-12-22 19:20                           ` Nadav Amit
2020-12-23 16:23                             ` Will Deacon
2020-12-23 19:04                               ` Nadav Amit
2020-12-23 22:05                         ` Andrea Arcangeli
2020-12-23 22:45                           ` Nadav Amit
2020-12-23 23:55                             ` Andrea Arcangeli
2020-12-21 21:55                   ` Peter Xu
2020-12-21 23:13                     ` Linus Torvalds
2020-12-21 19:53             ` Peter Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CALCETrV=8tY7h=aaudWBEn-MJnNkm2wz5qjH49SYqwkjYTpOaA@mail.gmail.com' \
    --to=luto@kernel.org \
    --cc=aarcange@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mike.kravetz@oracle.com \
    --cc=minchan@kernel.org \
    --cc=nadav.amit@gmail.com \
    --cc=peterx@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rppt@linux.vnet.ibm.com \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=will@kernel.org \
    --cc=xemul@openvz.org \
    --cc=yuzhao@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).