[PATCH] drm: Hold idr_mutex for _drm_lease_revoke

[Chris Wilson]

Quoting Chris Wilson (2017-10-19 11:53:44)
> _drm_lease_revoke() requires it callers to hold the idr_mutex, but its
> only caller did not. Every device release would then trigger:
> 
>  WARNING: CPU: 7 PID: 4169 at drivers/gpu/drm/drm_lease.c:313 _drm_lease_revoke+0x12c/0x140
>  Modules linked in: vgem snd_hda_codec_hdmi snd_hda_codec_generic i915 x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul snd_hda_intel ghash_clmulni_intel snd_hda_codec r8169 snd_hwdep mii snd_hda_core snd_pcm mei_me mei prime_numbers i2c_hid pinctrl_sunrisepoint pinctrl_intel
>  CPU: 7 PID: 4169 Comm: pm_backlight Tainted: G     U  W       4.14.0-rc5-CI-CI_DRM_3262+ #1
>  Hardware name: TOSHIBA SATELLITE P50-C/06F4                            , BIOS 1.40 03/29/2016
>  task: ffff8801f5a2a880 task.stack: ffffc900007e4000
>  RIP: 0010:_drm_lease_revoke+0x12c/0x140
>  RSP: 0018:ffffc900007e7da8 EFLAGS: 00010246
>  RAX: 0000000000000000 RBX: ffff8801decdafd8 RCX: 0000000000000001
>  RDX: 0000000000000000 RSI: 00000000ffffffff RDI: ffff88026afc05d0
>  RBP: ffffc900007e7dd0 R08: ffff8801f5a2b168 R09: 0000000000000000
>  R10: ffffc900007e7dd0 R11: 0000000000000001 R12: ffff88026afc0000
>  R13: ffff8802730609f8 R14: ffff8802730609f8 R15: dead000000000100
>  FS:  00007f9848442a40(0000) GS:ffff880281dc0000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 00007f9459c9ff80 CR3: 00000001df36e003 CR4: 00000000003606e0
>  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>  DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>  Call Trace:
>   drm_master_release+0xa5/0x120
>   drm_release+0x345/0x3c0
>   __fput+0xb9/0x200
>   ____fput+0xe/0x10
>   task_work_run+0x89/0xc0
>   exit_to_usermode_loop+0x83/0x90
>   syscall_return_slowpath+0xd0/0x110
>   entry_SYSCALL_64_fastpath+0xaf/0xb1
>  RIP: 0033:0x7f984691b730
>  RSP: 002b:00007fffdf0092e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000003
>  RAX: 0000000000000000 RBX: 00007fffdf0093a0 RCX: 00007f984691b730
>  RDX: 00007fffdf0092d0 RSI: 0000000040086409 RDI: 0000000000000003
>  RBP: 0000000000000000 R08: 0000557b2a88c7c0 R09: 0000000000000001
>  R10: 0000000000000069 R11: 0000000000000246 R12: 0000000000000000
>  R13: 0000000000000002 R14: 0000000000000001 R15: 0000557b2a88c4e0
>  Code: 00 00 49 81 ec f8 00 00 00 e9 20 ff ff ff 48 8b 47 08 be ff ff ff ff 48 8d b8 d0 05 00 00 e8 cc 07 ad ff 85 c0 0f 85 f9 fe ff ff <0f> ff e9 f2 fe ff ff 90 90 90 90 90 90 90 90 90 90 90 90 90 0f

Insufficient, as we now get

[  214.187528] INFO: trying to register non-static key.
[  214.192480] the code is fine but needs lockdep annotation.
[  214.197942] turning off the locking correctness validator.
[  214.203407] CPU: 1 PID: 3139 Comm: gem_ringfill Tainted: G     U          4.14.0-rc5-CI-Trybot_1284+ #1
[  214.212768] Hardware name: Dell Inc. OptiPlex 755                 /0PU052, BIOS A08 02/19/2008
[  214.221347] Call Trace:
[  214.223785]  dump_stack+0x68/0x9f
[  214.227087]  register_lock_class+0x3fd/0x580
[  214.231339]  ? __lock_acquire+0x4ab/0x1b00
[  214.235419]  ? __lock_acquire+0x4ab/0x1b00
[  214.239499]  __lock_acquire+0xa4/0x1b00
[  214.243320]  ? drm_lease_revoke+0x29/0x160
[  214.247401]  ? __this_cpu_preempt_check+0x13/0x20
[  214.252087]  ? trace_hardirqs_on_caller+0xe3/0x1b0
[  214.256857]  lock_acquire+0xb0/0x200
[  214.260416]  ? lock_acquire+0xb0/0x200
[  214.264148]  ? drm_lease_revoke+0x29/0x160
[  214.268229]  __mutex_lock+0x86/0x9b0
[  214.271787]  ? drm_lease_revoke+0x29/0x160
[  214.275867]  ? drm_lease_revoke+0x29/0x160
[  214.279947]  ? __mutex_lock+0x437/0x9b0
[  214.283767]  ? __call_rcu.constprop.51+0x122/0x260
[  214.288538]  ? drm_master_release+0x33/0x120
[  214.292789]  ? __this_cpu_preempt_check+0x13/0x20
[  214.297475]  ? trace_hardirqs_on_caller+0xe3/0x1b0
[  214.302245]  ? trace_hardirqs_on+0xd/0x10
[  214.306239]  mutex_lock_nested+0x1b/0x20
[  214.310145]  ? mutex_lock_nested+0x1b/0x20
[  214.314226]  drm_lease_revoke+0x29/0x160
[  214.318132]  drm_master_release+0xa5/0x120
[  214.322212]  drm_release+0x345/0x3c0
[  214.325772]  __fput+0xb9/0x200
[  214.328811]  ____fput+0xe/0x10
[  214.331853]  task_work_run+0x89/0xc0
[  214.335413]  exit_to_usermode_loop+0x83/0x90
[  214.339664]  syscall_return_slowpath+0xd0/0x110
[  214.344177]  entry_SYSCALL_64_fastpath+0xaf/0xb1
[  214.348776] RIP: 0033:0x7f5239e75730
[  214.352334] RSP: 002b:00007fff06432538 EFLAGS: 00000246 ORIG_RAX: 0000000000000003
[  214.359876] RAX: 0000000000000000 RBX: 0000000000000cbe RCX: 00007f5239e75730
[  214.366986] RDX: 00007fff06432510 RSI: 0000000040086442 RDI: 0000000000000005
[  214.374094] RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000000
[  214.381204] R10: 0000000000000008 R11: 0000000000000246 R12: 0000000000000cbe
[  214.388312] R13: 0000000000000005 R14: 0000000000000001 R15: 0000000000000000

i.e. not every drm device has mode_config. Something like
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 4c14b2cbc733..c40e603e0559 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -285,7 +285,8 @@ void drm_master_release(struct drm_file *file_priv)
        if (dev->master == file_priv->master)
                drm_drop_master(dev, file_priv);
 out:
-       if (file_priv->is_master) {
+       if (drm_core_check_feature(dev, DRIVER_MODESET) &&
+           file_priv->is_master) {
                /* Revoke any leases held by this or lessees, but only if
                 * this is the "real" master
                 */
?
-Chris