linux-kernel - Re: [PATCH 2/2] mm,fork: introduce MADV

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:   Fri, 11 Aug 2017 12:42:38 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Rik van Riel <riel@...hat.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Michal Hocko <mhocko@...nel.org>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        linux-mm <linux-mm@...ck.org>,
        Florian Weimer <fweimer@...hat.com>, colm@...costs.net,
        Andrew Morton <akpm@...ux-foundation.org>,
        Kees Cook <keescook@...omium.org>,
        Andy Lutomirski <luto@...capital.net>,
        Will Drewry <wad@...omium.org>, Ingo Molnar <mingo@...nel.org>,
        "Kirill A. Shutemov" <kirill@...temov.name>,
        Dave Hansen <dave.hansen@...el.com>,
        Linux API <linux-api@...r.kernel.org>,
        Matthew Wilcox <willy@...radead.org>
Subject: Re: [PATCH 2/2] mm,fork: introduce MADV_WIPEONFORK

On Fri, Aug 11, 2017 at 12:19 PM,  <riel@...hat.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