github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-10-23 23:51:02 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

drm/i915/gem: Don't fault on presumed_offset write-back

Browse Source 
The execbuffer relocation fast path writes each updated presumed_offset
back to the user's relocation array with __put_user(), ignoring any
failure.  It does that while holding the objects' dma_resv locks, and
with page faults enabled unless the relocation cache happens to have an
atomic mapping active.  So if that write faults, for example on a
copy-on-write page after a fork(), the fault handler takes mmap_lock
under dma_resv.  That is the opposite of the order the i915 fault
handler uses, which takes the object lock under mmap_lock.

The comment above it claimed this could not happen, because "we did
previously check if the relocations were writable (access_ok)".
access_ok() is purely a range check.  The comment dates from when this
was access_ok(VERIFY_WRITE), and that only ever made a difference on the
original i386, whose write-protect bit did not stop supervisor writes.
So nothing has made sure the array was writable for decades.

Do the write with page faults disabled, and if it fails, fall back to
the slow path exactly as we already do when reading the relocations
fails part-way through.  We cannot just ignore the failure: the object
has already been relocated, and a stale presumed_offset could later
match again and skip a relocation that is needed.  The slow path
prefaults and retries.  If the write still fails, it copies the
relocations and marks every presumed_offset invalid with faults allowed
and no locks held, which breaks copy-on-write as needed.

One user-visible change: a relocation array in genuinely read-only
memory used to get through the fast path, silently losing the
presumed_offset updates.  It now gets -EFAULT from the slow path, as it
already did whenever anything else sent execbuffer there.  The uapi has
the kernel updating presumed_offset, so the array is expected to be
writable.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
Linus Torvalds 2026-10-19 19:24:15 -10:00
parent 738989053b
commit 03c19e449a
1 changed files with 27 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
View File

@ -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,32 @@ 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();
if (unlikely(err)) {
remain = -EFAULT;
goto out;
}
} while (r++, --count);
urelocs += ARRAY_SIZE(stack);
} while (remain);