[PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios

Hui Su posted 1 patch 1 month, 1 week ago
mm/migrate_device.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
[PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios
Posted by Hui Su 1 month, 1 week ago
migrate_device_range() and migrate_device_pfns() clear the entries
following a compound folio so that the PFN arrays retain their
page-granular representation.

If a compound folio extends beyond the end of the caller-provided range,
the loops clear all following folio entries without limiting them to the
number of slots remaining in the npages-sized array, causing an
out-of-bounds write.

Do not proceed with a compound folio if its page-granular representation
does not fit entirely in the remaining PFN array. If this happens, drop
any reference and lock acquired for the folio, clear the remaining
entries, and stop collecting.

Observed with a KASAN x86 QEMU kernel using the HMM
migrate_anon_huge_zero selftest. Closing /dev/hmm_dmirror0 after
migrating an anonymous huge page to device memory exercises:

  dmirror_fops_release()
    -> dmirror_device_evict_chunk()
      -> migrate_device_range()

Fixes: a30b48bf1b24 ("mm/migrate_device: implement THP migration of zone device pages")
Cc: stable@vger.kernel.org
Signed-off-by: Hui Su <sh_def@163.com>
---
Changes in v2:
- Do not partially represent a compound folio when it does not fit in
  the remaining PFN array.
- Drop any reference and lock acquired for that folio, clear the
  remaining entries, and stop collecting, as suggested by Balbir Singh.

v1: https://lore.kernel.org/lkml/20260817074350.442493-2-sh_def@163.com/

 mm/migrate_device.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index 908d2d4ec43a..69b8d0660bab 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -1400,6 +1400,15 @@ int migrate_device_range(unsigned long *src_pfns, unsigned long start,
 
 		src_pfns[i] = migrate_device_pfn_lock(pfn);
 		nr = folio_nr_pages(folio);
+		if (nr > npages - i) {
+			if (src_pfns[i] & MIGRATE_PFN_MIGRATE) {
+				folio_unlock(folio);
+				folio_put(folio);
+			}
+			memset(&src_pfns[i], 0,
+			       (npages - i) * sizeof(*src_pfns));
+			break;
+		}
 		if (nr > 1) {
 			src_pfns[i] |= MIGRATE_PFN_COMPOUND;
 			for (j = 1; j < nr; j++)
@@ -1434,6 +1443,15 @@ int migrate_device_pfns(unsigned long *src_pfns, unsigned long npages)
 
 		src_pfns[i] = migrate_device_pfn_lock(src_pfns[i]);
 		nr = folio_nr_pages(folio);
+		if (nr > npages - i) {
+			if (src_pfns[i] & MIGRATE_PFN_MIGRATE) {
+				folio_unlock(folio);
+				folio_put(folio);
+			}
+			memset(&src_pfns[i], 0,
+			       (npages - i) * sizeof(*src_pfns));
+			break;
+		}
 		if (nr > 1) {
 			src_pfns[i] |= MIGRATE_PFN_COMPOUND;
 			for (j = 1; j < nr; j++)
-- 
2.54.0
Re: [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios
Posted by David Hildenbrand (Arm) 1 month ago
On 8/17/26 14:08, Hui Su wrote:
> migrate_device_range() and migrate_device_pfns() clear the entries
> following a compound folio so that the PFN arrays retain their
> page-granular representation.
> 
> If a compound folio extends beyond the end of the caller-provided range,
> the loops clear all following folio entries without limiting them to the
> number of slots remaining in the npages-sized array, causing an
> out-of-bounds write.
> 
> Do not proceed with a compound folio if its page-granular representation
> does not fit entirely in the remaining PFN array. If this happens, drop
> any reference and lock acquired for the folio, clear the remaining
> entries, and stop collecting.
> 
> Observed with a KASAN x86 QEMU kernel using the HMM
> migrate_anon_huge_zero selftest. Closing /dev/hmm_dmirror0 after
> migrating an anonymous huge page to device memory exercises:
> 
>   dmirror_fops_release()
>     -> dmirror_device_evict_chunk()
>       -> migrate_device_range()
> 
> Fixes: a30b48bf1b24 ("mm/migrate_device: implement THP migration of zone device pages")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hui Su <sh_def@163.com>
> ---
> Changes in v2:
> - Do not partially represent a compound folio when it does not fit in
>   the remaining PFN array.
> - Drop any reference and lock acquired for that folio, clear the
>   remaining entries, and stop collecting, as suggested by Balbir Singh.
> 
> v1: https://lore.kernel.org/lkml/20260817074350.442493-2-sh_def@163.com/
> 
>  mm/migrate_device.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index 908d2d4ec43a..69b8d0660bab 100644
> --- a/mm/migrate_device.c
> +++ b/mm/migrate_device.c
> @@ -1400,6 +1400,15 @@ int migrate_device_range(unsigned long *src_pfns, unsigned long start,
>  
>  		src_pfns[i] = migrate_device_pfn_lock(pfn);
>  		nr = folio_nr_pages(folio);
> +		if (nr > npages - i) {
> +			if (src_pfns[i] & MIGRATE_PFN_MIGRATE) {
> +				folio_unlock(folio);
> +				folio_put(folio);
> +			}
> +			memset(&src_pfns[i], 0,
> +			       (npages - i) * sizeof(*src_pfns));
> +			break;
> +		}
>  		if (nr > 1) {
>  			src_pfns[i] |= MIGRATE_PFN_COMPOUND;
>  			for (j = 1; j < nr; j++)
> @@ -1434,6 +1443,15 @@ int migrate_device_pfns(unsigned long *src_pfns, unsigned long npages)
>  
>  		src_pfns[i] = migrate_device_pfn_lock(src_pfns[i]);
>  		nr = folio_nr_pages(folio);
> +		if (nr > npages - i) {
> +			if (src_pfns[i] & MIGRATE_PFN_MIGRATE) {
> +				folio_unlock(folio);
> +				folio_put(folio);
> +			}
> +			memset(&src_pfns[i], 0,
> +			       (npages - i) * sizeof(*src_pfns));
> +			break;
> +		}
>  		if (nr > 1) {
>  			src_pfns[i] |= MIGRATE_PFN_COMPOUND;
>  			for (j = 1; j < nr; j++)

The code duplication here makes me angry. :) And using a memset on one branch but
not on the other is weird.

But is this the right fix or rather what

	https://lore.kernel.org/r/20260805231041.3791771-3-matthew.brost@intel.com 

tried? That fix would also need a cleanup but seems code-wise simpler. A cleanup could look like:

diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index 762c5cee8fecc..480bf59bc2425 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -1419,10 +1419,9 @@ int migrate_device_range(unsigned long *src_pfns, unsigned long start,
        for (pfn = start, i = 0; i < npages; pfn++, i++) {
                struct page *page = pfn_to_page(pfn);
                struct folio *folio = page_folio(page);
-               unsigned int nr = 1;
+               unsigned int nr = min(folio_nr_pages(folio), npages - i);
 
                src_pfns[i] = migrate_device_pfn_lock(pfn);
-               nr = folio_nr_pages(folio);
                if (nr > 1) {
                        src_pfns[i] |= MIGRATE_PFN_COMPOUND;
                        for (j = 1; j < nr; j++)
@@ -1453,10 +1452,9 @@ int migrate_device_pfns(unsigned long *src_pfns, unsigned long npages)
        for (i = 0; i < npages; i++) {
                struct page *page = pfn_to_page(src_pfns[i]);
                struct folio *folio = page_folio(page);
-               unsigned int nr = 1;
+               unsigned int nr = min(folio_nr_pages(folio), npages - i);
 
                src_pfns[i] = migrate_device_pfn_lock(src_pfns[i]);
-               nr = folio_nr_pages(folio);
                if (nr > 1) {
                        src_pfns[i] |= MIGRATE_PFN_COMPOUND;
                        for (j = 1; j < nr; j++)


And as a further cleanup, we'd better de-duplicate that code and possibly use a
memset for clearing, getting rid of j entirely.



-- 
Cheers,

David
Re: [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios
Posted by Matthew Brost 1 month ago
On Thu, Aug 27, 2026 at 05:14:40PM +0200, David Hildenbrand (Arm) wrote:
> On 8/17/26 14:08, Hui Su wrote:
> > migrate_device_range() and migrate_device_pfns() clear the entries
> > following a compound folio so that the PFN arrays retain their
> > page-granular representation.
> > 
> > If a compound folio extends beyond the end of the caller-provided range,
> > the loops clear all following folio entries without limiting them to the
> > number of slots remaining in the npages-sized array, causing an
> > out-of-bounds write.
> > 
> > Do not proceed with a compound folio if its page-granular representation
> > does not fit entirely in the remaining PFN array. If this happens, drop
> > any reference and lock acquired for the folio, clear the remaining
> > entries, and stop collecting.
> > 
> > Observed with a KASAN x86 QEMU kernel using the HMM
> > migrate_anon_huge_zero selftest. Closing /dev/hmm_dmirror0 after
> > migrating an anonymous huge page to device memory exercises:
> > 
> >   dmirror_fops_release()
> >     -> dmirror_device_evict_chunk()
> >       -> migrate_device_range()
> > 
> > Fixes: a30b48bf1b24 ("mm/migrate_device: implement THP migration of zone device pages")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Hui Su <sh_def@163.com>
> > ---
> > Changes in v2:
> > - Do not partially represent a compound folio when it does not fit in
> >   the remaining PFN array.
> > - Drop any reference and lock acquired for that folio, clear the
> >   remaining entries, and stop collecting, as suggested by Balbir Singh.
> > 
> > v1: https://lore.kernel.org/lkml/20260817074350.442493-2-sh_def@163.com/
> > 
> >  mm/migrate_device.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> > index 908d2d4ec43a..69b8d0660bab 100644
> > --- a/mm/migrate_device.c
> > +++ b/mm/migrate_device.c
> > @@ -1400,6 +1400,15 @@ int migrate_device_range(unsigned long *src_pfns, unsigned long start,
> >  
> >  		src_pfns[i] = migrate_device_pfn_lock(pfn);
> >  		nr = folio_nr_pages(folio);
> > +		if (nr > npages - i) {
> > +			if (src_pfns[i] & MIGRATE_PFN_MIGRATE) {
> > +				folio_unlock(folio);
> > +				folio_put(folio);
> > +			}
> > +			memset(&src_pfns[i], 0,
> > +			       (npages - i) * sizeof(*src_pfns));
> > +			break;
> > +		}
> >  		if (nr > 1) {
> >  			src_pfns[i] |= MIGRATE_PFN_COMPOUND;
> >  			for (j = 1; j < nr; j++)
> > @@ -1434,6 +1443,15 @@ int migrate_device_pfns(unsigned long *src_pfns, unsigned long npages)
> >  
> >  		src_pfns[i] = migrate_device_pfn_lock(src_pfns[i]);
> >  		nr = folio_nr_pages(folio);
> > +		if (nr > npages - i) {
> > +			if (src_pfns[i] & MIGRATE_PFN_MIGRATE) {
> > +				folio_unlock(folio);
> > +				folio_put(folio);
> > +			}
> > +			memset(&src_pfns[i], 0,
> > +			       (npages - i) * sizeof(*src_pfns));
> > +			break;
> > +		}
> >  		if (nr > 1) {
> >  			src_pfns[i] |= MIGRATE_PFN_COMPOUND;
> >  			for (j = 1; j < nr; j++)
> 
> The code duplication here makes me angry. :) And using a memset on one branch but
> not on the other is weird.
> 
> But is this the right fix or rather what
> 
> 	https://lore.kernel.org/r/20260805231041.3791771-3-matthew.brost@intel.com 
> 

I'm not sure my patch is right now that I think about, or if this patch
is right, or your suggestion either :).

> tried? That fix would also need a cleanup but seems code-wise simpler. A cleanup could look like:
> 
> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index 762c5cee8fecc..480bf59bc2425 100644
> --- a/mm/migrate_device.c
> +++ b/mm/migrate_device.c
> @@ -1419,10 +1419,9 @@ int migrate_device_range(unsigned long *src_pfns, unsigned long start,
>         for (pfn = start, i = 0; i < npages; pfn++, i++) {
>                 struct page *page = pfn_to_page(pfn);
>                 struct folio *folio = page_folio(page);
> -               unsigned int nr = 1;
> +               unsigned int nr = min(folio_nr_pages(folio), npages - i);
>  
>                 src_pfns[i] = migrate_device_pfn_lock(pfn);
> -               nr = folio_nr_pages(folio);
>                 if (nr > 1) {
>                         src_pfns[i] |= MIGRATE_PFN_COMPOUND;
>                         for (j = 1; j < nr; j++)
> @@ -1453,10 +1452,9 @@ int migrate_device_pfns(unsigned long *src_pfns, unsigned long npages)
>         for (i = 0; i < npages; i++) {
>                 struct page *page = pfn_to_page(src_pfns[i]);
>                 struct folio *folio = page_folio(page);
> -               unsigned int nr = 1;
> +               unsigned int nr = min(folio_nr_pages(folio), npages - i);

I think the semantics of both these function should be that if a user
passes in a PFN array and one of the PFNs exceeds the `npages` limit,
that is a misuse of this function and we should unwind and return an
error. The caller is providing the PFNs and should therefore know the
backing page alignment, so `npages` should always end exactly on a page
boundary. The sole caller of migrate_device_pfns function is
`drm_gpusvm/pagemap`, which certainly adheres to this requirement.

The two callers of migrate_device_range, also seemly adheres to this
requirement.

Matt

>  
>                 src_pfns[i] = migrate_device_pfn_lock(src_pfns[i]);
> -               nr = folio_nr_pages(folio);
>                 if (nr > 1) {
>                         src_pfns[i] |= MIGRATE_PFN_COMPOUND;
>                         for (j = 1; j < nr; j++)
> 
> 
> And as a further cleanup, we'd better de-duplicate that code and possibly use a
> memset for clearing, getting rid of j entirely.
> 
> 
> 
> -- 
> Cheers,
> 
> David
Re: [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios
Posted by David Hildenbrand (Arm) 3 weeks ago
On 8/28/26 06:44, Matthew Brost wrote:
> On Thu, Aug 27, 2026 at 05:14:40PM +0200, David Hildenbrand (Arm) wrote:
>> On 8/17/26 14:08, Hui Su wrote:
>>> migrate_device_range() and migrate_device_pfns() clear the entries
>>> following a compound folio so that the PFN arrays retain their
>>> page-granular representation.
>>>
>>> If a compound folio extends beyond the end of the caller-provided range,
>>> the loops clear all following folio entries without limiting them to the
>>> number of slots remaining in the npages-sized array, causing an
>>> out-of-bounds write.
>>>
>>> Do not proceed with a compound folio if its page-granular representation
>>> does not fit entirely in the remaining PFN array. If this happens, drop
>>> any reference and lock acquired for the folio, clear the remaining
>>> entries, and stop collecting.
>>>
>>> Observed with a KASAN x86 QEMU kernel using the HMM
>>> migrate_anon_huge_zero selftest. Closing /dev/hmm_dmirror0 after
>>> migrating an anonymous huge page to device memory exercises:
>>>
>>>   dmirror_fops_release()
>>>     -> dmirror_device_evict_chunk()
>>>       -> migrate_device_range()
>>>
>>> Fixes: a30b48bf1b24 ("mm/migrate_device: implement THP migration of zone device pages")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Hui Su <sh_def@163.com>
>>> ---
>>> Changes in v2:
>>> - Do not partially represent a compound folio when it does not fit in
>>>   the remaining PFN array.
>>> - Drop any reference and lock acquired for that folio, clear the
>>>   remaining entries, and stop collecting, as suggested by Balbir Singh.
>>>
>>> v1: https://lore.kernel.org/lkml/20260817074350.442493-2-sh_def@163.com/
>>>
>>>  mm/migrate_device.c | 18 ++++++++++++++++++
>>>  1 file changed, 18 insertions(+)
>>>
>>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>>> index 908d2d4ec43a..69b8d0660bab 100644
>>> --- a/mm/migrate_device.c
>>> +++ b/mm/migrate_device.c
>>> @@ -1400,6 +1400,15 @@ int migrate_device_range(unsigned long *src_pfns, unsigned long start,
>>>  
>>>  		src_pfns[i] = migrate_device_pfn_lock(pfn);
>>>  		nr = folio_nr_pages(folio);
>>> +		if (nr > npages - i) {
>>> +			if (src_pfns[i] & MIGRATE_PFN_MIGRATE) {
>>> +				folio_unlock(folio);
>>> +				folio_put(folio);
>>> +			}
>>> +			memset(&src_pfns[i], 0,
>>> +			       (npages - i) * sizeof(*src_pfns));
>>> +			break;
>>> +		}
>>>  		if (nr > 1) {
>>>  			src_pfns[i] |= MIGRATE_PFN_COMPOUND;
>>>  			for (j = 1; j < nr; j++)
>>> @@ -1434,6 +1443,15 @@ int migrate_device_pfns(unsigned long *src_pfns, unsigned long npages)
>>>  
>>>  		src_pfns[i] = migrate_device_pfn_lock(src_pfns[i]);
>>>  		nr = folio_nr_pages(folio);
>>> +		if (nr > npages - i) {
>>> +			if (src_pfns[i] & MIGRATE_PFN_MIGRATE) {
>>> +				folio_unlock(folio);
>>> +				folio_put(folio);
>>> +			}
>>> +			memset(&src_pfns[i], 0,
>>> +			       (npages - i) * sizeof(*src_pfns));
>>> +			break;
>>> +		}
>>>  		if (nr > 1) {
>>>  			src_pfns[i] |= MIGRATE_PFN_COMPOUND;
>>>  			for (j = 1; j < nr; j++)
>>
>> The code duplication here makes me angry. :) And using a memset on one branch but
>> not on the other is weird.
>>
>> But is this the right fix or rather what
>>
>> 	https://lore.kernel.org/r/20260805231041.3791771-3-matthew.brost@intel.com 
>>
> 
> I'm not sure my patch is right now that I think about, or if this patch
> is right, or your suggestion either :).
> 
>> tried? That fix would also need a cleanup but seems code-wise simpler. A cleanup could look like:
>>
>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>> index 762c5cee8fecc..480bf59bc2425 100644
>> --- a/mm/migrate_device.c
>> +++ b/mm/migrate_device.c
>> @@ -1419,10 +1419,9 @@ int migrate_device_range(unsigned long *src_pfns, unsigned long start,
>>         for (pfn = start, i = 0; i < npages; pfn++, i++) {
>>                 struct page *page = pfn_to_page(pfn);
>>                 struct folio *folio = page_folio(page);
>> -               unsigned int nr = 1;
>> +               unsigned int nr = min(folio_nr_pages(folio), npages - i);
>>  
>>                 src_pfns[i] = migrate_device_pfn_lock(pfn);
>> -               nr = folio_nr_pages(folio);
>>                 if (nr > 1) {
>>                         src_pfns[i] |= MIGRATE_PFN_COMPOUND;
>>                         for (j = 1; j < nr; j++)
>> @@ -1453,10 +1452,9 @@ int migrate_device_pfns(unsigned long *src_pfns, unsigned long npages)
>>         for (i = 0; i < npages; i++) {
>>                 struct page *page = pfn_to_page(src_pfns[i]);
>>                 struct folio *folio = page_folio(page);
>> -               unsigned int nr = 1;
>> +               unsigned int nr = min(folio_nr_pages(folio), npages - i);
> 
> I think the semantics of both these function should be that if a user
> passes in a PFN array and one of the PFNs exceeds the `npages` limit,
> that is a misuse of this function and we should unwind and return an
> error. The caller is providing the PFNs and should therefore know the
> backing page alignment, so `npages` should always end exactly on a page
> boundary. The sole caller of migrate_device_pfns function is
> `drm_gpusvm/pagemap`, which certainly adheres to this requirement.
> 
> The two callers of migrate_device_range, also seemly adheres to this
> requirement.

That's what I was thinking: because how should we partially migrate folios?
Doesn't make sense :)

If this cannot be triggered today, neither Fixes: nor CC: stable is appropriate.

Agreed that we should just warn.

-- 
Cheers,

David
Re: [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios
Posted by Hui Su 2 weeks, 5 days ago
On Mon, Sep 07, 2026 at 02:23:33PM +0200, David Hildenbrand (Arm) wrote:
> That's what I was thinking: because how should we partially migrate folios?
> Doesn't make sense :)
>
> If this cannot be triggered today, neither Fixes: nor CC: stable is appropriate.
>
> Agreed that we should just warn.

Hi David, Matthew,

Sorry, I missed this earlier.

This fix has already landed in mainline, but I agree that the API semantics
should be clarified. Partially migrating a compound folio does not make
sense. If the intended contract is that callers must not provide a range
that cuts through a compound folio, then treating such input as caller
misuse with an explicit WARN/error path sounds reasonable.

The reason I added the Fixes tag and Cc'd stable was that I observed an
actual KASAN out-of-bounds write with the HMM migrate_anon_huge_zero
selftest. The path was:

  dmirror_fops_release()
    -> dmirror_device_evict_chunk()
      -> migrate_device_range()

My intent was to prevent migrate_device_range() from writing past the
caller-provided src_pfns array when such an input is observed. That said,
silently stopping the collection is not necessarily the best API semantics
if this should be treated as caller misuse.

I will rerun the reproducer on current mainline and report back with the
result. If the preferred direction is to make this an explicit WARN/error
path instead, I can send a follow-up patch on top of mainline.

Thanks,
Hui
Re: [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios
Posted by David Hildenbrand (Arm) 2 weeks, 5 days ago
On 9/9/26 13:20, Hui Su wrote:
> On Mon, Sep 07, 2026 at 02:23:33PM +0200, David Hildenbrand (Arm) wrote:
>> That's what I was thinking: because how should we partially migrate folios?
>> Doesn't make sense :)
>>
>> If this cannot be triggered today, neither Fixes: nor CC: stable is appropriate.
>>
>> Agreed that we should just warn.
> 
> Hi David, Matthew,
> 
> Sorry, I missed this earlier.
> 
> This fix has already landed in mainline, but I agree that the API semantics
> should be clarified. Partially migrating a compound folio does not make
> sense. If the intended contract is that callers must not provide a range
> that cuts through a compound folio, then treating such input as caller
> misuse with an explicit WARN/error path sounds reasonable.
> 
> The reason I added the Fixes tag and Cc'd stable was that I observed an
> actual KASAN out-of-bounds write with the HMM migrate_anon_huge_zero
> selftest. The path was:
> 
>   dmirror_fops_release()
>     -> dmirror_device_evict_chunk()
>       -> migrate_device_range()
> 
> My intent was to prevent migrate_device_range() from writing past the
> caller-provided src_pfns array when such an input is observed. That said,
> silently stopping the collection is not necessarily the best API semantics
> if this should be treated as caller misuse.
> 
> I will rerun the reproducer on current mainline and report back with the
> result. If the preferred direction is to make this an explicit WARN/error
> path instead, I can send a follow-up patch on top of mainline.

Yes, and please clean up the code in any case; the duplication should be avoided.

-- 
Cheers,

David
Re: [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios
Posted by Hui Su 2 weeks, 3 days ago
On Wed, Sep 09, 2026 at 03:43:03PM +0200, David Hildenbrand (Arm) wrote:
> Yes, and please clean up the code in any case; the duplication should be avoided.

Hi David, Matthew,

I reran the original HMM migrate_anon_huge_zero reproducer on current
mainline.

The private-device case passes, and I could not reproduce the original
KASAN out-of-bounds write. I also instrumented the truncated
compound-folio case, and that condition was not hit. The coherent-device
case was skipped on this setup.

So for the follow-up cleanup, I don't plan to add a Fixes tag or Cc
stable. It only consolidates the duplicated compound-folio handling,
uses memset() for the tail entries, and adds WARN_ON_ONCE() for the
caller invariant discussed here.

The original stable Cc was based on the KASAN OOB I observed on the tree
I tested at the time.

I'll send the cleanup as a separate patch.

Thanks,
Hui
Re: [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios
Posted by Andrew Morton 1 month, 1 week ago
On Mon, 17 Aug 2026 20:08:00 +0800 Hui Su <sh_def@163.com> wrote:

> migrate_device_range() and migrate_device_pfns() clear the entries
> following a compound folio so that the PFN arrays retain their
> page-granular representation.
> 
> If a compound folio extends beyond the end of the caller-provided range,
> the loops clear all following folio entries without limiting them to the
> number of slots remaining in the npages-sized array, causing an
> out-of-bounds write.
> 
> Do not proceed with a compound folio if its page-granular representation
> does not fit entirely in the remaining PFN array. If this happens, drop
> any reference and lock acquired for the folio, clear the remaining
> entries, and stop collecting.
> 
> Observed with a KASAN x86 QEMU kernel using the HMM
> migrate_anon_huge_zero selftest. Closing /dev/hmm_dmirror0 after
> migrating an anonymous huge page to device memory exercises:
> 
>   dmirror_fops_release()
>     -> dmirror_device_evict_chunk()
>       -> migrate_device_range()

It isn't clear (to me, at least) what "exercises:" means.  Was there a
WARN?  Did the kernel crash?  Did it erase all my cat videos?

IOW, in detail, what are the userspace-visible effects of the bug? 
Please add this info to the changelog and maintain it.

Sashiko might have found some things.  Some pre-existing, some newly added:
	https://sashiko.dev/#/patchset/20260817120758.669807-3-sh_def@163.com
Re: [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios
Posted by Balbir Singh 1 month, 1 week ago
On 8/18/26 4:21 AM, Andrew Morton wrote:
> On Mon, 17 Aug 2026 20:08:00 +0800 Hui Su <sh_def@163.com> wrote:
> 
>> migrate_device_range() and migrate_device_pfns() clear the entries
>> following a compound folio so that the PFN arrays retain their
>> page-granular representation.
>>
>> If a compound folio extends beyond the end of the caller-provided range,
>> the loops clear all following folio entries without limiting them to the
>> number of slots remaining in the npages-sized array, causing an
>> out-of-bounds write.
>>
>> Do not proceed with a compound folio if its page-granular representation
>> does not fit entirely in the remaining PFN array. If this happens, drop
>> any reference and lock acquired for the folio, clear the remaining
>> entries, and stop collecting.
>>
>> Observed with a KASAN x86 QEMU kernel using the HMM
>> migrate_anon_huge_zero selftest. Closing /dev/hmm_dmirror0 after
>> migrating an anonymous huge page to device memory exercises:
>>
>>   dmirror_fops_release()
>>     -> dmirror_device_evict_chunk()
>>       -> migrate_device_range()
> 
> It isn't clear (to me, at least) what "exercises:" means.  Was there a
> WARN?  Did the kernel crash?  Did it erase all my cat videos?
> 
> IOW, in detail, what are the userspace-visible effects of the bug? 
> Please add this info to the changelog and maintain it.
> 
> Sashiko might have found some things.  Some pre-existing, some newly added:
> 	https://sashiko.dev/#/patchset/20260817120758.669807-3-sh_def@163.com
> 
> 

userspace-visible effects are a function of the usage of the API by the device
driver. If the device driver passes in npages that don't account for PMD pages,
it would be an issue. In the case above, closing the device will evict pages
from the device back to the CPU, but I am surprised that the size of npages was
not properly split, we should fix that as well.

FYI: I've tried reading through Sashiko's report, but there is too much
pre-existing content that it points to and it can be quite time consuming. I am
working on adding new test cases to try and catch some of the issues found

Balbir