Currently mremap folio pte batch ignores the writable bit during figuring out a set of similar ptes mapping the same folio. Suppose that the first pte of the batch is writable while the others are not - set_ptes will end up setting the writable bit on the other ptes, which is a violation of mremap semantics. Therefore, use FPB_RESPECT_WRITE to check the writable bit while determining the pte batch.
Cc: stable@vger.kernel.org #6.17 Fixes: f822a9a81a31 ("mm: optimize mremap() by PTE batching") Reported-by: David Hildenbrand david@redhat.com Debugged-by: David Hildenbrand david@redhat.com Signed-off-by: Dev Jain dev.jain@arm.com --- mm-selftests pass. Based on mm-new. Need David H. to confirm whether the repro passes.
mm/mremap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/mremap.c b/mm/mremap.c index a7f531c17b79..8ad06cf50783 100644 --- a/mm/mremap.c +++ b/mm/mremap.c @@ -187,7 +187,7 @@ static int mremap_folio_pte_batch(struct vm_area_struct *vma, unsigned long addr if (!folio || !folio_test_large(folio)) return 1;
- return folio_pte_batch(folio, ptep, pte, max_nr); + return folio_pte_batch_flags(folio, NULL, ptep, &pte, max_nr, FPB_RESPECT_WRITE); }
static int move_ptes(struct pagetable_move_control *pmc,
On Tue, Oct 28, 2025 at 12:09:52PM +0530, Dev Jain wrote:
Currently mremap folio pte batch ignores the writable bit during figuring out a set of similar ptes mapping the same folio. Suppose that the first pte of the batch is writable while the others are not - set_ptes will end up setting the writable bit on the other ptes, which is a violation of mremap semantics. Therefore, use FPB_RESPECT_WRITE to check the writable bit while determining the pte batch.
Hmm, it seems to be like we're doing the wrong thing by default here? I must admit I haven't followed the contpte work as much as I would've liked, but it doesn't make much sense to me why FPB_RESPECT_WRITE would be an option you have to explicitly pass, and where folio_pte_batch (the "simple" interface) doesn't Just Do The Right Thing for naive callers.
Auditing all callers: - khugepaged clears a variable number of ptes - memory.c clears a variable number of ptes - mempolicy.c grabs folios for migrations - mlock.c steps over nr_ptes - 1 ptes, speeding up traversal - mremap is borked since we're remapping nr_ptes ptes - rmap.c TTU unmaps nr_ptes ptes for a given folio
so while the vast majority of callers don't seem to care, it would make sense that folio_pte_batch() works conservatively by default, and folio_pte_batch_flags() would allow for further batching (or maybe we would add a separate folio_pte_batch_clear() or folio_pte_batch_greedy() or whatnot).
Cc: stable@vger.kernel.org #6.17 Fixes: f822a9a81a31 ("mm: optimize mremap() by PTE batching") Reported-by: David Hildenbrand david@redhat.com Debugged-by: David Hildenbrand david@redhat.com Signed-off-by: Dev Jain dev.jain@arm.com
But the solution itself looks okay to me. so, fwiw:
Acked-by: Pedro Falcato pfalcato@suse.de
On 28.10.25 12:48, Pedro Falcato wrote:
On Tue, Oct 28, 2025 at 12:09:52PM +0530, Dev Jain wrote:
Currently mremap folio pte batch ignores the writable bit during figuring out a set of similar ptes mapping the same folio. Suppose that the first pte of the batch is writable while the others are not - set_ptes will end up setting the writable bit on the other ptes, which is a violation of mremap semantics. Therefore, use FPB_RESPECT_WRITE to check the writable bit while determining the pte batch.
Hmm, it seems to be like we're doing the wrong thing by default here? I must admit I haven't followed the contpte work as much as I would've liked, but it doesn't make much sense to me why FPB_RESPECT_WRITE would be an option you have to explicitly pass, and where folio_pte_batch (the "simple" interface) doesn't Just Do The Right Thing for naive callers.
We use the "simple" version to apply to as many callers as possible: the common case, not some "let's be super careful" scenarios.
Auditing all callers:
- khugepaged clears a variable number of ptes
- memory.c clears a variable number of ptes
- mempolicy.c grabs folios for migrations
- mlock.c steps over nr_ptes - 1 ptes, speeding up traversal
- mremap is borked since we're remapping nr_ptes ptes
- rmap.c TTU unmaps nr_ptes ptes for a given folio
so while the vast majority of callers don't seem to care, it would make sense that folio_pte_batch() works conservatively by default, and folio_pte_batch_flags() would allow for further batching (or maybe we would add a separate folio_pte_batch_clear() or folio_pte_batch_greedy() or whatnot).
I think really the tricky part is when we'e not only scanning or clearing, but actually want to "set" ptes again based on the result, like we do here.
For that we could consider having a second variant. But if it ends up having only a single caller, it's also not that great.
Cc: stable@vger.kernel.org #6.17 Fixes: f822a9a81a31 ("mm: optimize mremap() by PTE batching") Reported-by: David Hildenbrand david@redhat.com Debugged-by: David Hildenbrand david@redhat.com Signed-off-by: Dev Jain dev.jain@arm.com
But the solution itself looks okay to me. so, fwiw:
Acked-by: Pedro Falcato pfalcato@suse.de
Backport might end up being a bit tricky I suspect.
Acked-by: David Hildenbrand david@redhat.com
You'll probably get a tag just from using the British English spelling of 'honour' from me :P (joking! ;)
On Tue, Oct 28, 2025 at 12:09:52PM +0530, Dev Jain wrote:
Currently mremap folio pte batch ignores the writable bit during figuring out a set of similar ptes mapping the same folio. Suppose that the first pte of the batch is writable while the others are not - set_ptes will end up setting the writable bit on the other ptes, which is a violation of mremap semantics. Therefore, use FPB_RESPECT_WRITE to check the writable bit while determining the pte batch.
Yikes.
Cc: stable@vger.kernel.org #6.17 Fixes: f822a9a81a31 ("mm: optimize mremap() by PTE batching") Reported-by: David Hildenbrand david@redhat.com Debugged-by: David Hildenbrand david@redhat.com Signed-off-by: Dev Jain dev.jain@arm.com
LGTM, so:
Reviewed-by: Lorenzo Stoakes lorenzo.stoakes@oracle.com
mm-selftests pass. Based on mm-new. Need David H. to confirm whether the repro passes.
Given he A-b'd I assume it did :)
mm/mremap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/mremap.c b/mm/mremap.c index a7f531c17b79..8ad06cf50783 100644 --- a/mm/mremap.c +++ b/mm/mremap.c @@ -187,7 +187,7 @@ static int mremap_folio_pte_batch(struct vm_area_struct *vma, unsigned long addr if (!folio || !folio_test_large(folio)) return 1;
- return folio_pte_batch(folio, ptep, pte, max_nr);
- return folio_pte_batch_flags(folio, NULL, ptep, &pte, max_nr, FPB_RESPECT_WRITE);
}
static int move_ptes(struct pagetable_move_control *pmc,
2.30.2
linux-stable-mirror@lists.linaro.org
-
David Hildenbrand -
Dev Jain -
Lorenzo Stoakes -
Pedro Falcato