Re: [PATCH v2 02/17] drm/pagemap, drm/xe: Add refcounting to struct drm_pagemap

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

Hi, Himal!

On Wed, 2025-11-12 at 11:37 +0530, Ghimiray, Himal Prasad wrote:
> 
> 
> On 11-11-2025 22:13, Thomas Hellström wrote:
> > With the end goal of being able to free unused pagemaps
> > and allocate them on demand, add a refcount to struct drm_pagemap,
> > remove the xe embedded drm_pagemap, allocating and freeing it
> > explicitly.
> > 
> > v2:
> > - Make the drm_pagemap pointer in drm_gpusvm_pages reference-
> > counted.
> > 
> > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > Reviewed-by: Matthew Brost <matthew.brost@intel.com> #v1
> > ---
> >   drivers/gpu/drm/drm_gpusvm.c       |  4 ++-
> >   drivers/gpu/drm/drm_pagemap.c      | 51
> > ++++++++++++++++++++++++++++++
> >   drivers/gpu/drm/xe/xe_svm.c        | 26 ++++++++++-----
> >   drivers/gpu/drm/xe/xe_vram_types.h |  2 +-
> >   include/drm/drm_pagemap.h          | 36 +++++++++++++++++++++
> >   5 files changed, 109 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gpusvm.c
> > b/drivers/gpu/drm/drm_gpusvm.c
> > index 73e550c8ff8c..1f96375d1f2b 100644
> > --- a/drivers/gpu/drm/drm_gpusvm.c
> > +++ b/drivers/gpu/drm/drm_gpusvm.c
> > @@ -1038,6 +1038,7 @@ static void __drm_gpusvm_unmap_pages(struct
> > drm_gpusvm *gpusvm,
> >   		flags.has_dma_mapping = false;
> >   		WRITE_ONCE(svm_pages->flags.__flags,
> > flags.__flags);
> >   
> > +		drm_pagemap_put(svm_pages->dpagemap);
> >   		svm_pages->dpagemap = NULL;
> >   	}
> >   }
> > @@ -1431,7 +1432,8 @@ int drm_gpusvm_get_pages(struct drm_gpusvm
> > *gpusvm,
> >   
> >   	if (pagemap) {
> >   		flags.has_devmem_pages = true;
> > -		svm_pages->dpagemap = dpagemap;
> > +		drm_pagemap_put(svm_pages->dpagemap);
> 
> Dont we risk a UAF for dpagemap if svm_pages->dpagemap is same as
> dpagemap ?

Thanks for reviewing. Here the dpagemap refcount is protected from
dropping to zero by a page presence in the CPU address space and us
holding the notifier lock. But agree that this looks bad from a
reader's perspective. I'll fix that up.

Thanks,
Thomas