1. 24 4月, 2019 1 次提交
    • C
      drm/i915: Avoid use-after-free in reporting create.size · 929eec99
      Chris Wilson 提交于
      We have to avoid chasing after a userspace race!
      
      <3>[  473.114328] BUG: KASAN: use-after-free in i915_gem_create+0x1d2/0x1f0 [i915]
      <3>[  473.114389] Read of size 8 at addr ffff88815bf1d840 by task gem_flink_race/1541
      
      <4>[  473.114464] CPU: 1 PID: 1541 Comm: gem_flink_race Tainted: G     U            5.1.0-rc4-g7d07e025e786-kasan_88+ #1
      <4>[  473.114469] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./J4205-ITX, BIOS P1.10 09/29/2016
      <4>[  473.114474] Call Trace:
      <4>[  473.114488]  dump_stack+0x7c/0xbb
      <4>[  473.114612]  ? i915_gem_create+0x1d2/0x1f0 [i915]
      <4>[  473.114621]  print_address_description+0x65/0x270
      <4>[  473.114728]  ? i915_gem_create+0x1d2/0x1f0 [i915]
      <4>[  473.114839]  ? i915_gem_create+0x1d2/0x1f0 [i915]
      <4>[  473.114848]  kasan_report+0x149/0x18d
      <4>[  473.114962]  ? i915_gem_create+0x1d2/0x1f0 [i915]
      <4>[  473.115069]  i915_gem_create+0x1d2/0x1f0 [i915]
      <4>[  473.115176]  ? i915_gem_object_create.part.28+0x4b0/0x4b0 [i915]
      <4>[  473.115289]  ? i915_gem_dumb_create+0x1a0/0x1a0 [i915]
      <4>[  473.115297]  drm_ioctl_kernel+0x192/0x260
      <4>[  473.115306]  ? drm_ioctl_permit+0x280/0x280
      <4>[  473.115326]  drm_ioctl+0x67c/0x960
      <4>[  473.115438]  ? i915_gem_dumb_create+0x1a0/0x1a0 [i915]
      <4>[  473.115448]  ? drm_getstats+0x20/0x20
      <4>[  473.115459]  ? __lock_acquire+0xa66/0x3fe0
      <4>[  473.115474]  ? _raw_spin_unlock_irqrestore+0x39/0x60
      <4>[  473.115485]  ? debug_object_active_state+0x2ea/0x4e0
      <4>[  473.115496]  ? debug_show_all_locks+0x2d0/0x2d0
      <4>[  473.115513]  do_vfs_ioctl+0x18d/0xfa0
      <4>[  473.115522]  ? check_flags.part.27+0x440/0x440
      <4>[  473.115532]  ? ioctl_preallocate+0x1a0/0x1a0
      <4>[  473.115547]  ? __fget+0x2ac/0x410
      <4>[  473.115561]  ? __ia32_sys_dup3+0xb0/0xb0
      <4>[  473.115569]  ? rwlock_bug.part.0+0x90/0x90
      <4>[  473.115590]  ksys_ioctl+0x35/0x70
      <4>[  473.115597]  ? lockdep_hardirqs_off+0x1cb/0x2b0
      <4>[  473.115608]  __x64_sys_ioctl+0x6a/0xb0
      <4>[  473.115614]  ? lockdep_hardirqs_on+0x342/0x590
      <4>[  473.115623]  do_syscall_64+0x97/0x400
      <4>[  473.115633]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
      <4>[  473.115641] RIP: 0033:0x7fce590d55d7
      <4>[  473.115649] Code: b3 66 90 48 8b 05 b1 48 2d 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 81 48 2d 00 f7 d8 64 89 01 48
      <4>[  473.115655] RSP: 002b:00007fce4d525ba8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
      <4>[  473.115662] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fce590d55d7
      <4>[  473.115667] RDX: 00007fce4d525c10 RSI: 00000000c010645b RDI: 0000000000000007
      <4>[  473.115672] RBP: 00007fce4d525c10 R08: 00007fce4d526700 R09: 00007fce4d526700
      <4>[  473.115677] R10: 0000000000000054 R11: 0000000000000246 R12: 00000000c010645b
      <4>[  473.115682] R13: 0000000000000007 R14: 0000000000000000 R15: 00007ffe0e4a7450
      
      <3>[  473.115731] Allocated by task 1541:
      <4>[  473.115766]  kmem_cache_alloc+0xce/0x290
      <4>[  473.115895]  i915_gem_object_create.part.28+0x1c/0x4b0 [i915]
      <4>[  473.116000]  i915_gem_create+0xe3/0x1f0 [i915]
      <4>[  473.116008]  drm_ioctl_kernel+0x192/0x260
      <4>[  473.116013]  drm_ioctl+0x67c/0x960
      <4>[  473.116020]  do_vfs_ioctl+0x18d/0xfa0
      <4>[  473.116026]  ksys_ioctl+0x35/0x70
      <4>[  473.116032]  __x64_sys_ioctl+0x6a/0xb0
      <4>[  473.116038]  do_syscall_64+0x97/0x400
      <4>[  473.116044]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
      
      <3>[  473.116071] Freed by task 1542:
      <4>[  473.116101]  kmem_cache_free+0xb7/0x2f0
      <4>[  473.116205]  __i915_gem_free_objects+0x7d4/0xe10 [i915]
      <4>[  473.116311]  i915_gem_create_ioctl+0xaa/0xd0 [i915]
      <4>[  473.116318]  drm_ioctl_kernel+0x192/0x260
      <4>[  473.116323]  drm_ioctl+0x67c/0x960
      <4>[  473.116330]  do_vfs_ioctl+0x18d/0xfa0
      <4>[  473.116335]  ksys_ioctl+0x35/0x70
      <4>[  473.116341]  __x64_sys_ioctl+0x6a/0xb0
      <4>[  473.116347]  do_syscall_64+0x97/0x400
      <4>[  473.116354]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
      
      Testcase: igt/gem_flink_race/flink_close
      Fixes: e163484a ("drm/i915: Update size upon return from GEM_CREATE")
      Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk>
      Cc: Michał Winiarski <michal.winiarski@intel.com>
      Reviewed-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190417132507.27133-1-chris@chris-wilson.co.uk
      (cherry picked from commit 99534023)
      Signed-off-by: NJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
      929eec99
  2. 11 4月, 2019 1 次提交
  3. 08 4月, 2019 1 次提交
  4. 04 4月, 2019 1 次提交
  5. 02 4月, 2019 1 次提交
    • C
      drm/i915: Prefault before locking pages in shmem_pwrite · b01720bf
      Chris Wilson 提交于
      If the user passes in a pointer to a GGTT mmaping of the same buffer
      being written to, we can hit a deadlock in acquiring the shmemfs page
      (once as the write destination and then as the read source).
      
      [<0>] io_schedule+0xd/0x30
      [<0>] __lock_page+0x105/0x1b0
      [<0>] find_lock_entry+0x55/0x90
      [<0>] shmem_getpage_gfp+0xbb/0x800
      [<0>] shmem_read_mapping_page_gfp+0x2d/0x50
      [<0>] shmem_get_pages+0x158/0x5d0 [i915]
      [<0>] ____i915_gem_object_get_pages+0x17/0x90 [i915]
      [<0>] __i915_gem_object_get_pages+0x57/0x70 [i915]
      [<0>] i915_gem_fault+0x1b4/0x5c0 [i915]
      [<0>] __do_fault+0x2d/0x80
      [<0>] __handle_mm_fault+0xad4/0xfb0
      [<0>] handle_mm_fault+0xe6/0x1f0
      [<0>] __do_page_fault+0x18f/0x3f0
      [<0>] page_fault+0x1b/0x20
      [<0>] copy_user_enhanced_fast_string+0x7/0x10
      [<0>] _copy_from_user+0x37/0x60
      [<0>] shmem_pwrite+0xf0/0x160 [i915]
      [<0>] i915_gem_pwrite_ioctl+0x14e/0x520 [i915]
      [<0>] drm_ioctl_kernel+0x81/0xd0
      [<0>] drm_ioctl+0x1a7/0x310
      [<0>] do_vfs_ioctl+0x88/0x5d0
      [<0>] ksys_ioctl+0x35/0x70
      [<0>] __x64_sys_ioctl+0x11/0x20
      [<0>] do_syscall_64+0x39/0xe0
      [<0>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
      
      We can reduce (but not eliminate!) the chance of this happening by
      faulting the user_data before we take the page lock in
      pagecache_write_begin(). One way to eliminate the potential recursion
      here is by disabling pagefaults for the copy, and handling the fallback
      to use an alternative method -- so convert to use kmap_atomic (which
      should disable preemption and pagefaulting for the copy) and report
      ENODEV instead of EFAULT so that our caller tries again with a different
      copy mechanism -- we already check that the page should have been
      faultable so a false negative should be rare.
      
      Testcase: igt/gem_pwrite/self
      Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk>
      Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
      Cc: Matthew Auld <matthew.william.auld@gmail.com>
      Reviewed-by: NMatthew Auld <matthew.william.auld@gmail.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190401133909.31203-1-chris@chris-wilson.co.uk
      b01720bf
  6. 31 3月, 2019 1 次提交
  7. 27 3月, 2019 1 次提交
  8. 25 3月, 2019 1 次提交
  9. 22 3月, 2019 3 次提交
    • S
      drm/i915/guc: GuC suspend path cleanup · b9d52d38
      Sujaritha Sundaresan 提交于
      Adding a call to intel_uc_suspend in i915_gem_suspend, which
      is a common point for the suspend/resume and hibernate paths.
      This fixes an unbalanced call that causes issues with the CTB
      register/deregister.
      
      v2: Making the call unconditional (Daniele)
      	Moving the call to after the GEM_BUG_ON (Chris)
      
      Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
      Cc: Chris Wilson <chris@chris-wilson.co.uk>
      Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
      Signed-off-by: NSujaritha Sundaresan <sujaritha.sundaresan@intel.com>
      Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk>
      Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190321203804.6845-1-sujaritha.sundaresan@intel.com
      b9d52d38
    • C
      drm/i915: Skip object locking around a no-op set-domain ioctl · 754a2544
      Chris Wilson 提交于
      If we are already in the desired write domain of a set-domain ioctl,
      then there is nothing for us to do and we can quickly return back to
      userspace, avoiding any lock contention. By recognising that the
      write_domain is always a subset of the read_domains, and excluding the
      no-op case of requiring 0 read_domains in the ioctl, we can infer if the
      current write_domain matches the target read_domains, there is nothing
      for us to do.
      
      Secondary aspect of this is that we undo the arbitrary fetching and
      potential flushing of all pages for a set-domain(.write=CPU) call on a
      fresh object -- which was introduced simply because we do the get-pages
      before taking the struct_mutex.
      
      References: 40e62d5d ("drm/i915: Acquire the backing storage outside of struct_mutex in set-domain")
      Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk>
      Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
      Cc: Matthew Auld <matthew.william.auld@gmail.com>
      Reviewed-by: NMatthew Auld <matthew.william.auld@gmail.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190321161908.8007-2-chris@chris-wilson.co.uk
      754a2544
    • C
      drm/i915: Flush pages on acquisition · a679f58d
      Chris Wilson 提交于
      When we return pages to the system, we ensure that they are marked as
      being in the CPU domain since any external access is uncontrolled and we
      must assume the worst. This means that we need to always flush the pages
      on acquisition if we need to use them on the GPU, and from the beginning
      have used set-domain. Set-domain is overkill for the purpose as it is a
      general synchronisation barrier, but our intent is to only flush the
      pages being swapped in. If we move that flush into the pages acquisition
      phase, we know then that when we have obj->mm.pages, they are coherent
      with the GPU and need only maintain that status without resorting to
      heavy handed use of set-domain.
      
      The principle knock-on effect for userspace is through mmap-gtt
      pagefaulting. Our uAPI has always implied that the GTT mmap was async
      (especially as when any pagefault occurs is unpredicatable to userspace)
      and so userspace had to apply explicit domain control itself
      (set-domain). However, swapping is transparent to the kernel, and so on
      first fault we need to acquire the pages and make them coherent for
      access through the GTT. Our use of set-domain here leaks into the uABI
      that the first pagefault was synchronous. This is unintentional and
      baring a few igt should be unoticed, nevertheless we bump the uABI
      version for mmap-gtt to reflect the change in behaviour.
      
      Another implication of the change is that gem_create() is presumed to
      create an object that is coherent with the CPU and is in the CPU write
      domain, so a set-domain(CPU) following a gem_create() would be a minor
      operation that merely checked whether we could allocate all pages for
      the object. On applying this change, a set-domain(CPU) causes a clflush
      as we acquire the pages. This will have a small impact on mesa as we move
      the clflush here on !llc from execbuf time to create, but that should
      have minimal performance impact as the same clflush exists but is now
      done early and because of the clflush issue, userspace recycles bo and
      so should resist allocating fresh objects.
      
      Internally, the presumption that objects are created in the CPU
      write-domain and remain so through writes to obj->mm.mapping is more
      prevalent than I expected; but easy enough to catch and apply a manual
      flush.
      
      For the future, we should push the page flush from the central
      set_pages() into the callers so that we can more finely control when it
      is applied, but for now doing it one location is easier to validate, at
      the cost of sometimes flushing when there is no need.
      Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk>
      Cc: Matthew Auld <matthew.william.auld@gmail.com>
      Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
      Cc: Antonio Argenziano <antonio.argenziano@intel.com>
      Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
      Reviewed-by: NMatthew Auld <matthew.william.auld@gmail.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190321161908.8007-1-chris@chris-wilson.co.uk
      a679f58d
  10. 21 3月, 2019 2 次提交
  11. 19 3月, 2019 1 次提交
  12. 18 3月, 2019 1 次提交
  13. 09 3月, 2019 2 次提交
  14. 08 3月, 2019 8 次提交
  15. 06 3月, 2019 3 次提交
  16. 28 2月, 2019 4 次提交
  17. 22 2月, 2019 1 次提交
  18. 21 2月, 2019 2 次提交
  19. 13 2月, 2019 2 次提交
  20. 12 2月, 2019 1 次提交
  21. 09 2月, 2019 1 次提交
    • C
      drm/i915: Revoke mmaps and prevent access to fence registers across reset · 2caffbf1
      Chris Wilson 提交于
      Previously, we were able to rely on the recursive properties of
      struct_mutex to allow us to serialise revoking mmaps and reacquiring the
      FENCE registers with them being clobbered over a global device reset.
      I then proceeded to throw out the baby with the bath water in order to
      pursue a struct_mutex-less reset.
      
      Perusing LWN for alternative strategies, the dilemma on how to serialise
      access to a global resource on one side was answered by
      https://lwn.net/Articles/202847/ -- Sleepable RCU:
      
          1  int readside(void) {
          2      int idx;
          3      rcu_read_lock();
          4	   if (nomoresrcu) {
          5          rcu_read_unlock();
          6	       return -EINVAL;
          7      }
          8	   idx = srcu_read_lock(&ss);
          9	   rcu_read_unlock();
          10	   /* SRCU read-side critical section. */
          11	   srcu_read_unlock(&ss, idx);
          12	   return 0;
          13 }
          14
          15 void cleanup(void)
          16 {
          17     nomoresrcu = 1;
          18     synchronize_rcu();
          19     synchronize_srcu(&ss);
          20     cleanup_srcu_struct(&ss);
          21 }
      
      No more worrying about stop_machine, just an uber-complex mutex,
      optimised for reads, with the overhead pushed to the rare reset path.
      
      However, we do run the risk of a deadlock as we allocate underneath the
      SRCU read lock, and the allocation may require a GPU reset, causing a
      dependency cycle via the in-flight requests. We resolve that by declaring
      the driver wedged and cancelling all in-flight rendering.
      
      v2: Use expedited rcu barriers to match our earlier timing
      characteristics.
      v3: Try to annotate locking contexts for sparse
      v4: Reduce selftest lock duration to avoid a reset deadlock with fences
      v5: s/srcu/reset_backoff_srcu/
      v6: Remove more stale comments
      
      Testcase: igt/gem_mmap_gtt/hang
      Fixes: eb8d0f5a ("drm/i915: Remove GPU reset dependence on struct_mutex")
      Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk>
      Cc: Mika Kuoppala <mika.kuoppala@intel.com>
      Reviewed-by: NMika Kuoppala <mika.kuoppala@linux.intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190208153708.20023-2-chris@chris-wilson.co.uk
      2caffbf1
  22. 07 2月, 2019 1 次提交