summaryrefslogtreecommitdiff
path: root/drivers/gpu
diff options
context:
space:
mode:
authorLinus Torvalds <torvalds@linux-foundation.org>2026-10-21 07:19:30 -1000
committerLinus Torvalds <torvalds@linux-foundation.org>2026-10-21 07:19:30 -1000
commit22a78a25438d246e4e951960523bee6a5155897d (patch)
treea7f0fa921b89128fe082215e995ff8b091f8a07d /drivers/gpu
parenta804c5742bff224d4cb4d25dd383554d598be32b (diff)
parent14b482c98dfe9683b12338561a6e9bd44f7337a9 (diff)
Merge __{get,put}_user() removal branch
This is long over-due: the __get_user() and __put_user() functions are entirely historical, and are never the rigth thing to use any more. And when mis-used (read: every case) they are not only slower than the right thing, but also insecure because they don't check the user address range. Most of the usage sites have been converted long ago, but various left-overs remained. So this started out as a single patch that I wrote as a "show people how to convert __{get,put}_user() to modern code". In particular, Thomas last year implemented the very nice new scoped user access region helpers (see commit e497310b4ffb: "uaccess: Provide scoped user access regions") that makes the modern interfaces particularly easy to use and really removes any excuse for the old horrors. That small example patch then grew to this, partly using Claude as a glorified coccinelle script to apply the pattern to other places. There were a few different reasons for using __put_user() and __get_user() rather than the alternatives, but the original historical one was performance: when doing multiple accesses off the same base pointer, doing a single range check followed by multiple non-checked accesses used to create smaller and faster code. The keyword here is "used to". It hasn't been true for years, because the range check isn't actually the real cost these days. The real cost of user accesses on modern hardware is turning the user accesses on and off in the first place (on x86, using the STAC/CLAC instructions). Those sequences get turned into 'scoped_user_read_access() { }' blocks, see for example that first patch in the series: "signal: use scoped user access for old-style sigaction". Another, historical reason for using __get_user() and friends is that you actually want to do a kernel pointer access, but the pointer might not be entirely reliable (typically during fault handling), so you want to have exception handling for the kernel access. The modern way to do that kind of non-faulting kernel access is using "get_kernel_nofault()", but some code predates that helper existing at all, and used __get_user() as a way to just get a memory access with exception handling. For that pattern, see for example "alpha: use get_kernel_nofault() in dik_show_code()" A third bad reason for using __get_user() and friends was that the proper get_user() had a "might_fault()" in it, and code that ran in atomic regions but hadn't disabled page faults would result in a warning. That kind of code needs to explicitly disable page faults, because "atomic region" is not well defined at run-time and can be invisible (eg a preempt-off region in a non-preemptible kernel). See "vdpa_sim: disable page faults around vringh_need_notify_iotlb()" for an example of that pattern, where the local_bh_disable() now also grows a pagefault_disable(), which makes the normal get_user() no longer warn about the user access in non-process context, because it can see that the potential page fault will not cause problems. And finally, the really nasty reason to use __get_user() and friends: pointers that can be either kernel pointers or user pointers. We've gotten rid of almost all such cases, but a couple remained. See for example "x86/sev: Use explicit user-or-kernel accessors for #VC memory operands". NOTE! This series removes all __get/put_user() calls from generic code but only does the final removal of the whole interface on x86. Other architectures have not been entirely converted, for a variety of reasons: - arm64 makes __{get,put}_user() be exactly the same as {get,put}_user() so it's harmless and didn't have the dangers of the legacy use cases. - any architecture that uses <asm-generic/uaccess.h> will still use __{get,put}_user as the underlying helper function name - several architectures had some legacy uses that weren't entirely trivial and I didn't want to extend this patch series to things that I don't know and that don't really matter. but I did do a couple of the obvious "follows the exact same pattern" just because it could be done mindlessly and automatically. Let's see if I broke anything, but we should have done this years ago. * uaccess-cleanups: (48 commits) drm/i915/gem: ignore errors on presumed_offset write-back powerpc/align: use get_user_instr() for a user-mode instruction powerpc/signal64: use scoped user access in setup_trampoline() powerpc/signal64: use get_user() for the single context reads powerpc/signal32: use get_user() for the single context reads KVM: PPC: Book3S HV: use get_user()/put_user() for the HPT fd powerpc/powernv: use get_user()/put_user() in the LPC debugfs files s390: use get_user()/put_user() instead of the unchecked variants ARM: OABI: use scoped user access in epoll_put_uevent() ARM: signal: stop using __put_user() for the signal frame ARM: signal: check the FDPIC function descriptor address alpha: split the prefetch check in do_page_fault() by mode openrisc: use get_kernel_nofault() in show_registers() alpha: use get_kernel_nofault() in dik_show_code() nios2: use scoped user access for the signal frame nios2: check the signal frame address before writing to it hexagon: use scoped user access for the signal frame alpha: use scoped user access when setting up signal frames alpha: use scoped user access in the sigreturn path alpha: use scoped user access in osf_sigaction() ...
Diffstat (limited to 'drivers/gpu')
-rw-r--r--drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c63
-rw-r--r--drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c8
2 files changed, 43 insertions, 28 deletions
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index d5aadced7da0..1b7f5bf13e50 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1522,8 +1522,9 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
/*
* We must check that the entire relocation array is safe
- * to read. However, if the array is not writable the user loses
- * the updated relocation values.
+ * to read. Whether it is also writable only shows when we
+ * write back the presumed offsets below, and if that fails we
+ * fall back to the slow path.
*/
if (unlikely(!access_ok(urelocs, remain * sizeof(*urelocs))))
return -EFAULT;
@@ -1553,6 +1554,7 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
remain -= count;
do {
u64 offset = eb_relocate_entry(eb, ev, r);
+ int err;
if (likely(offset == 0))
continue;
@@ -1562,28 +1564,41 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
goto out;
}
/*
- * Note that reporting an error now
- * leaves everything in an inconsistent
- * state as we have *already* changed
- * the relocation value inside the
- * object. As we have not changed the
- * reloc.presumed_offset or will not
- * change the execobject.offset, on the
- * call we may not rewrite the value
- * inside the object, leaving it
- * dangling and causing a GPU hang. Unless
- * userspace dynamically rebuilds the
- * relocations on each execbuf rather than
- * presume a static tree.
+ * Write back the presumed offset. We hold the
+ * objects' dma_resv locks here, so this must not
+ * take a page fault: that would take mmap_lock
+ * under dma_resv, the opposite of the order our
+ * own fault handler uses.
*
- * We did previously check if the relocations
- * were writable (access_ok), an error now
- * would be a strange race with mprotect,
- * having already demonstrated that we
- * can read from this userspace address.
+ * If the write fails (the page is read-only,
+ * copy-on-write, or gone since we read it), we
+ * must not just carry on. We have already changed
+ * the relocation value inside the object, and a
+ * stale presumed_offset could later match again
+ * and make us skip a relocation that is needed,
+ * hanging the GPU. So fall back to the slow path,
+ * as for a failed read: it retries after
+ * prefaulting and, failing that, marks every
+ * presumed_offset invalid with faults allowed and
+ * no locks held.
*/
offset = gen8_canonical_addr(offset & ~UPDATE);
- __put_user(offset, &urelocs[r - stack].presumed_offset);
+ pagefault_disable();
+ err = put_user(offset, &urelocs[r - stack].presumed_offset);
+ pagefault_enable();
+
+ /*
+ * Error handling is disabled on purpose. No, it makes
+ * no sense, and the code will loop over the relocations
+ * without writing them back and just waste CPU time,
+ * but that's apparently what the mprotect test-cases
+ * want. See commit 97a37c919f62 ("drm/i915/gem: Ignore
+ * readonly failures when updating relocs")
+ */
+ if (unlikely(err) && 0) {
+ remain = -EFAULT;
+ goto out;
+ }
} while (r++, --count);
urelocs += ARRAY_SIZE(stack);
} while (remain);
@@ -1635,11 +1650,11 @@ static int check_relocations(const struct drm_i915_gem_exec_object2 *entry)
end = addr + size;
for (; addr < end; addr += PAGE_SIZE) {
- int err = __get_user(c, addr);
+ int err = get_user(c, addr);
if (err)
return err;
}
- return __get_user(c, end - 1);
+ return get_user(c, end - 1);
}
static int eb_copy_relocations(const struct i915_execbuffer *eb)
@@ -2837,7 +2852,7 @@ add_timeline_fence_array(struct i915_execbuffer *eb,
if (user_fence.flags & __I915_EXEC_FENCE_UNKNOWN_FLAGS)
return -EINVAL;
- if (__get_user(point, user_values++))
+ if (get_user(point, user_values++))
return -EFAULT;
syncobj = drm_syncobj_find(eb->file, user_fence.handle);
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index d01acfb7d93d..e6748856a0a8 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -1465,7 +1465,7 @@ static int __igt_mmap_access(struct drm_i915_private *i915,
return addr;
ptr = (unsigned long __user *)addr;
- err = __put_user(A, ptr);
+ err = put_user(A, ptr);
if (err) {
pr_err("%s(%s): failed to write into user mmap\n",
obj->mm.region->name, repr_mmap_type(type));
@@ -1490,7 +1490,7 @@ static int __igt_mmap_access(struct drm_i915_private *i915,
intel_gt_flush_ggtt_writes(to_gt(i915));
- err = __get_user(y, ptr);
+ err = get_user(y, ptr);
if (err) {
pr_err("%s(%s): failed to read from user mmap\n",
obj->mm.region->name, repr_mmap_type(type));
@@ -1738,12 +1738,12 @@ static int prefault_range(u64 start, u64 len)
end = addr + len;
for (; addr < end; addr += PAGE_SIZE) {
- err = __get_user(c, addr);
+ err = get_user(c, addr);
if (err)
return err;
}
- return __get_user(c, end - 1);
+ return get_user(c, end - 1);
}
static int __igt_mmap_revoke(struct drm_i915_private *i915,