linux-kernel - Re: [PATCH 1/4] mm: Trial do_wp

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, 17 Sep 2020 11:11:06 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Jason Gunthorpe <jgg@...pe.ca>
Cc:     Peter Xu <peterx@...hat.com>, John Hubbard <jhubbard@...dia.com>,
        Leon Romanovsky <leonro@...dia.com>,
        Linux-MM <linux-mm@...ck.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "Maya B . Gokhale" <gokhale2@...l.gov>,
        Yang Shi <yang.shi@...ux.alibaba.com>,
        Marty Mcfadden <mcfadden8@...l.gov>,
        Kirill Shutemov <kirill@...temov.name>,
        Oleg Nesterov <oleg@...hat.com>, Jann Horn <jannh@...gle.com>,
        Jan Kara <jack@...e.cz>, Kirill Tkhai <ktkhai@...tuozzo.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Christoph Hellwig <hch@....de>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 1/4] mm: Trial do_wp_page() simplification

> I botched the last version of the patch, here is something a bit
> better.

So I'd like to understand why this problem happens.

Myt argument to Hugh a few weeks ago was that page pinning should take
care of all this:

 (a) if the pinner is going to change the page, it will have to get
the pin with FOLL_WRITE in addition to FOLL_PIN

 (b) that means it will do the COW and mark the page writable and dirty

 (c) the whole _point_ of the FOLL_PIN is that subsequent operations
shouldn't make it non-writable any more (ie it can't be unmapped, and
we should synchronize on fork etc)

So I get the strong feeling that this whole approach to "fix" COW is
really papering over the real problem.

To me, the whole point of pinning is to avoid issues like this. If it
didn't fix this issue, then why did we go to all the effort of
treating pinned pages differently?

Your patch may avoid the problem, but I think it implies things are
horribly horribly broken in pinning land.

I also note that the _only_ user of page_maybe_dma_pinned() right now
is a debug check in gup_benchmark. I think your patch may _work_, and
I think that thanks to page_maybe_dma_pinned() it might even avoid the
horrible case, but I feel we should just truly fix pinning to be
meaningful instead.

IOW, in addition to keeping the page mapped, it should keep the page writable.

Why isn't it?

                     Linus