The userptr interface records the requested access mode in
etnaviv_obj->userptr.ro (set when ETNA_USERPTR_WRITE is absent), but
etnaviv_iommu_map_gem() ignores it and maps every buffer with
ETNAVIV_PROT_READ | ETNAVIV_PROT_WRITE. A buffer pinned without
write permission is therefore writable by the GPU, which can mutate
page-cache data visible to other processes.
Build the protection mask from userptr.ro instead.
On MMUv2 this is a real enforcement change: etnaviv_iommuv2_map()
encodes the writeable bit per entry, so omitting ETNAVIV_PROT_WRITE
clears MMUv2_PTE_WRITEABLE and the hardware refuses GPU writes.
MMUv1 cannot enforce read-only at all. Its page table entries are
bare physical addresses -- etnaviv_iommuv1_map() accepts a prot
argument and discards it -- so passing ETNAVIV_PROT_READ has no
effect on v1 hardware. What can be improved there is the mapping
path taken: a single-entry contiguous userptr BO currently takes the
MMUv1 linear-window shortcut in etnaviv_iommu_map_gem(), which hands
the GPU an offset into a window of up to 2 GiB rather than a
page-table mapping of just the pinned pages. Setting
ETNA_BO_FORCE_MMU on read-only userptr BOs at creation time
suppresses that shortcut and confines the GPU to the mapped pages.
To be explicit, because the two halves differ: this makes read-only
userptr genuinely read-only on MMUv2, and on MMUv1 it only narrows
what the GPU can reach. A read-only userptr BO on v1 hardware
remains GPU-writable. Rejecting such mappings outright was
considered and dropped.
Fixes: a8c21a5451d8 ("drm/etnaviv: add initial etnaviv DRM driver")
Link:
https://lore.kernel.org/all/[email protected]/
Suggested-by: Lucas Stach <[email protected]>
Cc: [email protected]
Assisted-by: Codex:gpt-5.5
Assisted-by: Claude:claude-opus-5
Signed-off-by: Christopher Lusk <[email protected]>
---
Changes since RFC v1 [1]:
- Per Lucas Stachs feedback on Ziyi Guos parallel patch [2][3]:
set ETNA_BO_FORCE_MMU on read-only userptr BOs at creation time
rather than rejecting read-only mappings on MMUv1 with -ENODEV.
- Commit message now states the MMUv1 limitation explicitly:
forcing the page-table path is containment, not write protection,
because MMUv1 PTEs have no writeable bit.
- Two-file change (etnaviv_gem.c + etnaviv_mmu.c) instead of the
single-file etnaviv_mmu.c change in the RFC.
The same bug was independently found by Ziyi Guo, who posted a fix [2]
six days before my RFC. Neither patch was merged. This v2 takes the
approach Lucas preferred in reply to his [3], and keeps his
prot-in-a-local shape rather than inlining the condition at the call
site.
Open question for Lucas: on MMUv1 a caller asking for a read-only
userptr BO now gets a mapping that is not read-only, silently. I
have left it silent to keep the diff small for stable, but a
drm_warn_once() in etnaviv_gem_new_userptr() would make the
limitation visible to userspace developers. Happy to add one if you
prefer.
Tooling and testing, per Documentation/process/generated-content.rst:
- The bug was found by static analysis rather than by hand: a
mechanism-first variant-analysis pass over page-backed buffer
sinks, looking for sites where a recorded access-mode flag is not
carried into the mapping that would enforce it. The same pass
flagged an equivalent shape in drivers/accel/ivpu, which turned out
to duplicate commit 7dd57d7a6350.
- The patch, the changelog and this text were drafted with LLM
assistance (see the Assisted-by tags) and reviewed line by line by
me; I take responsibility for all of it.
- The MMUv1/MMUv2 asymmetry above was established by reading
etnaviv_iommuv1_map() and etnaviv_iommuv2_map() directly, not by
trusting the tool: an earlier draft of this patch claimed MMUv1
enforcement and was wrong.
- Testing: compile-tested only. Built on x86_64 with COMPILE_TEST=y,
CONFIG_DRM_ETNAVIV=m, gcc 15.2.1, W=1 -- clean, no new warnings.
Base: drm-misc-next abc1e559f8e5. No Vivante hardware is available
to me, so neither the MMUv2 enforcement path nor the MMUv1
containment change is verified at runtime. A test from anyone with
GC hardware would be very welcome, and I am happy to arrange
hardware validation before merge if you would rather have it first.
[1] https://lore.kernel.org/all/[email protected]/
[2]
https://lore.kernel.org/all/[email protected]/
[3]
https://lore.kernel.org/all/[email protected]/
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 13 ++++++++++++-
drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 10 +++++++++-
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index b0436a1e10..b043a56e3e 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -735,9 +735,20 @@ int etnaviv_gem_new_userptr(struct drm_device *dev, struct
drm_file *file,
uintptr_t ptr, u32 size, u32 flags, u32 *handle)
{
struct etnaviv_gem_object *etnaviv_obj;
+ u32 bo_flags = ETNA_BO_CACHED;
int ret;
- ret = etnaviv_gem_new_private(dev, size, ETNA_BO_CACHED,
+ /*
+ * Keep read-only userptr BOs out of the MMUv1 linear window, which
+ * would expose far more than the pinned pages to the GPU. MMUv1
+ * PTEs have no writeable bit, so this confines the GPU rather than
+ * making the BO read-only; MMUv2 enforces read-only per PTE in
+ * etnaviv_iommu_map_gem().
+ */
+ if (!(flags & ETNA_USERPTR_WRITE))
+ bo_flags |= ETNA_BO_FORCE_MMU;
+
+ ret = etnaviv_gem_new_private(dev, size, bo_flags,
&etnaviv_gem_userptr_ops, &etnaviv_obj);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
index e3572461b5..569f729681 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
@@ -269,10 +269,18 @@ int etnaviv_iommu_map_gem(struct etnaviv_iommu_context
*context,
{
struct sg_table *sgt = etnaviv_obj->sgt;
struct drm_mm_node *node;
+ int prot = ETNAVIV_PROT_READ;
int ret;
lockdep_assert_held(&etnaviv_obj->lock);
+ /*
+ * Read-only userptr BOs drop ETNAVIV_PROT_WRITE. MMUv2 honors this
+ * via MMUv2_PTE_WRITEABLE; MMUv1 ignores prot entirely.
+ */
+ if (!etnaviv_obj->userptr.mm || !etnaviv_obj->userptr.ro)
+ prot |= ETNAVIV_PROT_WRITE;
+
mutex_lock(&context->lock);
/* v1 MMU can optimize single entry (contiguous) scatterlists */
@@ -301,7 +309,7 @@ int etnaviv_iommu_map_gem(struct etnaviv_iommu_context
*context,
mapping->iova = node->start;
ret = etnaviv_iommu_map(context, node->start, etnaviv_obj->size, sgt,
- ETNAVIV_PROT_READ | ETNAVIV_PROT_WRITE);
+ prot);
if (ret < 0) {
drm_mm_remove_node(node);
--
2.54.0
