From: Ilia Levi <ilia.levi@intel.com>
To: intel-xe@lists.freedesktop.org
Cc: ilia.levi@intel.com, koby.elbaz@intel.com,
meny.yossefi@intel.com, shuicheng.lin@intel.com,
thomas.hellstrom@intel.com, matthew.auld@intel.com,
matthew.brost@intel.com
Subject: [PATCH v3 6/7] drm/xe/mmio_gem: fix destroy flow
Date: Thu, 23 Jul 2026 19:18:31 +0300 [thread overview]
Message-ID: <20260723161832.137153-7-ilia.levi@intel.com> (raw)
In-Reply-To: <20260723161832.137153-1-ilia.levi@intel.com>
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: 1ffcf8b8ae8a ("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>
---
drivers/gpu/drm/xe/xe_mmio_gem.c | 42 ++++++++++++++++++++++++++------
1 file changed, 35 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_mmio_gem.c b/drivers/gpu/drm/xe/xe_mmio_gem.c
index 5bd2759fc876..ef48642a0733 100644
--- a/drivers/gpu/drm/xe/xe_mmio_gem.c
+++ b/drivers/gpu/drm/xe/xe_mmio_gem.c
@@ -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(vm_get_page_prot(vma->vm_flags));
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;
+}
--
2.49.1
next prev parent reply other threads:[~2026-07-23 16:19 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-23 16:18 [PATCH v3 0/7] drm/xe/mmio_gem: fix fault handler and destroy path Ilia Levi
2026-07-23 16:18 ` [PATCH v3 1/7] drm/xe/mmio_gem: forbid VMA split Ilia Levi
2026-07-23 16:25 ` Matthew Auld
2026-07-23 16:18 ` [PATCH v3 2/7] drm/xe/mmio_gem: use write-back mapping for dummy page Ilia Levi
2026-07-23 17:16 ` Matthew Auld
2026-07-24 9:49 ` Levi, Ilia
2026-07-28 16:37 ` Matthew Auld
2026-07-23 16:18 ` [PATCH v3 3/7] drm/xe/mmio_gem: simplify fault handler loop Ilia Levi
2026-07-23 17:22 ` Matthew Auld
2026-07-23 16:18 ` [PATCH v3 4/7] drm/xe/mmio_gem: Revoke drm_vma_node on xe_mmio_gem destroy Ilia Levi
2026-07-23 16:18 ` [PATCH v3 5/7] drm/xe/mmio_gem: cache the dummy page per object Ilia Levi
2026-07-28 16:33 ` Matthew Auld
2026-07-23 16:18 ` Ilia Levi [this message]
2026-07-24 12:50 ` [PATCH v3 6/7] drm/xe/mmio_gem: fix destroy flow Matthew Auld
2026-07-23 16:18 ` [PATCH v3 7/7] drm/xe: convert PCI barrier mmap to use xe_mmio_gem Ilia Levi
2026-07-23 16:59 ` ✓ CI.KUnit: success for drm/xe/mmio_gem: fix fault handler and destroy path (rev3) Patchwork
2026-07-23 17:34 ` ✓ Xe.CI.BAT: " Patchwork
2026-07-24 17:14 ` ✓ Xe.CI.FULL: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260723161832.137153-7-ilia.levi@intel.com \
--to=ilia.levi@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=koby.elbaz@intel.com \
--cc=matthew.auld@intel.com \
--cc=matthew.brost@intel.com \
--cc=meny.yossefi@intel.com \
--cc=shuicheng.lin@intel.com \
--cc=thomas.hellstrom@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox