1. 26 Jul, 2021 3 commits
    • Zheyu Ma's avatar
      video: fbdev: kyro: Error out if 'pixclock' equals zero · 1520b4b7
      Zheyu Ma authored
      The userspace program could pass any values to the driver through
      ioctl() interface. if the driver doesn't check the value of 'pixclock',
      it may cause divide error because the value of 'lineclock' and
      'frameclock' will be zero.
      
      Fix this by checking whether 'pixclock' is zero in kyrofb_check_var().
      
      The following log reveals it:
      
      [  103.073930] divide error: 0000 [#1] PREEMPT SMP KASAN PTI
      [  103.073942] CPU: 4 PID: 12483 Comm: syz-executor Not tainted 5.14.0-rc2-00478-g2734d6c1-dirty #118
      [  103.073959] RIP: 0010:kyrofb_set_par+0x316/0xc80
      [  103.074045] Call Trace:
      [  103.074048]  ? ___might_sleep+0x1ee/0x2d0
      [  103.074060]  ? kyrofb_ioctl+0x330/0x330
      [  103.074069]  fb_set_var+0x5bf/0xeb0
      [  103.074078]  ? fb_blank+0x1a0/0x1a0
      [  103.074085]  ? lock_acquire+0x3bd/0x530
      [  103.074094]  ? lock_release+0x810/0x810
      [  103.074103]  ? ___might_sleep+0x1ee/0x2d0
      [  103.074114]  ? __mutex_lock+0x620/0x1190
      [  103.074126]  ? trace_hardirqs_on+0x6a/0x1c0
      [  103.074137]  do_fb_ioctl+0x31e/0x700
      [  103.074144]  ? fb_getput_cmap+0x280/0x280
      [  103.074152]  ? rcu_read_lock_sched_held+0x11/0x80
      [  103.074162]  ? rcu_read_lock_sched_held+0x11/0x80
      [  103.074171]  ? __sanitizer_cov_trace_switch+0x67/0xf0
      [  103.074181]  ? __sanitizer_cov_trace_const_cmp2+0x20/0x80
      [  103.074191]  ? do_vfs_ioctl+0x14b/0x16c0
      [  103.074199]  ? vfs_fileattr_set+0xb60/0xb60
      [  103.074207]  ? rcu_read_lock_sched_held+0x11/0x80
      [  103.074216]  ? lock_release+0x483/0x810
      [  103.074224]  ? __fget_files+0x217/0x3d0
      [  103.074234]  ? __fget_files+0x239/0x3d0
      [  103.074243]  ? do_fb_ioctl+0x700/0x700
      [  103.074250]  fb_ioctl+0xe6/0x130
      Signed-off-by: default avatarZheyu Ma <zheyuma97@gmail.com>
      Signed-off-by: default avatarSam Ravnborg <sam@ravnborg.org>
      Link: https://patchwork.freedesktop.org/patch/msgid/1627293835-17441-3-git-send-email-zheyuma97@gmail.com
      1520b4b7
    • Zheyu Ma's avatar
      video: fbdev: asiliantfb: Error out if 'pixclock' equals zero · b36b242d
      Zheyu Ma authored
      The userspace program could pass any values to the driver through
      ioctl() interface. If the driver doesn't check the value of 'pixclock',
      it may cause divide error.
      
      Fix this by checking whether 'pixclock' is zero first.
      
      The following log reveals it:
      
      [   43.861711] divide error: 0000 [#1] PREEMPT SMP KASAN PTI
      [   43.861737] CPU: 2 PID: 11764 Comm: i740 Not tainted 5.14.0-rc2-00513-gac532c9bbcfb-dirty #224
      [   43.861756] RIP: 0010:asiliantfb_check_var+0x4e/0x730
      [   43.861843] Call Trace:
      [   43.861848]  ? asiliantfb_remove+0x190/0x190
      [   43.861858]  fb_set_var+0x2e4/0xeb0
      [   43.861866]  ? fb_blank+0x1a0/0x1a0
      [   43.861873]  ? lock_acquire+0x1ef/0x530
      [   43.861884]  ? lock_release+0x810/0x810
      [   43.861892]  ? lock_is_held_type+0x100/0x140
      [   43.861903]  ? ___might_sleep+0x1ee/0x2d0
      [   43.861914]  ? __mutex_lock+0x620/0x1190
      [   43.861921]  ? do_fb_ioctl+0x313/0x700
      [   43.861929]  ? mutex_lock_io_nested+0xfa0/0xfa0
      [   43.861936]  ? __this_cpu_preempt_check+0x1d/0x30
      [   43.861944]  ? _raw_spin_unlock_irqrestore+0x46/0x60
      [   43.861952]  ? lockdep_hardirqs_on+0x59/0x100
      [   43.861959]  ? _raw_spin_unlock_irqrestore+0x46/0x60
      [   43.861967]  ? trace_hardirqs_on+0x6a/0x1c0
      [   43.861978]  do_fb_ioctl+0x31e/0x700
      Signed-off-by: default avatarZheyu Ma <zheyuma97@gmail.com>
      Signed-off-by: default avatarSam Ravnborg <sam@ravnborg.org>
      Link: https://patchwork.freedesktop.org/patch/msgid/1627293835-17441-2-git-send-email-zheyuma97@gmail.com
      b36b242d
    • Simon Ser's avatar
      drm: document drm_property_enum.value for bitfields · 3012248f
      Simon Ser authored
      When a property has the type DRM_MODE_PROP_BITMASK, the value field
      stores a bitshift, not a bitmask, which can be surprising.
      Signed-off-by: default avatarSimon Ser <contact@emersion.fr>
      Cc: Leandro Ribeiro <leandro.ribeiro@collabora.com>
      Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Acked-by: default avatarPekka Paalanen <pekka.paalanen@collabora.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/NUZTPTKKZtAlDhxIXFB1qrUqWBYKapkBxCnb1S1bc3g@cp3-web-033.plabs.ch
      3012248f
  2. 25 Jul, 2021 9 commits
  3. 23 Jul, 2021 11 commits
  4. 22 Jul, 2021 1 commit
  5. 21 Jul, 2021 11 commits
  6. 20 Jul, 2021 5 commits
    • Juan A. Suarez Romero's avatar
      drm/v3d: Expose performance counters to userspace · 26a4dc29
      Juan A. Suarez Romero authored
      The V3D engine has several hardware performance counters that can of
      interest for userspace performance analysis tools.
      
      This exposes new ioctls to create and destroy performance monitor
      objects, as well as to query the counter values.
      
      Each created performance monitor object has an ID that can be attached
      to CL/CSD submissions, so the driver enables the requested counters when
      the job is submitted, and updates the performance monitor values when
      the job is done.
      
      It is up to the user to ensure all the jobs have been finished before
      getting the performance monitor values. It is also up to the user to
      properly synchronize BCL jobs when submitting jobs with different
      performance monitors attached.
      
      Cc: Daniel Vetter <daniel@ffwll.ch>
      Cc: David Airlie <airlied@linux.ie>
      Cc: Emma Anholt <emma@anholt.net>
      To: dri-devel@lists.freedesktop.org
      Signed-off-by: default avatarJuan A. Suarez Romero <jasuarez@igalia.com>
      Acked-by: default avatarMelissa Wen <mwen@igalia.com>
      Signed-off-by: default avatarMelissa Wen <melissa.srw@gmail.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210608111541.461991-1-jasuarez@igalia.com
      26a4dc29
    • Desmond Cheong Zhi Xi's avatar
      drm: protect drm_master pointers in drm_lease.c · 56f0729a
      Desmond Cheong Zhi Xi authored
      drm_file->master pointers should be protected by
      drm_device.master_mutex or drm_file.master_lookup_lock when being
      dereferenced.
      
      However, in drm_lease.c, there are multiple instances where
      drm_file->master is accessed and dereferenced while neither lock is
      held. This makes drm_lease.c vulnerable to use-after-free bugs.
      
      We address this issue in 2 ways:
      
      1. Add a new drm_file_get_master() function that calls drm_master_get
      on drm_file->master while holding on to
      drm_file.master_lookup_lock. Since drm_master_get increments the
      reference count of master, this prevents master from being freed until
      we unreference it with drm_master_put.
      
      2. In each case where drm_file->master is directly accessed and
      eventually dereferenced in drm_lease.c, we wrap the access in a call
      to the new drm_file_get_master function, then unreference the master
      pointer once we are done using it.
      Reported-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDesmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
      Reviewed-by: default avatarEmil Velikov <emil.l.velikov@gmail.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210712043508.11584-6-desmondcheongzx@gmail.com
      56f0729a
    • Desmond Cheong Zhi Xi's avatar
      drm: serialize drm_file.master with a new spinlock · 0b0860a3
      Desmond Cheong Zhi Xi authored
      Currently, drm_file.master pointers should be protected by
      drm_device.master_mutex when being dereferenced. This is because
      drm_file.master is not invariant for the lifetime of drm_file. If
      drm_file is not the creator of master, then drm_file.is_master is
      false, and a call to drm_setmaster_ioctl will invoke
      drm_new_set_master, which then allocates a new master for drm_file and
      puts the old master.
      
      Thus, without holding drm_device.master_mutex, the old value of
      drm_file.master could be freed while it is being used by another
      concurrent process.
      
      However, it is not always possible to lock drm_device.master_mutex to
      dereference drm_file.master. Through the fbdev emulation code, this
      might occur in a deep nest of other locks. But drm_device.master_mutex
      is also the outermost lock in the nesting hierarchy, so this leads to
      potential deadlocks.
      
      To address this, we introduce a new spin lock at the bottom of the
      lock hierarchy that only serializes drm_file.master. With this change,
      the value of drm_file.master changes only when both
      drm_device.master_mutex and drm_file.master_lookup_lock are
      held. Hence, any process holding either of those locks can ensure that
      the value of drm_file.master will not change concurrently.
      
      Since no lock depends on the new drm_file.master_lookup_lock, when
      drm_file.master is dereferenced, but drm_device.master_mutex cannot be
      held, we can safely protect the master pointer with
      drm_file.master_lookup_lock.
      Reported-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDesmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210712043508.11584-5-desmondcheongzx@gmail.com
      0b0860a3
    • Desmond Cheong Zhi Xi's avatar
      drm: add a locked version of drm_is_current_master · 1f7ef07c
      Desmond Cheong Zhi Xi authored
      While checking the master status of the DRM file in
      drm_is_current_master(), the device's master mutex should be
      held. Without the mutex, the pointer fpriv->master may be freed
      concurrently by another process calling drm_setmaster_ioctl(). This
      could lead to use-after-free errors when the pointer is subsequently
      dereferenced in drm_lease_owner().
      
      The callers of drm_is_current_master() from drm_auth.c hold the
      device's master mutex, but external callers do not. Hence, we implement
      drm_is_current_master_locked() to be used within drm_auth.c, and
      modify drm_is_current_master() to grab the device's master mutex
      before checking the master status.
      Reported-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Signed-off-by: default avatarDesmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
      Reviewed-by: default avatarEmil Velikov <emil.l.velikov@gmail.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210712043508.11584-4-desmondcheongzx@gmail.com
      1f7ef07c
    • Desmond Cheong Zhi Xi's avatar
      drm: avoid blocking in drm_clients_info's rcu section · 5eff9585
      Desmond Cheong Zhi Xi authored
      Inside drm_clients_info, the rcu_read_lock is held to lock
      pid_task()->comm. However, within this protected section, a call to
      drm_is_current_master is made, which involves a mutex lock in a future
      patch. However, this is illegal because the mutex lock might block
      while in the RCU read-side critical section.
      
      Since drm_is_current_master isn't protected by rcu_read_lock, we avoid
      this by moving it out of the RCU critical section.
      
      The following report came from intel-gfx ci's
      igt@debugfs_test@read_all_entries testcase:
      
      =============================
      [ BUG: Invalid wait context ]
      5.13.0-CI-Patchwork_20515+ #1 Tainted: G        W
      -----------------------------
      debugfs_test/1101 is trying to lock:
      ffff888132d901a8 (&dev->master_mutex){+.+.}-{3:3}, at:
      drm_is_current_master+0x1e/0x50
      other info that might help us debug this:
      context-{4:4}
      3 locks held by debugfs_test/1101:
       #0: ffff88810fdffc90 (&p->lock){+.+.}-{3:3}, at:
       seq_read_iter+0x53/0x3b0
       #1: ffff888132d90240 (&dev->filelist_mutex){+.+.}-{3:3}, at:
       drm_clients_info+0x63/0x2a0
       #2: ffffffff82734220 (rcu_read_lock){....}-{1:2}, at:
       drm_clients_info+0x1b1/0x2a0
      stack backtrace:
      CPU: 8 PID: 1101 Comm: debugfs_test Tainted: G        W
      5.13.0-CI-Patchwork_20515+ #1
      Hardware name: Intel Corporation CometLake Client Platform/CometLake S
      UDIMM (ERB/CRB), BIOS CMLSFWR1.R00.1263.D00.1906260926 06/26/2019
      Call Trace:
       dump_stack+0x7f/0xad
       __lock_acquire.cold.78+0x2af/0x2ca
       lock_acquire+0xd3/0x300
       ? drm_is_current_master+0x1e/0x50
       ? __mutex_lock+0x76/0x970
       ? lockdep_hardirqs_on+0xbf/0x130
       __mutex_lock+0xab/0x970
       ? drm_is_current_master+0x1e/0x50
       ? drm_is_current_master+0x1e/0x50
       ? drm_is_current_master+0x1e/0x50
       drm_is_current_master+0x1e/0x50
       drm_clients_info+0x107/0x2a0
       seq_read_iter+0x178/0x3b0
       seq_read+0x104/0x150
       full_proxy_read+0x4e/0x80
       vfs_read+0xa5/0x1b0
       ksys_read+0x5a/0xd0
       do_syscall_64+0x39/0xb0
       entry_SYSCALL_64_after_hwframe+0x44/0xae
      Signed-off-by: default avatarDesmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
      Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20210712043508.11584-3-desmondcheongzx@gmail.com
      5eff9585