Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 13 Nov 2022 18:56:30 -0500
From:   Peter Xu <peterx@redhat.com>
To:     Alistair Popple <apopple@nvidia.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        Andrea Arcangeli <aarcange@redhat.com>,
        Axel Rasmussen <axelrasmussen@google.com>,
        Ives van Hoorne <ives@codesandbox.io>,
        Nadav Amit <nadav.amit@gmail.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Mike Rapoport <rppt@linux.vnet.ibm.com>, stable@vger.kernel.org
Subject: Re: [PATCH v2 1/2] mm/migrate: Fix read-only page got writable when
 recover pte
Message-ID: <Y3GELocAJ+p+gKpc@x1n>
References: <20221110203132.1498183-1-peterx@redhat.com>
 <20221110203132.1498183-2-peterx@redhat.com>
 <87tu36icej.fsf@nvidia.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <87tu36icej.fsf@nvidia.com>
Precedence: bulk

On Fri, Nov 11, 2022 at 10:42:13AM +1100, Alistair Popple wrote:
> Hi Peter, for the patch feel free to add:
> 
> Reviewed-by: Alistair Popple <apopple@nvidia.com>

Will do, thanks.

> 
> I did wonder if this should be backported further for migrate_vma as
> well given that a migration failure there might lead a shmem read-only
> PTE to become read-write. I couldn't think of an obvious reason why that
> would cause an actual problem though.
> 
> I think folio_mkclean() will wrprotect the pte for writeback to swap,
> but it holds the page lock which prevents migrate_vma installing
> migration entries in the first place.
> 
> I suppose there is a small window there because migrate_vma will unlock
> the page before removing the migration entries. So to be safe we could
> consider going back to 8763cb45ab96 ("mm/migrate: new memory migration
> helper for use with device memory") but I doubt in practice it's a real
> problem.

IIRC migrate_vma API only supports anonymous memory, then it's not
affected by this issue?

One thing reminded me is I thought mprotect could be affected but I think
it's actually not, because mprotect is vma-based, and that should always be
fine with current mk_pte().  I'll remove the paragraph on mprotect in the
commit message; that could be slightly misleading.

-- 
Peter Xu