linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Rik van Riel <riel@redhat.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Michal Hocko <mhocko@kernel.org>,
	Mike Kravetz <mike.kravetz@oracle.com>,
	linux-mm <linux-mm@kvack.org>,
	Florian Weimer <fweimer@redhat.com>,
	colm@allcosts.net, Andrew Morton <akpm@linux-foundation.org>,
	Kees Cook <keescook@chromium.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Will Drewry <wad@chromium.org>, Ingo Molnar <mingo@kernel.org>,
	"Kirill A. Shutemov" <kirill@shutemov.name>,
	Dave Hansen <dave.hansen@intel.com>,
	Linux API <linux-api@vger.kernel.org>,
	Matthew Wilcox <willy@infradead.org>
Subject: Re: [PATCH 2/2] mm,fork: introduce MADV_WIPEONFORK
Date: Fri, 11 Aug 2017 12:42:38 -0700	[thread overview]
Message-ID: <CA+55aFzA+7CeCdUi-13DfOeE3FfhtTPMMmBA4UQx8FixXiD4YA@mail.gmail.com> (raw)
In-Reply-To: <20170811191942.17487-3-riel@redhat.com>

On Fri, Aug 11, 2017 at 12:19 PM,  <riel@redhat.com> wrote:
> diff --git a/mm/memory.c b/mm/memory.c
> index 0e517be91a89..f9b0ad7feb57 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1134,6 +1134,16 @@ int copy_page_range(struct mm_struct *dst_mm, struct mm_struct *src_mm,
>                         !vma->anon_vma)
>                 return 0;
>
> +       /*
> +        * With VM_WIPEONFORK, the child inherits the VMA from the
> +        * parent, but not its contents.
> +        *
> +        * A child accessing VM_WIPEONFORK memory will see all zeroes;
> +        * a child accessing VM_DONTCOPY memory receives a segfault.
> +        */
> +       if (vma->vm_flags & VM_WIPEONFORK)
> +               return 0;
> +

Is this right?

Yes, you don't do the page table copies. Fine. But you leave vma with
the the anon_vma pointer - doesn't that mean that it's still connected
to the original anonvma chain, and we might end up swapping something
in?

And even if that ends up not being an issue, I'd expect that you'd
want to break the anon_vma chain just to not make it grow
unnecessarily.

So my gut feel is that doing this in "copy_page_range()" is wrong, and
the logic should be moved up to dup_mmap(), where we can also
short-circuit the anon_vma chain entirely.

No?

The madvice() interface looks fine to me.

                  Linus

  reply	other threads:[~2017-08-11 19:42 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-11 19:19 [PATCH v3 0/2] mm,fork,security: introduce MADV_WIPEONFORK riel
2017-08-11 19:19 ` [PATCH 1/2] x86,mpx: make mpx depend on x86-64 to free up VMA flag riel
2017-08-11 19:19 ` [PATCH 2/2] mm,fork: introduce MADV_WIPEONFORK riel
2017-08-11 19:42   ` Linus Torvalds [this message]
2017-08-11 20:27     ` Rik van Riel
2017-08-11 20:50       ` Linus Torvalds
  -- strict thread matches above, loose matches on Subject: below --
2017-08-11 21:28 [PATCH v4 0/2] mm,fork,security: " riel
2017-08-11 21:28 ` [PATCH 2/2] mm,fork: " riel
2017-08-15 22:51   ` Andrew Morton
2017-08-16  2:18     ` Rik van Riel
2017-08-17 22:50       ` Andrew Morton
2017-08-18 16:28         ` Rik van Riel
2017-08-18 18:15           ` Andrew Morton
2017-08-19  0:02             ` Rik van Riel
2017-08-18 17:25   ` Mike Kravetz
2017-08-06 14:04 [PATCH v2 0/2] mm,fork,security: " riel
2017-08-06 14:04 ` [PATCH 2/2] mm,fork: " riel
2017-08-10 15:23   ` Michal Hocko
2017-08-11 15:23     ` Rik van Riel
2017-08-11 16:36       ` Mike Kravetz
2017-08-11 16:59         ` Rik van Riel
2017-08-11 17:07           ` Mike Kravetz
2017-08-04 19:07 [PATCH 0/2] mm,fork,security: " riel
2017-08-04 19:07 ` [PATCH 2/2] mm,fork: " riel
2017-08-04 23:09   ` Mike Kravetz
2017-08-05 14:05     ` Rik van Riel
2017-08-14 15:45   ` kbuild test robot
2017-08-04 19:01 [PATCH 0/2] mm,fork: MADV_WIPEONFORK - an empty VMA in the child riel
2017-08-04 19:01 ` [PATCH 2/2] mm,fork: introduce MADV_WIPEONFORK riel
2017-08-05 18:46   ` kbuild test robot
2017-08-05 19:33   ` kbuild test robot

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=CA+55aFzA+7CeCdUi-13DfOeE3FfhtTPMMmBA4UQx8FixXiD4YA@mail.gmail.com \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=colm@allcosts.net \
    --cc=dave.hansen@intel.com \
    --cc=fweimer@redhat.com \
    --cc=keescook@chromium.org \
    --cc=kirill@shutemov.name \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@amacapital.net \
    --cc=mhocko@kernel.org \
    --cc=mike.kravetz@oracle.com \
    --cc=mingo@kernel.org \
    --cc=riel@redhat.com \
    --cc=wad@chromium.org \
    --cc=willy@infradead.org \
    /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).