* [PATCH] drm/gpusvm, pagemap: Do not assume DRM pagemap owns device pages
@ 2026-04-09 1:55 Matthew Brost
2026-04-10 12:46 ` [PATCH] drm/gpusvm,pagemap: " Francois Dugast
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Matthew Brost @ 2026-04-09 1:55 UTC (permalink / raw)
To: intel-xe, dri-devel; +Cc: francois.dugast, himal.prasad.ghimiray
Update drm_pagemap_page_zone_device_data() to derive the pgmap ops from
the page and compare them against the DRM pagemap ops. If the ops do not
match, return NULL.
Also harden two risky call sites by checking for NULL after
hmm_range_fault() or migrate_vma_setup() when migrating to device
memory, as it is possible to encounter device pages that are not owned
by DRM pagemap.
Suggested-by: sashiko.dev
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/drm_gpusvm.c | 5 +++++
drivers/gpu/drm/drm_pagemap.c | 14 ++++++++++----
include/drm/drm_pagemap.h | 5 ++++-
3 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index 365a9c0b522a..b3cccd047a21 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -1506,6 +1506,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
struct drm_pagemap_zdd *__zdd =
drm_pagemap_page_zone_device_data(page);
+ if (!__zdd) {
+ err = -EINVAL;
+ goto err_unmap;
+ }
+
if (!ctx->allow_mixed &&
zdd != __zdd && i > 0) {
err = -EOPNOTSUPP;
diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
index d82ea7ccb8da..95c951c5b569 100644
--- a/drivers/gpu/drm/drm_pagemap.c
+++ b/drivers/gpu/drm/drm_pagemap.c
@@ -753,10 +753,16 @@ int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,
own_pages++;
goto next;
}
- cur.dpagemap = src_zdd->dpagemap;
- cur.ops = src_zdd->devmem_allocation->ops;
- cur.device = cur.dpagemap->drm->dev;
- pages[i] = src_page;
+ if (src_zdd) {
+ cur.dpagemap = src_zdd->dpagemap;
+ cur.ops = src_zdd->devmem_allocation->ops;
+ cur.device = cur.dpagemap->drm->dev;
+ pages[i] = src_page;
+ } else {
+ npages = i;
+ err = -EINVAL;
+ goto err_finalize;
+ }
}
if (!pages[i]) {
cur.dpagemap = NULL;
diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
index 95eb4b66b057..9b7c50932db5 100644
--- a/include/drm/drm_pagemap.h
+++ b/include/drm/drm_pagemap.h
@@ -367,12 +367,15 @@ int drm_pagemap_reinit(struct drm_pagemap *dpagemap);
* drm_pagemap_page_zone_device_data() - Page to zone_device_data
* @page: Pointer to the page
*
- * Return: Page's zone_device_data
+ * Return: Page's zone_device_data if owned by DRM pagemap, NULL otherwise
*/
static inline struct drm_pagemap_zdd *drm_pagemap_page_zone_device_data(struct page *page)
{
struct folio *folio = page_folio(page);
+ if (WARN_ON_ONCE(page_pgmap(page)->ops != drm_pagemap_pagemap_ops_get()))
+ return NULL;
+
return folio_zone_device_data(folio);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/gpusvm,pagemap: Do not assume DRM pagemap owns device pages
2026-04-09 1:55 [PATCH] drm/gpusvm, pagemap: Do not assume DRM pagemap owns device pages Matthew Brost
@ 2026-04-10 12:46 ` Francois Dugast
2026-04-10 20:02 ` Matthew Brost
2026-04-12 1:53 ` Claude review: drm/gpusvm, pagemap: " Claude Code Review Bot
2026-04-12 1:53 ` Claude Code Review Bot
2 siblings, 1 reply; 6+ messages in thread
From: Francois Dugast @ 2026-04-10 12:46 UTC (permalink / raw)
To: Matthew Brost; +Cc: intel-xe, dri-devel, himal.prasad.ghimiray
On Wed, Apr 08, 2026 at 06:55:12PM -0700, Matthew Brost wrote:
> Update drm_pagemap_page_zone_device_data() to derive the pgmap ops from
> the page and compare them against the DRM pagemap ops. If the ops do not
> match, return NULL.
>
> Also harden two risky call sites by checking for NULL after
> hmm_range_fault() or migrate_vma_setup() when migrating to device
> memory, as it is possible to encounter device pages that are not owned
> by DRM pagemap.
Shouldn't we also harden other calls to drm_pagemap_page_zone_device_data() in
drm_pagemap_migrate_map_device_private_pages()
drm_pagemap_migrate_unmap_pages()
drm_pagemap_migrate_populate_ram_pfn()
__drm_pagemap_migrate_to_ram()
drm_pagemap_folio_free()
drm_pagemap_migrate_to_ram()
drm_pagemap_page_to_dpagemap()
Francois
>
> Suggested-by: sashiko.dev
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
> drivers/gpu/drm/drm_gpusvm.c | 5 +++++
> drivers/gpu/drm/drm_pagemap.c | 14 ++++++++++----
> include/drm/drm_pagemap.h | 5 ++++-
> 3 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index 365a9c0b522a..b3cccd047a21 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1506,6 +1506,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> struct drm_pagemap_zdd *__zdd =
> drm_pagemap_page_zone_device_data(page);
>
> + if (!__zdd) {
> + err = -EINVAL;
> + goto err_unmap;
> + }
> +
> if (!ctx->allow_mixed &&
> zdd != __zdd && i > 0) {
> err = -EOPNOTSUPP;
> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> index d82ea7ccb8da..95c951c5b569 100644
> --- a/drivers/gpu/drm/drm_pagemap.c
> +++ b/drivers/gpu/drm/drm_pagemap.c
> @@ -753,10 +753,16 @@ int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,
> own_pages++;
> goto next;
> }
> - cur.dpagemap = src_zdd->dpagemap;
> - cur.ops = src_zdd->devmem_allocation->ops;
> - cur.device = cur.dpagemap->drm->dev;
> - pages[i] = src_page;
> + if (src_zdd) {
> + cur.dpagemap = src_zdd->dpagemap;
> + cur.ops = src_zdd->devmem_allocation->ops;
> + cur.device = cur.dpagemap->drm->dev;
> + pages[i] = src_page;
> + } else {
> + npages = i;
> + err = -EINVAL;
> + goto err_finalize;
> + }
> }
> if (!pages[i]) {
> cur.dpagemap = NULL;
> diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
> index 95eb4b66b057..9b7c50932db5 100644
> --- a/include/drm/drm_pagemap.h
> +++ b/include/drm/drm_pagemap.h
> @@ -367,12 +367,15 @@ int drm_pagemap_reinit(struct drm_pagemap *dpagemap);
> * drm_pagemap_page_zone_device_data() - Page to zone_device_data
> * @page: Pointer to the page
> *
> - * Return: Page's zone_device_data
> + * Return: Page's zone_device_data if owned by DRM pagemap, NULL otherwise
> */
> static inline struct drm_pagemap_zdd *drm_pagemap_page_zone_device_data(struct page *page)
> {
> struct folio *folio = page_folio(page);
>
> + if (WARN_ON_ONCE(page_pgmap(page)->ops != drm_pagemap_pagemap_ops_get()))
> + return NULL;
> +
> return folio_zone_device_data(folio);
> }
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/gpusvm,pagemap: Do not assume DRM pagemap owns device pages
2026-04-10 12:46 ` [PATCH] drm/gpusvm,pagemap: " Francois Dugast
@ 2026-04-10 20:02 ` Matthew Brost
2026-04-11 9:06 ` Francois Dugast
0 siblings, 1 reply; 6+ messages in thread
From: Matthew Brost @ 2026-04-10 20:02 UTC (permalink / raw)
To: Francois Dugast; +Cc: intel-xe, dri-devel, himal.prasad.ghimiray
On Fri, Apr 10, 2026 at 02:46:54PM +0200, Francois Dugast wrote:
> On Wed, Apr 08, 2026 at 06:55:12PM -0700, Matthew Brost wrote:
> > Update drm_pagemap_page_zone_device_data() to derive the pgmap ops from
> > the page and compare them against the DRM pagemap ops. If the ops do not
> > match, return NULL.
> >
> > Also harden two risky call sites by checking for NULL after
> > hmm_range_fault() or migrate_vma_setup() when migrating to device
> > memory, as it is possible to encounter device pages that are not owned
> > by DRM pagemap.
>
> Shouldn't we also harden other calls to drm_pagemap_page_zone_device_data() in
>
> drm_pagemap_migrate_map_device_private_pages()
> drm_pagemap_migrate_unmap_pages()
We sanitize prior to this in drm_pagemap_migrate_to_devmem or are
operating on pages handed back via populate_devmem_pfn.
> drm_pagemap_migrate_populate_ram_pfn()
Operating on page handed back via populate_devmem_pfn. Also wouldn't
NULL ptr reference.
> __drm_pagemap_migrate_to_ram()
> drm_pagemap_folio_free()
> drm_pagemap_migrate_to_ram()
These are in the vops path of pagemap we check against.
> drm_pagemap_page_to_dpagemap()
We sanitize prior to this in drm_gpusvm_get_pages.
Thus all the above sites I figure a warn is enough as it would indicate
a fairly serious bug in drm gpusvm/pagemap/calling driver which this
code completely controls.
The case where I do sanitize - after collection via hmm_range_fault,
migrate_vma_setup, I think it could be possible an outside driver has
moved pages to private (very unlikely) and this driver also tries to
move, so abort rather NULL ptr dereference.
So basically mitagated the 2 risky places with sanitization. Ofc we
could check this everywhere...
Matt
>
> Francois
>
> >
> > Suggested-by: sashiko.dev
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > ---
> > drivers/gpu/drm/drm_gpusvm.c | 5 +++++
> > drivers/gpu/drm/drm_pagemap.c | 14 ++++++++++----
> > include/drm/drm_pagemap.h | 5 ++++-
> > 3 files changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> > index 365a9c0b522a..b3cccd047a21 100644
> > --- a/drivers/gpu/drm/drm_gpusvm.c
> > +++ b/drivers/gpu/drm/drm_gpusvm.c
> > @@ -1506,6 +1506,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> > struct drm_pagemap_zdd *__zdd =
> > drm_pagemap_page_zone_device_data(page);
> >
> > + if (!__zdd) {
> > + err = -EINVAL;
> > + goto err_unmap;
> > + }
> > +
> > if (!ctx->allow_mixed &&
> > zdd != __zdd && i > 0) {
> > err = -EOPNOTSUPP;
> > diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> > index d82ea7ccb8da..95c951c5b569 100644
> > --- a/drivers/gpu/drm/drm_pagemap.c
> > +++ b/drivers/gpu/drm/drm_pagemap.c
> > @@ -753,10 +753,16 @@ int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,
> > own_pages++;
> > goto next;
> > }
> > - cur.dpagemap = src_zdd->dpagemap;
> > - cur.ops = src_zdd->devmem_allocation->ops;
> > - cur.device = cur.dpagemap->drm->dev;
> > - pages[i] = src_page;
> > + if (src_zdd) {
> > + cur.dpagemap = src_zdd->dpagemap;
> > + cur.ops = src_zdd->devmem_allocation->ops;
> > + cur.device = cur.dpagemap->drm->dev;
> > + pages[i] = src_page;
> > + } else {
> > + npages = i;
> > + err = -EINVAL;
> > + goto err_finalize;
> > + }
> > }
> > if (!pages[i]) {
> > cur.dpagemap = NULL;
> > diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
> > index 95eb4b66b057..9b7c50932db5 100644
> > --- a/include/drm/drm_pagemap.h
> > +++ b/include/drm/drm_pagemap.h
> > @@ -367,12 +367,15 @@ int drm_pagemap_reinit(struct drm_pagemap *dpagemap);
> > * drm_pagemap_page_zone_device_data() - Page to zone_device_data
> > * @page: Pointer to the page
> > *
> > - * Return: Page's zone_device_data
> > + * Return: Page's zone_device_data if owned by DRM pagemap, NULL otherwise
> > */
> > static inline struct drm_pagemap_zdd *drm_pagemap_page_zone_device_data(struct page *page)
> > {
> > struct folio *folio = page_folio(page);
> >
> > + if (WARN_ON_ONCE(page_pgmap(page)->ops != drm_pagemap_pagemap_ops_get()))
> > + return NULL;
> > +
> > return folio_zone_device_data(folio);
> > }
> >
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/gpusvm,pagemap: Do not assume DRM pagemap owns device pages
2026-04-10 20:02 ` Matthew Brost
@ 2026-04-11 9:06 ` Francois Dugast
0 siblings, 0 replies; 6+ messages in thread
From: Francois Dugast @ 2026-04-11 9:06 UTC (permalink / raw)
To: Matthew Brost; +Cc: intel-xe, dri-devel, himal.prasad.ghimiray
On Fri, Apr 10, 2026 at 01:02:00PM -0700, Matthew Brost wrote:
> On Fri, Apr 10, 2026 at 02:46:54PM +0200, Francois Dugast wrote:
> > On Wed, Apr 08, 2026 at 06:55:12PM -0700, Matthew Brost wrote:
> > > Update drm_pagemap_page_zone_device_data() to derive the pgmap ops from
> > > the page and compare them against the DRM pagemap ops. If the ops do not
> > > match, return NULL.
> > >
> > > Also harden two risky call sites by checking for NULL after
> > > hmm_range_fault() or migrate_vma_setup() when migrating to device
> > > memory, as it is possible to encounter device pages that are not owned
> > > by DRM pagemap.
> >
> > Shouldn't we also harden other calls to drm_pagemap_page_zone_device_data() in
> >
> > drm_pagemap_migrate_map_device_private_pages()
> > drm_pagemap_migrate_unmap_pages()
>
> We sanitize prior to this in drm_pagemap_migrate_to_devmem or are
> operating on pages handed back via populate_devmem_pfn.
>
> > drm_pagemap_migrate_populate_ram_pfn()
>
> Operating on page handed back via populate_devmem_pfn. Also wouldn't
> NULL ptr reference.
>
> > __drm_pagemap_migrate_to_ram()
> > drm_pagemap_folio_free()
> > drm_pagemap_migrate_to_ram()
>
> These are in the vops path of pagemap we check against.
>
> > drm_pagemap_page_to_dpagemap()
>
> We sanitize prior to this in drm_gpusvm_get_pages.
>
> Thus all the above sites I figure a warn is enough as it would indicate
> a fairly serious bug in drm gpusvm/pagemap/calling driver which this
> code completely controls.
>
> The case where I do sanitize - after collection via hmm_range_fault,
> migrate_vma_setup, I think it could be possible an outside driver has
> moved pages to private (very unlikely) and this driver also tries to
> move, so abort rather NULL ptr dereference.
Indeed, thanks for pointing it out and for the explanation.
Reviewed-by: Francois Dugast <francois.dugast@intel.com>
>
> So basically mitagated the 2 risky places with sanitization. Ofc we
> could check this everywhere...
>
> Matt
>
> >
> > Francois
> >
> > >
> > > Suggested-by: sashiko.dev
> > > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > > ---
> > > drivers/gpu/drm/drm_gpusvm.c | 5 +++++
> > > drivers/gpu/drm/drm_pagemap.c | 14 ++++++++++----
> > > include/drm/drm_pagemap.h | 5 ++++-
> > > 3 files changed, 19 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> > > index 365a9c0b522a..b3cccd047a21 100644
> > > --- a/drivers/gpu/drm/drm_gpusvm.c
> > > +++ b/drivers/gpu/drm/drm_gpusvm.c
> > > @@ -1506,6 +1506,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> > > struct drm_pagemap_zdd *__zdd =
> > > drm_pagemap_page_zone_device_data(page);
> > >
> > > + if (!__zdd) {
> > > + err = -EINVAL;
> > > + goto err_unmap;
> > > + }
> > > +
> > > if (!ctx->allow_mixed &&
> > > zdd != __zdd && i > 0) {
> > > err = -EOPNOTSUPP;
> > > diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> > > index d82ea7ccb8da..95c951c5b569 100644
> > > --- a/drivers/gpu/drm/drm_pagemap.c
> > > +++ b/drivers/gpu/drm/drm_pagemap.c
> > > @@ -753,10 +753,16 @@ int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,
> > > own_pages++;
> > > goto next;
> > > }
> > > - cur.dpagemap = src_zdd->dpagemap;
> > > - cur.ops = src_zdd->devmem_allocation->ops;
> > > - cur.device = cur.dpagemap->drm->dev;
> > > - pages[i] = src_page;
> > > + if (src_zdd) {
> > > + cur.dpagemap = src_zdd->dpagemap;
> > > + cur.ops = src_zdd->devmem_allocation->ops;
> > > + cur.device = cur.dpagemap->drm->dev;
> > > + pages[i] = src_page;
> > > + } else {
> > > + npages = i;
> > > + err = -EINVAL;
> > > + goto err_finalize;
> > > + }
> > > }
> > > if (!pages[i]) {
> > > cur.dpagemap = NULL;
> > > diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
> > > index 95eb4b66b057..9b7c50932db5 100644
> > > --- a/include/drm/drm_pagemap.h
> > > +++ b/include/drm/drm_pagemap.h
> > > @@ -367,12 +367,15 @@ int drm_pagemap_reinit(struct drm_pagemap *dpagemap);
> > > * drm_pagemap_page_zone_device_data() - Page to zone_device_data
> > > * @page: Pointer to the page
> > > *
> > > - * Return: Page's zone_device_data
> > > + * Return: Page's zone_device_data if owned by DRM pagemap, NULL otherwise
> > > */
> > > static inline struct drm_pagemap_zdd *drm_pagemap_page_zone_device_data(struct page *page)
> > > {
> > > struct folio *folio = page_folio(page);
> > >
> > > + if (WARN_ON_ONCE(page_pgmap(page)->ops != drm_pagemap_pagemap_ops_get()))
> > > + return NULL;
> > > +
> > > return folio_zone_device_data(folio);
> > > }
> > >
> > > --
> > > 2.34.1
> > >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Claude review: drm/gpusvm, pagemap: Do not assume DRM pagemap owns device pages
2026-04-09 1:55 [PATCH] drm/gpusvm, pagemap: Do not assume DRM pagemap owns device pages Matthew Brost
2026-04-10 12:46 ` [PATCH] drm/gpusvm,pagemap: " Francois Dugast
@ 2026-04-12 1:53 ` Claude Code Review Bot
2026-04-12 1:53 ` Claude Code Review Bot
2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-04-12 1:53 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/gpusvm, pagemap: Do not assume DRM pagemap owns device pages
Author: Matthew Brost <matthew.brost@intel.com>
Patches: 4
Reviewed: 2026-04-12T11:53:26.381511
---
This is a single patch that hardens the DRM pagemap/gpusvm code against encountering device pages not owned by DRM pagemap. The motivation is sound: if a device-private page comes from a different subsystem (not DRM pagemap), `drm_pagemap_page_zone_device_data()` returns meaningless data that would be blindly dereferenced. However, the implementation has several issues:
1. Using `WARN_ON_ONCE` for a legitimately expected condition is inappropriate — it should be a quiet check, not a stack trace.
2. The change to `drm_pagemap_page_zone_device_data()` creates an inconsistency with `drm_pagemap_page_to_dpagemap()` which still dereferences the now-nullable return without a NULL check.
3. The patch does not apply cleanly to drm-next because `drm_pagemap_migrate_to_devmem()` was restructured (the dereferences are now inside an `if (mdetails->source_peer_migrates)` block).
4. The `Suggested-by: sashiko.dev` tag is not a valid email address and doesn't conform to the standard kernel format of `Name <email>`.
The defensive NULL checks in the callers (patches 2 and 3 of the diff) are reasonable hardening, but the core change to `drm_pagemap_page_zone_device_data()` needs rework.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread
* Claude review: drm/gpusvm, pagemap: Do not assume DRM pagemap owns device pages
2026-04-09 1:55 [PATCH] drm/gpusvm, pagemap: Do not assume DRM pagemap owns device pages Matthew Brost
2026-04-10 12:46 ` [PATCH] drm/gpusvm,pagemap: " Francois Dugast
2026-04-12 1:53 ` Claude review: drm/gpusvm, pagemap: " Claude Code Review Bot
@ 2026-04-12 1:53 ` Claude Code Review Bot
2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-04-12 1:53 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**drm_pagemap.h: `drm_pagemap_page_zone_device_data()` change**
```c
+ if (WARN_ON_ONCE(page_pgmap(page)->ops != drm_pagemap_pagemap_ops_get()))
+ return NULL;
```
`WARN_ON_ONCE` is inappropriate here. The commit message itself says "it is possible to encounter device pages that are not owned by DRM pagemap" — this is an expected condition, not a kernel bug. `WARN_ON` emits a full stack trace to dmesg/console and is reserved for situations that indicate a programming error. If encountering a non-DRM device page is a legitimate runtime possibility, this should be a plain `if` check:
```c
if (page_pgmap(page)->ops != drm_pagemap_pagemap_ops_get())
return NULL;
```
Additionally, this change makes `drm_pagemap_page_zone_device_data()` return NULL in new cases, but `drm_pagemap_page_to_dpagemap()` at `drm_pagemap.c:1322-1327` was not updated:
```c
struct drm_pagemap *drm_pagemap_page_to_dpagemap(struct page *page)
{
struct drm_pagemap_zdd *zdd = drm_pagemap_page_zone_device_data(page);
return zdd->devmem_allocation->dpagemap; // NULL deref if zdd is NULL
}
```
This exported function will now crash if called on a non-DRM device page. Either this function should also get a NULL guard, or you should audit all callers to ensure they're already protected. Currently in `drm_gpusvm.c:1507`, the call to `drm_pagemap_page_to_dpagemap(page)` is reached after the new `__zdd` NULL check, so that specific path is safe. But other callers (current or future) are not.
**drm_gpusvm.c: NULL check after `drm_pagemap_page_zone_device_data()`**
```c
+ if (!__zdd) {
+ err = -EINVAL;
+ goto err_unmap;
+ }
```
This is the right defensive check. If `drm_pagemap_page_zone_device_data()` returns NULL (either from the new ops check or from `!CONFIG_ZONE_DEVICE`), the code would otherwise:
- Set `zdd = __zdd` (NULL) at line 1499
- Compare `pagemap != page_pgmap(page)` at line 1500, enter that branch
- Call `drm_pagemap_page_to_dpagemap(page)` at line 1507 which dereferences the NULL zdd
The `err_unmap` goto is the correct error path here. No issues with this hunk.
**drm_pagemap.c: NULL check for `src_zdd` in `drm_pagemap_migrate_to_devmem()`**
```c
- cur.dpagemap = src_zdd->dpagemap;
- cur.ops = src_zdd->devmem_allocation->ops;
- cur.device = cur.dpagemap->drm->dev;
- pages[i] = src_page;
+ if (src_zdd) {
+ cur.dpagemap = src_zdd->dpagemap;
+ cur.ops = src_zdd->devmem_allocation->ops;
+ cur.device = cur.dpagemap->drm->dev;
+ pages[i] = src_page;
+ } else {
+ npages = i;
+ err = -EINVAL;
+ goto err_finalize;
+ }
```
Several issues:
1. **Does not apply to drm-next.** In the current drm-next tree, these lines are inside `if (mdetails->source_peer_migrates)` (line 654), which the patch context doesn't show. The patch was made against a different base. This needs to be rebased.
2. **`npages = i` is correct** (vs `npages = i + 1` used at the existing error at line 688). At this point in the loop, `drm_pagemap_get_devmem_page(page, order, zdd)` at line 681 has NOT yet been called for page `i`, so page `i`'s zone_device_data is uninitialized. `drm_pagemap_migration_unlock_put_pages()` must not process it, so `npages = i` limits cleanup to pages 0..i-1 only. This is correct.
3. **Consider whether this error path leaks destination page `i`.** The destination page at index `i` was allocated by `populate_devmem_pfn()` (line 630) but `migrate_pfn()` at line 667 was not called for it. Whether `migrate_vma_finalize()` can clean it up depends on whether `populate_devmem_pfn` already set `MIGRATE_PFN_VALID`. If not, this page may leak. This should be verified against the driver implementations (e.g., Xe's `populate_devmem_pfn` callback).
**Commit message / metadata:**
`Suggested-by: sashiko.dev` — this is not a valid kernel tag. The `Suggested-by` format requires `Name <email>`, per `Documentation/process/submitting-patches.rst`. A bare domain name is not an email address.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-04-12 1:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-09 1:55 [PATCH] drm/gpusvm, pagemap: Do not assume DRM pagemap owns device pages Matthew Brost
2026-04-10 12:46 ` [PATCH] drm/gpusvm,pagemap: " Francois Dugast
2026-04-10 20:02 ` Matthew Brost
2026-04-11 9:06 ` Francois Dugast
2026-04-12 1:53 ` Claude review: drm/gpusvm, pagemap: " Claude Code Review Bot
2026-04-12 1:53 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox