mirror of
https://github.com/torvalds/linux.git
synced 2026-09-23 22:14:03 +02:00
drm/xe/mmio_gem: fix destroy flow
xe_mmio_gem_destroy() currently frees the GEM object directly, bypassing
reference counting. Since existing VMAs hold a reference and the fault
handler accesses the object through vma->vm_private_data, this is
use-after-free. Additionally, nothing prevents the fault handler from
installing PTEs to the real MMIO after destroy.
Fix this with proper synchronization and refcounting. Also, do not set
vm_pgoff to zero. Many DRM drivers do this because helpers like
dma_mmap_pages() interpret vm_pgoff as an intra-buffer page offset;
leaving the DRM fake offset there would break these helpers.
Those drivers can get away with zeroing it because they map eagerly -
all PTEs are established before mmap returns, so vm_pgoff is never
consulted again. Our driver does not use such helpers and the newly
introduced call to drm_vma_node_unmap() relies on vm_pgoff being untouched.
v2: (Matt Auld)
- use dma_resv lock to serialize fault handler with destroy
- SIGBUS on access after destroy
Fixes: 1ffcf8b8ae ("drm/xe: Support for mmap-ing mmio regions")
Assisted-by: GitHub-Copilot:claude-opus-4.6
Signed-off-by: Ilia Levi <ilia.levi@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Link: https://patch.msgid.link/20260908165046.1393557-16-matthew.auld@intel.com
(cherry picked from commit fb2ee38bab8025ad6a7a9cbb4635c5a178e4a7bc)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
This commit is contained in:
parent
de40d31275
commit
d0c0952878
|
|
@ -38,6 +38,7 @@ struct xe_mmio_gem {
|
|||
struct drm_gem_object base;
|
||||
phys_addr_t phys_addr;
|
||||
struct page *dummy_page; /* protected by the GEM's dma_resv */
|
||||
bool destroyed; /* protected by the GEM's dma_resv */
|
||||
};
|
||||
|
||||
static int xe_mmio_gem_vm_may_split(struct vm_area_struct *area, unsigned long addr)
|
||||
|
|
@ -150,8 +151,22 @@ static void xe_mmio_gem_free(struct drm_gem_object *base)
|
|||
*/
|
||||
void xe_mmio_gem_destroy(struct xe_mmio_gem *gem, struct drm_file *file)
|
||||
{
|
||||
drm_vma_node_revoke(&gem->base.vma_node, file);
|
||||
xe_mmio_gem_free(&gem->base);
|
||||
struct drm_gem_object *base = &gem->base;
|
||||
struct drm_device *dev = base->dev;
|
||||
|
||||
drm_vma_node_revoke(&base->vma_node, file);
|
||||
|
||||
dma_resv_lock(base->resv, NULL);
|
||||
gem->destroyed = true;
|
||||
dma_resv_unlock(base->resv);
|
||||
/*
|
||||
* Setting 'destroyed' under lock takes care of the subsequent faults.
|
||||
* Zap the existing PTEs to cut off access to the real MMIO through
|
||||
* currently mapped pages.
|
||||
*/
|
||||
drm_vma_node_unmap(&base->vma_node, dev->anon_inode->i_mapping);
|
||||
|
||||
drm_gem_object_put(base);
|
||||
}
|
||||
|
||||
static int xe_mmio_gem_mmap(struct drm_gem_object *base, struct vm_area_struct *vma)
|
||||
|
|
@ -162,8 +177,6 @@ static int xe_mmio_gem_mmap(struct drm_gem_object *base, struct vm_area_struct *
|
|||
if ((vma->vm_flags & VM_SHARED) == 0)
|
||||
return -EINVAL;
|
||||
|
||||
/* Set vm_pgoff (used as a fake buffer offset by DRM) to 0 */
|
||||
vma->vm_pgoff = 0;
|
||||
vma->vm_page_prot = pgprot_noncached(vma_get_page_prot(vma));
|
||||
vm_flags_set(vma, VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP |
|
||||
VM_DONTCOPY | VM_NORESERVE);
|
||||
|
|
@ -176,10 +189,9 @@ static int alloc_dummy_page_if_needed(struct drm_gem_object *base)
|
|||
{
|
||||
struct xe_mmio_gem *obj = to_xe_mmio_gem(base);
|
||||
|
||||
dma_resv_lock(base->resv, NULL);
|
||||
dma_resv_assert_held(base->resv);
|
||||
if (!obj->dummy_page)
|
||||
obj->dummy_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
|
||||
dma_resv_unlock(base->resv);
|
||||
|
||||
return obj->dummy_page ? 0 : -ENOMEM;
|
||||
}
|
||||
|
|
@ -200,7 +212,7 @@ static vm_fault_t xe_mmio_gem_vm_fault_dummy_page(struct vm_fault *vmf)
|
|||
vm_get_page_prot(vma->vm_flags));
|
||||
}
|
||||
|
||||
static vm_fault_t xe_mmio_gem_vm_fault(struct vm_fault *vmf)
|
||||
static vm_fault_t xe_mmio_gem_vm_fault_locked(struct vm_fault *vmf)
|
||||
{
|
||||
struct vm_area_struct *vma = vmf->vma;
|
||||
struct drm_gem_object *base = vma->vm_private_data;
|
||||
|
|
@ -210,6 +222,10 @@ static vm_fault_t xe_mmio_gem_vm_fault(struct vm_fault *vmf)
|
|||
unsigned long addr, pfn;
|
||||
int idx;
|
||||
|
||||
dma_resv_assert_held(base->resv);
|
||||
if (obj->destroyed)
|
||||
return VM_FAULT_SIGBUS;
|
||||
|
||||
if (!drm_dev_enter(dev, &idx)) {
|
||||
/*
|
||||
* Provide a dummy page to avoid SIGBUS for events such as hot-unplug.
|
||||
|
|
@ -232,3 +248,15 @@ static vm_fault_t xe_mmio_gem_vm_fault(struct vm_fault *vmf)
|
|||
drm_dev_exit(idx);
|
||||
return ret;
|
||||
}
|
||||
|
||||
static vm_fault_t xe_mmio_gem_vm_fault(struct vm_fault *vmf)
|
||||
{
|
||||
struct vm_area_struct *vma = vmf->vma;
|
||||
struct drm_gem_object *base = vma->vm_private_data;
|
||||
vm_fault_t ret;
|
||||
|
||||
dma_resv_lock(base->resv, NULL);
|
||||
ret = xe_mmio_gem_vm_fault_locked(vmf);
|
||||
dma_resv_unlock(base->resv);
|
||||
return ret;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user