Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm/migrate_device: avoid overflowing migrate_vma collection arrays
@ 2026-07-08  1:50 Zi Yan
  2026-07-08  2:45 ` Balbir Singh
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Zi Yan @ 2026-07-08  1:50 UTC (permalink / raw)
  To: Andrew Morton, David Hildenbrand, Matthew Brost, Joshua Hahn,
	Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Balbir Singh, Kefeng Wang
  Cc: linux-mm, linux-kernel, Sashiko, Zi Yan

migrate_vma_collect_pmd() can drop pte lock to split a large folio and
restart. But the code does not handle restart properly when pmd becomes
huge or cleared. It can overflow migrate->dst and migrate->src arrays
during the hole or skip collection. Fix it by:
1. avoiding migrate_vma_collect_huge_pmd() if some collection is done,
2. skipping the rest of the range if pmd no longer points to a pte page
   table.

Fixes: a30b48bf1b244 ("mm/migrate_device: implement THP migration of zone device pages")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260706111958.3649651-1-wangkefeng.wang@huawei.com
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Zi Yan <ziy@nvidia.com>
---
The issue is spot by Sashiko during a patch[1] review as a pre-existing one.
This patch has the minimal change. An alternative is to reset
migrate->cpages and migrate->npages and restart the whole range from the
beginning, but that also requires a restoration of no-longer-present PTEs.

Link: https://lore.kernel.org/all/20260706111958.3649651-1-wangkefeng.wang@huawei.com/ [1]
---
 mm/migrate_device.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index 2fffeb1f99694..6ceb47ec1da24 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -257,7 +257,12 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
 	pte_t *ptep;
 
 again:
-	if (pmd_trans_huge(*pmdp) || !pmd_present(*pmdp)) {
+	/*
+	 * Only check pmd when addr is at start, namely no pte is collected.
+	 * It avoids collecting the same address range [start, addr) twice
+	 * and overflowing the collection arrays.
+	 */
+	if (addr == start && (pmd_trans_huge(*pmdp) || !pmd_present(*pmdp))) {
 		int ret = migrate_vma_collect_huge_pmd(pmdp, start, end, walk, fault_folio);
 
 		if (ret == -EAGAIN)
@@ -267,8 +272,18 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
 	}
 
 	ptep = pte_offset_map_lock(mm, pmdp, start, &ptl);
-	if (!ptep)
+	if (!ptep) {
+		/*
+		 * Skip the rest if pmd becomes huge or cleared. Flush if any
+		 * pte is modified
+		 */
+		if (addr != start) {
+			if (unmapped)
+				flush_tlb_range(walk->vma, start, end);
+			return migrate_vma_collect_skip(addr, end, walk);
+		}
 		goto again;
+	}
 	lazy_mmu_mode_enable();
 	ptep += (addr - start) / PAGE_SIZE;
 

---
base-commit: 302dfbff3e73be2ba5723237c6303244ec27cebb
change-id: 20260707-fix-array-overflow-in-migrate_vma_collect_pmd-c504b92613de

Best regards,
-- 
Yan, Zi



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm/migrate_device: avoid overflowing migrate_vma collection arrays
  2026-07-08  1:50 [PATCH] mm/migrate_device: avoid overflowing migrate_vma collection arrays Zi Yan
@ 2026-07-08  2:45 ` Balbir Singh
  2026-07-08  8:43 ` David Hildenbrand (Arm)
  2026-07-10  2:54 ` Zi Yan
  2 siblings, 0 replies; 9+ messages in thread
From: Balbir Singh @ 2026-07-08  2:45 UTC (permalink / raw)
  To: Zi Yan
  Cc: Andrew Morton, David Hildenbrand, Matthew Brost, Joshua Hahn,
	Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Kefeng Wang, linux-mm, linux-kernel, Sashiko

On Tue, Jul 07, 2026 at 09:50:20PM -0400, Zi Yan wrote:
> migrate_vma_collect_pmd() can drop pte lock to split a large folio and
> restart. But the code does not handle restart properly when pmd becomes
> huge or cleared. It can overflow migrate->dst and migrate->src arrays
> during the hole or skip collection. Fix it by:
> 1. avoiding migrate_vma_collect_huge_pmd() if some collection is done,
> 2. skipping the rest of the range if pmd no longer points to a pte page
>    table.
>
> Fixes: a30b48bf1b244 ("mm/migrate_device: implement THP migration of zone device pages")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260706111958.3649651-1-wangkefeng.wang@huawei.com
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
> The issue is spot by Sashiko during a patch[1] review as a pre-existing one.
> This patch has the minimal change. An alternative is to reset
> migrate->cpages and migrate->npages and restart the whole range from the
> beginning, but that also requires a restoration of no-longer-present PTEs.
> 
> Link: https://lore.kernel.org/all/20260706111958.3649651-1-wangkefeng.wang@huawei.com/ [1]
> ---
>  mm/migrate_device.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index 2fffeb1f99694..6ceb47ec1da24 100644
> --- a/mm/migrate_device.c
> +++ b/mm/migrate_device.c
> @@ -257,7 +257,12 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
>  	pte_t *ptep;
>  
>  again:
> -	if (pmd_trans_huge(*pmdp) || !pmd_present(*pmdp)) {
> +	/*
> +	 * Only check pmd when addr is at start, namely no pte is collected.
> +	 * It avoids collecting the same address range [start, addr) twice
> +	 * and overflowing the collection arrays.
> +	 */
> +	if (addr == start && (pmd_trans_huge(*pmdp) || !pmd_present(*pmdp))) {
>  		int ret = migrate_vma_collect_huge_pmd(pmdp, start, end, walk, fault_folio);
>  
>  		if (ret == -EAGAIN)
> @@ -267,8 +272,18 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
>  	}
>  
>  	ptep = pte_offset_map_lock(mm, pmdp, start, &ptl);
> -	if (!ptep)
> +	if (!ptep) {
> +		/*
> +		 * Skip the rest if pmd becomes huge or cleared. Flush if any
> +		 * pte is modified
> +		 */
> +		if (addr != start) {
> +			if (unmapped)
> +				flush_tlb_range(walk->vma, start, end);
> +			return migrate_vma_collect_skip(addr, end, walk);
> +		}
>  		goto again;
> +	}
>  	lazy_mmu_mode_enable();
>  	ptep += (addr - start) / PAGE_SIZE;
>  
> 

Looks good, Thanks!

Reviewed-by: Balbir Singh <balbirs@nvidia.com>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm/migrate_device: avoid overflowing migrate_vma collection arrays
  2026-07-08  1:50 [PATCH] mm/migrate_device: avoid overflowing migrate_vma collection arrays Zi Yan
  2026-07-08  2:45 ` Balbir Singh
@ 2026-07-08  8:43 ` David Hildenbrand (Arm)
  2026-07-08 14:35   ` Zi Yan
  2026-07-10  2:54 ` Zi Yan
  2 siblings, 1 reply; 9+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-08  8:43 UTC (permalink / raw)
  To: Zi Yan, Andrew Morton, Matthew Brost, Joshua Hahn, Rakie Kim,
	Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
	Balbir Singh, Kefeng Wang
  Cc: linux-mm, linux-kernel, Sashiko

On 7/8/26 03:50, Zi Yan wrote:
> migrate_vma_collect_pmd() can drop pte lock to split a large folio and
> restart. But the code does not handle restart properly when pmd becomes
> huge or cleared. It can overflow migrate->dst and migrate->src arrays
> during the hole or skip collection. Fix it by:
> 1. avoiding migrate_vma_collect_huge_pmd() if some collection is done,
> 2. skipping the rest of the range if pmd no longer points to a pte page
>    table.
> 
> Fixes: a30b48bf1b244 ("mm/migrate_device: implement THP migration of zone device pages")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260706111958.3649651-1-wangkefeng.wang@huawei.com
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
> The issue is spot by Sashiko during a patch[1] review as a pre-existing one.
> This patch has the minimal change. An alternative is to reset
> migrate->cpages and migrate->npages and restart the whole range from the
> beginning, but that also requires a restoration of no-longer-present PTEs.
> 
> Link: https://lore.kernel.org/all/20260706111958.3649651-1-wangkefeng.wang@huawei.com/ [1]
> ---
>  mm/migrate_device.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index 2fffeb1f99694..6ceb47ec1da24 100644
> --- a/mm/migrate_device.c
> +++ b/mm/migrate_device.c
> @@ -257,7 +257,12 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
>  	pte_t *ptep;
>  
>  again:
> -	if (pmd_trans_huge(*pmdp) || !pmd_present(*pmdp)) {
> +	/*
> +	 * Only check pmd when addr is at start, namely no pte is collected.
> +	 * It avoids collecting the same address range [start, addr) twice
> +	 * and overflowing the collection arrays.
> +	 */
> +	if (addr == start && (pmd_trans_huge(*pmdp) || !pmd_present(*pmdp))) {
>  		int ret = migrate_vma_collect_huge_pmd(pmdp, start, end, walk, fault_folio);
>  
>  		if (ret == -EAGAIN)
> @@ -267,8 +272,18 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
>  	}
>  
>  	ptep = pte_offset_map_lock(mm, pmdp, start, &ptl);
> -	if (!ptep)
> +	if (!ptep) {
> +		/*
> +		 * Skip the rest if pmd becomes huge or cleared. Flush if any
> +		 * pte is modified
> +		 */
> +		if (addr != start) {
> +			if (unmapped)
> +				flush_tlb_range(walk->vma, start, end);
> +			return migrate_vma_collect_skip(addr, end, walk);
> +		}
>  		goto again;
> +	}
>  	lazy_mmu_mode_enable();
>  	ptep += (addr - start) / PAGE_SIZE;

It's hard to express which feelings reading migrate_vma_collect_pmd() gives me,
haha :)


In case we split ... couldn't we just undo what we already did, before doing the
"goto again" ?

-- 
Cheers,

David


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm/migrate_device: avoid overflowing migrate_vma collection arrays
  2026-07-08  8:43 ` David Hildenbrand (Arm)
@ 2026-07-08 14:35   ` Zi Yan
  2026-07-08 14:43     ` Zi Yan
  0 siblings, 1 reply; 9+ messages in thread
From: Zi Yan @ 2026-07-08 14:35 UTC (permalink / raw)
  To: David Hildenbrand (Arm), Andrew Morton, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Balbir Singh, Kefeng Wang
  Cc: linux-mm, linux-kernel, Sashiko

On Wed Jul 8, 2026 at 4:43 AM EDT, David Hildenbrand (Arm) wrote:
> On 7/8/26 03:50, Zi Yan wrote:
>> migrate_vma_collect_pmd() can drop pte lock to split a large folio and
>> restart. But the code does not handle restart properly when pmd becomes
>> huge or cleared. It can overflow migrate->dst and migrate->src arrays
>> during the hole or skip collection. Fix it by:
>> 1. avoiding migrate_vma_collect_huge_pmd() if some collection is done,
>> 2. skipping the rest of the range if pmd no longer points to a pte page
>>    table.
>> 
>> Fixes: a30b48bf1b244 ("mm/migrate_device: implement THP migration of zone device pages")
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Closes: https://sashiko.dev/#/patchset/20260706111958.3649651-1-wangkefeng.wang@huawei.com
>> Assisted-by: Claude:claude-opus-4-8
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> ---
>> The issue is spot by Sashiko during a patch[1] review as a pre-existing one.
>> This patch has the minimal change. An alternative is to reset
>> migrate->cpages and migrate->npages and restart the whole range from the
>> beginning, but that also requires a restoration of no-longer-present PTEs.
>> 
>> Link: https://lore.kernel.org/all/20260706111958.3649651-1-wangkefeng.wang@huawei.com/ [1]
>> ---
>>  mm/migrate_device.c | 19 +++++++++++++++++--
>>  1 file changed, 17 insertions(+), 2 deletions(-)
>> 
>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>> index 2fffeb1f99694..6ceb47ec1da24 100644
>> --- a/mm/migrate_device.c
>> +++ b/mm/migrate_device.c
>> @@ -257,7 +257,12 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
>>  	pte_t *ptep;
>>  
>>  again:
>> -	if (pmd_trans_huge(*pmdp) || !pmd_present(*pmdp)) {
>> +	/*
>> +	 * Only check pmd when addr is at start, namely no pte is collected.
>> +	 * It avoids collecting the same address range [start, addr) twice
>> +	 * and overflowing the collection arrays.
>> +	 */
>> +	if (addr == start && (pmd_trans_huge(*pmdp) || !pmd_present(*pmdp))) {
>>  		int ret = migrate_vma_collect_huge_pmd(pmdp, start, end, walk, fault_folio);
>>  
>>  		if (ret == -EAGAIN)
>> @@ -267,8 +272,18 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
>>  	}
>>  
>>  	ptep = pte_offset_map_lock(mm, pmdp, start, &ptl);
>> -	if (!ptep)
>> +	if (!ptep) {
>> +		/*
>> +		 * Skip the rest if pmd becomes huge or cleared. Flush if any
>> +		 * pte is modified
>> +		 */
>> +		if (addr != start) {
>> +			if (unmapped)
>> +				flush_tlb_range(walk->vma, start, end);
>> +			return migrate_vma_collect_skip(addr, end, walk);
>> +		}
>>  		goto again;
>> +	}
>>  	lazy_mmu_mode_enable();
>>  	ptep += (addr - start) / PAGE_SIZE;
>
> It's hard to express which feelings reading migrate_vma_collect_pmd() gives me,
> haha :)

I guess I have the same feelings.
>
>
> In case we split ... couldn't we just undo what we already did, before doing the
> "goto again" ?

You mean we reset migrate->cpages and migrate->npages and restart from
the beginning? But it is not only that, since the code below also
changes PTEs into migration entries. We will need to revert them as
well.

Hmm, migrate_vma_collect() documents itself as "update the src array and
"takes a reference on the page" without mentioning changing PTEs. I
wonder why changing PTEs is necessary, since later migrate_vma_unmap()
also changes page table entries to migration entries, although
migrate_vma_unmap() changes all entries to a folio, whereas
migrate_vma_collect() only changes PTEs from the specified VMA.

-- 
Best Regards,
Yan, Zi



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm/migrate_device: avoid overflowing migrate_vma collection arrays
  2026-07-08 14:35   ` Zi Yan
@ 2026-07-08 14:43     ` Zi Yan
  2026-07-10 11:30       ` Zi Yan
  0 siblings, 1 reply; 9+ messages in thread
From: Zi Yan @ 2026-07-08 14:43 UTC (permalink / raw)
  To: David Hildenbrand (Arm), Andrew Morton, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Balbir Singh, Kefeng Wang
  Cc: linux-mm, linux-kernel, Sashiko

On Wed Jul 8, 2026 at 10:35 AM EDT, Zi Yan wrote:
> On Wed Jul 8, 2026 at 4:43 AM EDT, David Hildenbrand (Arm) wrote:
>> On 7/8/26 03:50, Zi Yan wrote:
>>> migrate_vma_collect_pmd() can drop pte lock to split a large folio and
>>> restart. But the code does not handle restart properly when pmd becomes
>>> huge or cleared. It can overflow migrate->dst and migrate->src arrays
>>> during the hole or skip collection. Fix it by:
>>> 1. avoiding migrate_vma_collect_huge_pmd() if some collection is done,
>>> 2. skipping the rest of the range if pmd no longer points to a pte page
>>>    table.
>>> 
>>> Fixes: a30b48bf1b244 ("mm/migrate_device: implement THP migration of zone device pages")
>>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>>> Closes: https://sashiko.dev/#/patchset/20260706111958.3649651-1-wangkefeng.wang@huawei.com
>>> Assisted-by: Claude:claude-opus-4-8
>>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>>> ---
>>> The issue is spot by Sashiko during a patch[1] review as a pre-existing one.
>>> This patch has the minimal change. An alternative is to reset
>>> migrate->cpages and migrate->npages and restart the whole range from the
>>> beginning, but that also requires a restoration of no-longer-present PTEs.
>>> 
>>> Link: https://lore.kernel.org/all/20260706111958.3649651-1-wangkefeng.wang@huawei.com/ [1]
>>> ---
>>>  mm/migrate_device.c | 19 +++++++++++++++++--
>>>  1 file changed, 17 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>>> index 2fffeb1f99694..6ceb47ec1da24 100644
>>> --- a/mm/migrate_device.c
>>> +++ b/mm/migrate_device.c
>>> @@ -257,7 +257,12 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
>>>  	pte_t *ptep;
>>>  
>>>  again:
>>> -	if (pmd_trans_huge(*pmdp) || !pmd_present(*pmdp)) {
>>> +	/*
>>> +	 * Only check pmd when addr is at start, namely no pte is collected.
>>> +	 * It avoids collecting the same address range [start, addr) twice
>>> +	 * and overflowing the collection arrays.
>>> +	 */
>>> +	if (addr == start && (pmd_trans_huge(*pmdp) || !pmd_present(*pmdp))) {
>>>  		int ret = migrate_vma_collect_huge_pmd(pmdp, start, end, walk, fault_folio);
>>>  
>>>  		if (ret == -EAGAIN)
>>> @@ -267,8 +272,18 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
>>>  	}
>>>  
>>>  	ptep = pte_offset_map_lock(mm, pmdp, start, &ptl);
>>> -	if (!ptep)
>>> +	if (!ptep) {
>>> +		/*
>>> +		 * Skip the rest if pmd becomes huge or cleared. Flush if any
>>> +		 * pte is modified
>>> +		 */
>>> +		if (addr != start) {
>>> +			if (unmapped)
>>> +				flush_tlb_range(walk->vma, start, end);
>>> +			return migrate_vma_collect_skip(addr, end, walk);
>>> +		}
>>>  		goto again;
>>> +	}
>>>  	lazy_mmu_mode_enable();
>>>  	ptep += (addr - start) / PAGE_SIZE;
>>
>> It's hard to express which feelings reading migrate_vma_collect_pmd() gives me,
>> haha :)
>
> I guess I have the same feelings.
>>
>>
>> In case we split ... couldn't we just undo what we already did, before doing the
>> "goto again" ?
>
> You mean we reset migrate->cpages and migrate->npages and restart from
> the beginning? But it is not only that, since the code below also
> changes PTEs into migration entries. We will need to revert them as
> well.
>
> Hmm, migrate_vma_collect() documents itself as "update the src array and
> "takes a reference on the page" without mentioning changing PTEs. I
> wonder why changing PTEs is necessary, since later migrate_vma_unmap()
> also changes page table entries to migration entries, although
> migrate_vma_unmap() changes all entries to a folio, whereas
> migrate_vma_collect() only changes PTEs from the specified VMA.

OK, it is an optimization[1] when migrate_vma*() was introduced. If
there is only one mapping, migrate_vma_collect() will set migration
entry immediately without waiting until migrate_vma_unmap(). Fun.

[1] Commit 8c3328f1f36a5 ("mm/migrate: migrate_vma() unmap page from vma while collecting pages")
-- 
Best Regards,
Yan, Zi



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm/migrate_device: avoid overflowing migrate_vma collection arrays
  2026-07-08  1:50 [PATCH] mm/migrate_device: avoid overflowing migrate_vma collection arrays Zi Yan
  2026-07-08  2:45 ` Balbir Singh
  2026-07-08  8:43 ` David Hildenbrand (Arm)
@ 2026-07-10  2:54 ` Zi Yan
  2 siblings, 0 replies; 9+ messages in thread
From: Zi Yan @ 2026-07-10  2:54 UTC (permalink / raw)
  To: Andrew Morton, David Hildenbrand, Matthew Brost, Joshua Hahn,
	Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Balbir Singh, Kefeng Wang
  Cc: linux-mm, linux-kernel, Sashiko, Zi Yan

On Tue Jul 7, 2026 at 9:50 PM EDT, Zi Yan wrote:
> migrate_vma_collect_pmd() can drop pte lock to split a large folio and
> restart. But the code does not handle restart properly when pmd becomes
> huge or cleared. It can overflow migrate->dst and migrate->src arrays
> during the hole or skip collection. Fix it by:
> 1. avoiding migrate_vma_collect_huge_pmd() if some collection is done,
> 2. skipping the rest of the range if pmd no longer points to a pte page
>    table.
>
> Fixes: a30b48bf1b244 ("mm/migrate_device: implement THP migration of zone device pages")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260706111958.3649651-1-wangkefeng.wang@huawei.com
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
> The issue is spot by Sashiko during a patch[1] review as a pre-existing one.
> This patch has the minimal change. An alternative is to reset
> migrate->cpages and migrate->npages and restart the whole range from the
> beginning, but that also requires a restoration of no-longer-present PTEs.
>
> Link: https://lore.kernel.org/all/20260706111958.3649651-1-wangkefeng.wang@huawei.com/ [1]
> ---
>  mm/migrate_device.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
>

Sashiko has a new concern:

https://sashiko.dev/#/patchset/20260707-fix-array-overflow-in-migrate_vma_collect_pmd-v1-1-ce3ff4627653@nvidia.com

Q: migrate_vma_collect_pmd() drops the page table lock without flushing
the TLB leave stale entries active? When an order-0 page is collected and
its present PTE becomes a migration entry without flushing TLBs, later
CPUs with the stale TLB can still write to the page, potentially leading
to data corruption.

Answer: No, migrate_vma_collect_pmd() adds a reference to the collected
pages, so these pages are not going away. In addition, unmapped is
incremented after a page collection and warrants a TLB flush before the
code exits migrate_vma_collect_pmd():

1. pte_offset_map_lock() fails, this patch adds a flush.
2. split fails, a flush is already in the code.
3. the whole range is processed, a flush is at the end of the function.


-- 
Best Regards,
Yan, Zi



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm/migrate_device: avoid overflowing migrate_vma collection arrays
  2026-07-08 14:43     ` Zi Yan
@ 2026-07-10 11:30       ` Zi Yan
  2026-07-10 11:56         ` David Hildenbrand (Arm)
  0 siblings, 1 reply; 9+ messages in thread
From: Zi Yan @ 2026-07-10 11:30 UTC (permalink / raw)
  To: David Hildenbrand (Arm), Andrew Morton, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Balbir Singh, Kefeng Wang
  Cc: linux-mm, linux-kernel, Sashiko

On Wed Jul 8, 2026 at 10:43 AM EDT, Zi Yan wrote:
> On Wed Jul 8, 2026 at 10:35 AM EDT, Zi Yan wrote:
>> On Wed Jul 8, 2026 at 4:43 AM EDT, David Hildenbrand (Arm) wrote:
>>> On 7/8/26 03:50, Zi Yan wrote:
>>>> migrate_vma_collect_pmd() can drop pte lock to split a large folio and
>>>> restart. But the code does not handle restart properly when pmd becomes
>>>> huge or cleared. It can overflow migrate->dst and migrate->src arrays
>>>> during the hole or skip collection. Fix it by:
>>>> 1. avoiding migrate_vma_collect_huge_pmd() if some collection is done,
>>>> 2. skipping the rest of the range if pmd no longer points to a pte page
>>>>    table.
>>>> 
>>>> Fixes: a30b48bf1b244 ("mm/migrate_device: implement THP migration of zone device pages")
>>>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>>>> Closes: https://sashiko.dev/#/patchset/20260706111958.3649651-1-wangkefeng.wang@huawei.com
>>>> Assisted-by: Claude:claude-opus-4-8
>>>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>>>> ---
>>>> The issue is spot by Sashiko during a patch[1] review as a pre-existing one.
>>>> This patch has the minimal change. An alternative is to reset
>>>> migrate->cpages and migrate->npages and restart the whole range from the
>>>> beginning, but that also requires a restoration of no-longer-present PTEs.
>>>> 
>>>> Link: https://lore.kernel.org/all/20260706111958.3649651-1-wangkefeng.wang@huawei.com/ [1]
>>>> ---
>>>>  mm/migrate_device.c | 19 +++++++++++++++++--
>>>>  1 file changed, 17 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>>>> index 2fffeb1f99694..6ceb47ec1da24 100644
>>>> --- a/mm/migrate_device.c
>>>> +++ b/mm/migrate_device.c
>>>> @@ -257,7 +257,12 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
>>>>  	pte_t *ptep;
>>>>  
>>>>  again:
>>>> -	if (pmd_trans_huge(*pmdp) || !pmd_present(*pmdp)) {
>>>> +	/*
>>>> +	 * Only check pmd when addr is at start, namely no pte is collected.
>>>> +	 * It avoids collecting the same address range [start, addr) twice
>>>> +	 * and overflowing the collection arrays.
>>>> +	 */
>>>> +	if (addr == start && (pmd_trans_huge(*pmdp) || !pmd_present(*pmdp))) {
>>>>  		int ret = migrate_vma_collect_huge_pmd(pmdp, start, end, walk, fault_folio);
>>>>  
>>>>  		if (ret == -EAGAIN)
>>>> @@ -267,8 +272,18 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
>>>>  	}
>>>>  
>>>>  	ptep = pte_offset_map_lock(mm, pmdp, start, &ptl);
>>>> -	if (!ptep)
>>>> +	if (!ptep) {
>>>> +		/*
>>>> +		 * Skip the rest if pmd becomes huge or cleared. Flush if any
>>>> +		 * pte is modified
>>>> +		 */
>>>> +		if (addr != start) {
>>>> +			if (unmapped)
>>>> +				flush_tlb_range(walk->vma, start, end);
>>>> +			return migrate_vma_collect_skip(addr, end, walk);
>>>> +		}
>>>>  		goto again;
>>>> +	}
>>>>  	lazy_mmu_mode_enable();
>>>>  	ptep += (addr - start) / PAGE_SIZE;
>>>
>>> It's hard to express which feelings reading migrate_vma_collect_pmd() gives me,
>>> haha :)
>>
>> I guess I have the same feelings.
>>>
>>>
>>> In case we split ... couldn't we just undo what we already did, before doing the
>>> "goto again" ?
>>
>> You mean we reset migrate->cpages and migrate->npages and restart from
>> the beginning? But it is not only that, since the code below also
>> changes PTEs into migration entries. We will need to revert them as
>> well.
>>
>> Hmm, migrate_vma_collect() documents itself as "update the src array and
>> "takes a reference on the page" without mentioning changing PTEs. I
>> wonder why changing PTEs is necessary, since later migrate_vma_unmap()
>> also changes page table entries to migration entries, although
>> migrate_vma_unmap() changes all entries to a folio, whereas
>> migrate_vma_collect() only changes PTEs from the specified VMA.
>
> OK, it is an optimization[1] when migrate_vma*() was introduced. If
> there is only one mapping, migrate_vma_collect() will set migration
> entry immediately without waiting until migrate_vma_unmap(). Fun.
>
> [1] Commit 8c3328f1f36a5 ("mm/migrate: migrate_vma() unmap page from vma while collecting pages")

Hi David,

Do you think this patch is good to get in? I would like to get ack from
you.

Thanks.

-- 
Best Regards,
Yan, Zi



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm/migrate_device: avoid overflowing migrate_vma collection arrays
  2026-07-10 11:30       ` Zi Yan
@ 2026-07-10 11:56         ` David Hildenbrand (Arm)
  2026-07-10 15:35           ` Zi Yan
  0 siblings, 1 reply; 9+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-10 11:56 UTC (permalink / raw)
  To: Zi Yan, Andrew Morton, Matthew Brost, Joshua Hahn, Rakie Kim,
	Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
	Balbir Singh, Kefeng Wang
  Cc: linux-mm, linux-kernel, Sashiko

On 7/10/26 13:30, Zi Yan wrote:
> On Wed Jul 8, 2026 at 10:43 AM EDT, Zi Yan wrote:
>> On Wed Jul 8, 2026 at 10:35 AM EDT, Zi Yan wrote:
>>>
>>> I guess I have the same feelings.
>>>
>>> You mean we reset migrate->cpages and migrate->npages and restart from
>>> the beginning? But it is not only that, since the code below also
>>> changes PTEs into migration entries. We will need to revert them as
>>> well.
>>>
>>> Hmm, migrate_vma_collect() documents itself as "update the src array and
>>> "takes a reference on the page" without mentioning changing PTEs. I
>>> wonder why changing PTEs is necessary, since later migrate_vma_unmap()
>>> also changes page table entries to migration entries, although
>>> migrate_vma_unmap() changes all entries to a folio, whereas
>>> migrate_vma_collect() only changes PTEs from the specified VMA.
>>
>> OK, it is an optimization[1] when migrate_vma*() was introduced. If
>> there is only one mapping, migrate_vma_collect() will set migration
>> entry immediately without waiting until migrate_vma_unmap(). Fun.
>>
>> [1] Commit 8c3328f1f36a5 ("mm/migrate: migrate_vma() unmap page from vma while collecting pages")
> 
> Hi David,
> 

Hi,

> Do you think this patch is good to get in? I would like to get ack from
> you.

I dislike that we fail collecting even though there is something (different)
mapped now.

In case we stumble over a PMD, there sure are no migration entries on the PTE
level anymore?

-- 
Cheers,

David


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mm/migrate_device: avoid overflowing migrate_vma collection arrays
  2026-07-10 11:56         ` David Hildenbrand (Arm)
@ 2026-07-10 15:35           ` Zi Yan
  0 siblings, 0 replies; 9+ messages in thread
From: Zi Yan @ 2026-07-10 15:35 UTC (permalink / raw)
  To: David Hildenbrand (Arm), Andrew Morton, Matthew Brost,
	Joshua Hahn, Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
	Alistair Popple, Balbir Singh, Kefeng Wang
  Cc: linux-mm, linux-kernel, Sashiko

On Fri Jul 10, 2026 at 7:56 AM EDT, David Hildenbrand (Arm) wrote:
> On 7/10/26 13:30, Zi Yan wrote:
>> On Wed Jul 8, 2026 at 10:43 AM EDT, Zi Yan wrote:
>>> On Wed Jul 8, 2026 at 10:35 AM EDT, Zi Yan wrote:
>>>>
>>>> I guess I have the same feelings.
>>>>
>>>> You mean we reset migrate->cpages and migrate->npages and restart from
>>>> the beginning? But it is not only that, since the code below also
>>>> changes PTEs into migration entries. We will need to revert them as
>>>> well.
>>>>
>>>> Hmm, migrate_vma_collect() documents itself as "update the src array and
>>>> "takes a reference on the page" without mentioning changing PTEs. I
>>>> wonder why changing PTEs is necessary, since later migrate_vma_unmap()
>>>> also changes page table entries to migration entries, although
>>>> migrate_vma_unmap() changes all entries to a folio, whereas
>>>> migrate_vma_collect() only changes PTEs from the specified VMA.
>>>
>>> OK, it is an optimization[1] when migrate_vma*() was introduced. If
>>> there is only one mapping, migrate_vma_collect() will set migration
>>> entry immediately without waiting until migrate_vma_unmap(). Fun.
>>>
>>> [1] Commit 8c3328f1f36a5 ("mm/migrate: migrate_vma() unmap page from vma while collecting pages")
>> 
>> Hi David,
>> 
>
> Hi,
>
>> Do you think this patch is good to get in? I would like to get ack from
>> you.
>
> I dislike that we fail collecting even though there is something (different)
> mapped now.

Not all the time. After a folio split and PTE lock is dropped, there are
different cases:

1. no page table change, the collection grabs the PTE lock and continues;

2. some PTE(s) is changed, the same as 1 and the collection continues;

3. the PMD pointing to the PTE page table is changed to either a leaf
   PMD or an invalid PMD, the collection cannot grab the PTE lock to
   work on the remaming PTEs, since they are gone. For the collected
   PFNs (they are mapped more than once) and folios with elevated
   refcount (they are mapped once and unmapped here as an optimization),
   they will be processed later in migrate_vma_unmap(). Since
   migrate_vma_collect() is best effort, there is no need to revert and
   try to recollect from the beginning (to get that possible large
   folio or skip).

BTW, recollection will be more feasible if migrate_vma_collect() does
not do unmap singly-mapped optimization, since in the case, no PTE is
changed, we just need to reset migrate->->cpages and migrate->npages and
restart from the beginning.

>
> In case we stumble over a PMD, there sure are no migration entries on the PTE
> level anymore?

Right. The PTE page table should be gone and the original PMD, pointing
to the PTE page table, becomes a leaf PMD or an invalid PMD, since
someone changed it when the PTE lock is dropped.

-- 
Best Regards,
Yan, Zi



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-07-10 15:35 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-08  1:50 [PATCH] mm/migrate_device: avoid overflowing migrate_vma collection arrays Zi Yan
2026-07-08  2:45 ` Balbir Singh
2026-07-08  8:43 ` David Hildenbrand (Arm)
2026-07-08 14:35   ` Zi Yan
2026-07-08 14:43     ` Zi Yan
2026-07-10 11:30       ` Zi Yan
2026-07-10 11:56         ` David Hildenbrand (Arm)
2026-07-10 15:35           ` Zi Yan
2026-07-10  2:54 ` Zi Yan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox