Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 24 Sep 2020 11:02:37 -0300
From:   Jason Gunthorpe <jgg@ziepe.ca>
To:     Jan Kara <jack@suse.cz>
Cc:     Peter Xu <peterx@redhat.com>, John Hubbard <jhubbard@nvidia.com>,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Andrew Morton <akpm@linux-foundation.org>,
        Michal Hocko <mhocko@suse.com>,
        Kirill Tkhai <ktkhai@virtuozzo.com>,
        Kirill Shutemov <kirill@shutemov.name>,
        Hugh Dickins <hughd@google.com>,
        Christoph Hellwig <hch@lst.de>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Oleg Nesterov <oleg@redhat.com>,
        Leon Romanovsky <leonro@nvidia.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Jann Horn <jannh@google.com>
Subject: Re: [PATCH 1/5] mm: Introduce mm_struct.has_pinned
Message-ID: <20200924140237.GD9916@ziepe.ca>
References: <224908c1-5d0f-8e01-baa9-94ec2374971f@nvidia.com>
 <20200922151736.GD19098@xz-x1>
 <20200922161046.GB731578@ziepe.ca>
 <20200922175415.GI19098@xz-x1>
 <20200922191116.GK8409@ziepe.ca>
 <20200923002735.GN19098@xz-x1>
 <20200923131043.GA59978@xz-x1>
 <20200923142003.GB15875@quack2.suse.cz>
 <20200923171207.GB9916@ziepe.ca>
 <20200924074409.GB27019@quack2.suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200924074409.GB27019@quack2.suse.cz>
Precedence: bulk

On Thu, Sep 24, 2020 at 09:44:09AM +0200, Jan Kara wrote:
> > After the page is pinned it is prevented from being freed and
> > recycled. After GUP has the pin it must check that the PTE still
> > points at the same page, otherwise it might have pinned a page that is
> > alreay free'd - and that would be a use-after-free issue.
> 
> I don't think a page use-after-free is really the reason - we add page
> reference through page_ref_add_unless(page, x, 0) - i.e., it will fail for
> already freed page. 

I mean, the page could have been freed and already reallocated with a
positive refcount, so the add_unless check isn't protective.

The add_unless prevents the page from being freed. The 2nd pte read
ensures it wasn't already freed/reassigned before the pin.

If something drives the page refcount to zero then it is already
synchronized with GUP fast because of the atomic add_unless, no need
to re-check the pte for that case?? But I don't know what the DAX case
is you mentioned.

Jason