* [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios
@ 2026-08-17 12:08 Hui Su
2026-08-17 18:21 ` Andrew Morton
2026-08-27 15:14 ` David Hildenbrand (Arm)
0 siblings, 2 replies; 9+ messages in thread
From: Hui Su @ 2026-08-17 12:08 UTC (permalink / raw)
To: akpm, david, balbirs
Cc: ziy, matthew.brost, joshua.hahnjy, rakie.kim, byungchul, gourry,
ying.huang, apopple, linux-mm, linux-kernel, stable, Hui Su
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios 2026-08-17 12:08 [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios Hui Su @ 2026-08-17 18:21 ` Andrew Morton 2026-08-18 11:13 ` Balbir Singh 2026-08-27 15:14 ` David Hildenbrand (Arm) 1 sibling, 1 reply; 9+ messages in thread From: Andrew Morton @ 2026-08-17 18:21 UTC (permalink / raw) To: Hui Su Cc: david, balbirs, ziy, matthew.brost, joshua.hahnjy, rakie.kim, byungchul, gourry, ying.huang, apopple, linux-mm, linux-kernel, stable 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios 2026-08-17 18:21 ` Andrew Morton @ 2026-08-18 11:13 ` Balbir Singh 0 siblings, 0 replies; 9+ messages in thread From: Balbir Singh @ 2026-08-18 11:13 UTC (permalink / raw) To: Andrew Morton, Hui Su Cc: david, ziy, matthew.brost, joshua.hahnjy, rakie.kim, byungchul, gourry, ying.huang, apopple, linux-mm, linux-kernel, stable 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios 2026-08-17 12:08 [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios Hui Su 2026-08-17 18:21 ` Andrew Morton @ 2026-08-27 15:14 ` David Hildenbrand (Arm) 2026-08-28 4:44 ` Matthew Brost 1 sibling, 1 reply; 9+ messages in thread From: David Hildenbrand (Arm) @ 2026-08-27 15:14 UTC (permalink / raw) To: Hui Su, akpm, balbirs Cc: ziy, matthew.brost, joshua.hahnjy, rakie.kim, byungchul, gourry, ying.huang, apopple, linux-mm, linux-kernel, stable 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios 2026-08-27 15:14 ` David Hildenbrand (Arm) @ 2026-08-28 4:44 ` Matthew Brost 2026-09-07 12:23 ` David Hildenbrand (Arm) 0 siblings, 1 reply; 9+ messages in thread From: Matthew Brost @ 2026-08-28 4:44 UTC (permalink / raw) To: David Hildenbrand (Arm) Cc: Hui Su, akpm, balbirs, ziy, joshua.hahnjy, rakie.kim, byungchul, gourry, ying.huang, apopple, linux-mm, linux-kernel, stable 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios 2026-08-28 4:44 ` Matthew Brost @ 2026-09-07 12:23 ` David Hildenbrand (Arm) 2026-09-09 11:20 ` Hui Su 0 siblings, 1 reply; 9+ messages in thread From: David Hildenbrand (Arm) @ 2026-09-07 12:23 UTC (permalink / raw) To: Matthew Brost Cc: Hui Su, akpm, balbirs, ziy, joshua.hahnjy, rakie.kim, byungchul, gourry, ying.huang, apopple, linux-mm, linux-kernel, stable 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios 2026-09-07 12:23 ` David Hildenbrand (Arm) @ 2026-09-09 11:20 ` Hui Su 2026-09-09 13:43 ` David Hildenbrand (Arm) 0 siblings, 1 reply; 9+ messages in thread From: Hui Su @ 2026-09-09 11:20 UTC (permalink / raw) To: David Hildenbrand (Arm) Cc: Matthew Brost, Andrew Morton, Balbir Singh, Zhi Wang, Joshua Hahn, Rakie Kim, Byungchul Park, Gourry, Ying Huang, Alex Popple, linux-mm, linux-kernel, stable 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios 2026-09-09 11:20 ` Hui Su @ 2026-09-09 13:43 ` David Hildenbrand (Arm) 2026-09-11 5:40 ` Hui Su 0 siblings, 1 reply; 9+ messages in thread From: David Hildenbrand (Arm) @ 2026-09-09 13:43 UTC (permalink / raw) To: Hui Su Cc: Matthew Brost, Andrew Morton, Balbir Singh, Zhi Wang, Joshua Hahn, Rakie Kim, Byungchul Park, Gourry, Ying Huang, Alex Popple, linux-mm, linux-kernel, stable 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios 2026-09-09 13:43 ` David Hildenbrand (Arm) @ 2026-09-11 5:40 ` Hui Su 0 siblings, 0 replies; 9+ messages in thread From: Hui Su @ 2026-09-11 5:40 UTC (permalink / raw) To: David Hildenbrand (Arm) Cc: Matthew Brost, Andrew Morton, Balbir Singh, Zhi Wang, Joshua Hahn, Rakie Kim, Byungchul Park, Gourry, Ying Huang, Alex Popple, linux-mm, linux-kernel, Hui Su 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-09-11 5:41 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-08-17 12:08 [PATCH v2] mm/migrate_device: avoid out-of-bounds writes for compound folios Hui Su 2026-08-17 18:21 ` Andrew Morton 2026-08-18 11:13 ` Balbir Singh 2026-08-27 15:14 ` David Hildenbrand (Arm) 2026-08-28 4:44 ` Matthew Brost 2026-09-07 12:23 ` David Hildenbrand (Arm) 2026-09-09 11:20 ` Hui Su 2026-09-09 13:43 ` David Hildenbrand (Arm) 2026-09-11 5:40 ` Hui Su
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®