Re: [PATCH]] drm/dp check aux_dev before use in drm_dp_aux_dev_get_by_minor()
On Mon, 7 Sep 2020, Ville Syrjälä wrote: > On Fri, Sep 04, 2020 at 12:21:26AM -0700, Zwane Mwaikambo wrote: > > I observed this when unplugging a DP monitor whilst a computer is asleep > > and then waking it up. This left DP chardev nodes still being present on > > the filesystem and accessing these device nodes caused an oops because > > drm_dp_aux_dev_get_by_minor() assumes a device exists if it is opened. > > This can also be reproduced by creating a device node with mknod(1) and > > issuing an open(2) > > > > [166164.933198] BUG: kernel NULL pointer dereference, address: > > 0018 > > [166164.933202] #PF: supervisor read access in kernel mode > > [166164.933204] #PF: error_code(0x) - not-present page > > [166164.933205] PGD 0 P4D 0 > > [166164.933208] Oops: [#1] PREEMPT SMP NOPTI > > [166164.933211] CPU: 4 PID: 99071 Comm: fwupd Tainted: GW > > 5.8.0-rc6+ #1 > > [166164.933213] Hardware name: LENOVO 20RD002VUS/20RD002VUS, BIOS R16ET25W > > (1.11 ) 04/21/2020 > > [166164.933232] RIP: 0010:drm_dp_aux_dev_get_by_minor+0x29/0x70 > > [drm_kms_helper] > > [166164.933234] Code: 00 0f 1f 44 00 00 55 48 89 e5 41 54 41 89 fc 48 c7 > > c7 60 01 a4 c0 e8 26 ab 30 d7 44 89 e6 48 c7 c7 80 01 a4 c0 e8 47 94 d6 d6 > > <8b> 50 18 49 89 c4 48 8d 78 18 85 d2 74 33 8d 4a 01 89 d0 f0 0f b1 > > [166164.933236] RSP: 0018:b7d7c41cbbf0 EFLAGS: 00010246 > > [166164.933237] RAX: RBX: 8a90001fe900 RCX: > > > > [166164.933238] RDX: RSI: 0003 RDI: > > c0a40180 > > [166164.933239] RBP: b7d7c41cbbf8 R08: R09: > > 8a93e157d6d0 > > [166164.933240] R10: R11: c0a40188 R12: > > 0003 > > [166164.933241] R13: 8a9402200e80 R14: 8a90001fe900 R15: > > > > [166164.933244] FS: 7f7fb041eb00() GS:8a941150() > > knlGS: > > [166164.933245] CS: 0010 DS: ES: CR0: 80050033 > > [166164.933246] CR2: 0018 CR3: 352c2003 CR4: > > 003606e0 > > [166164.933247] Call Trace: > > [166164.933264] auxdev_open+0x1b/0x40 [drm_kms_helper] > > [166164.933278] chrdev_open+0xa7/0x1c0 > > [166164.933282] ? cdev_put.part.0+0x20/0x20 > > [166164.933287] do_dentry_open+0x161/0x3c0 > > [166164.933291] vfs_open+0x2d/0x30 > > [166164.933297] path_openat+0xb27/0x10e0 > > [166164.933306] ? atime_needs_update+0x73/0xd0 > > [166164.933309] do_filp_open+0x91/0x100 > > [166164.933313] ? __alloc_fd+0xb2/0x150 > > [166164.933316] do_sys_openat2+0x210/0x2d0 > > [166164.933318] do_sys_open+0x46/0x80 > > [166164.933320] __x64_sys_openat+0x20/0x30 > > [166164.933328] do_syscall_64+0x52/0xc0 > > [166164.96] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > > > > > (gdb) disassemble drm_dp_aux_dev_get_by_minor+0x29 > > Dump of assembler code for function drm_dp_aux_dev_get_by_minor: > >0x00017b10 <+0>: callq 0x17b15 > > > >0x00017b15 <+5>: push %rbp > >0x00017b16 <+6>: mov%rsp,%rbp > >0x00017b19 <+9>: push %r12 > >0x00017b1b <+11>:mov%edi,%r12d > >0x00017b1e <+14>:mov$0x0,%rdi > >0x00017b25 <+21>:callq 0x17b2a > > > >0x00017b2a <+26>:mov%r12d,%esi > >0x00017b2d <+29>:mov$0x0,%rdi > >0x00017b34 <+36>:callq 0x17b39 > > > >0x00017b39 <+41>:mov0x18(%rax),%edx <= > >0x00017b3c <+44>:mov%rax,%r12 > >0x00017b3f <+47>:lea0x18(%rax),%rdi > >0x00017b43 <+51>:test %edx,%edx > >0x00017b45 <+53>:je 0x17b7a > > > >0x00017b47 <+55>:lea0x1(%rdx),%ecx > >0x00017b4a <+58>:mov%edx,%eax > >0x00017b4c <+60>:lock cmpxchg %ecx,(%rdi) > >0x00017b50 <+64>:jne0x17b76 > > > >0x00017b52 <+66>:test %edx,%edx > >0x00017b54 <+68>:js 0x17b6d > > > >0x00017b56 <+70>:test %ecx,%ecx > >0x00017b58 <+72>:js 0x17b6d > > > >0x00017b5a <+74>:mov$0x0,%rdi > >0x00017b61 <+81>:callq 0x17b66 > > > >0x00017b66 <+86>:mov%r12,%rax > >0x00017b69 <+89>:pop%r12 > >0x00017b6b <+91>:pop%rbp > >0x00017b6c <+92>:retq > >0x00017b6d <+93>:xor%esi,%esi > >0x00017b6f <+95>:callq 0x17b74 > > > >0x00017b74 <+100>: jmp0x17b5a > > > >0x00017b76 <+102>: mov%eax,%edx > >0x00017b78 <+104>: jmp0x17b43 > > > >0x00017b7a <+106>: xor%r12d,%r12d > >0x00017b7d <+109>: jmp0x17b5a > > > > End of assembler dump. > > > > (gdb) list
Re: [PATCH]] drm/dp check aux_dev before use in drm_dp_aux_dev_get_by_minor()
On Fri, Sep 04, 2020 at 12:21:26AM -0700, Zwane Mwaikambo wrote:
> I observed this when unplugging a DP monitor whilst a computer is asleep
> and then waking it up. This left DP chardev nodes still being present on
> the filesystem and accessing these device nodes caused an oops because
> drm_dp_aux_dev_get_by_minor() assumes a device exists if it is opened.
> This can also be reproduced by creating a device node with mknod(1) and
> issuing an open(2)
>
> [166164.933198] BUG: kernel NULL pointer dereference, address:
> 0018
> [166164.933202] #PF: supervisor read access in kernel mode
> [166164.933204] #PF: error_code(0x) - not-present page
> [166164.933205] PGD 0 P4D 0
> [166164.933208] Oops: [#1] PREEMPT SMP NOPTI
> [166164.933211] CPU: 4 PID: 99071 Comm: fwupd Tainted: GW
> 5.8.0-rc6+ #1
> [166164.933213] Hardware name: LENOVO 20RD002VUS/20RD002VUS, BIOS R16ET25W
> (1.11 ) 04/21/2020
> [166164.933232] RIP: 0010:drm_dp_aux_dev_get_by_minor+0x29/0x70
> [drm_kms_helper]
> [166164.933234] Code: 00 0f 1f 44 00 00 55 48 89 e5 41 54 41 89 fc 48 c7
> c7 60 01 a4 c0 e8 26 ab 30 d7 44 89 e6 48 c7 c7 80 01 a4 c0 e8 47 94 d6 d6
> <8b> 50 18 49 89 c4 48 8d 78 18 85 d2 74 33 8d 4a 01 89 d0 f0 0f b1
> [166164.933236] RSP: 0018:b7d7c41cbbf0 EFLAGS: 00010246
> [166164.933237] RAX: RBX: 8a90001fe900 RCX:
>
> [166164.933238] RDX: RSI: 0003 RDI:
> c0a40180
> [166164.933239] RBP: b7d7c41cbbf8 R08: R09:
> 8a93e157d6d0
> [166164.933240] R10: R11: c0a40188 R12:
> 0003
> [166164.933241] R13: 8a9402200e80 R14: 8a90001fe900 R15:
>
> [166164.933244] FS: 7f7fb041eb00() GS:8a941150()
> knlGS:
> [166164.933245] CS: 0010 DS: ES: CR0: 80050033
> [166164.933246] CR2: 0018 CR3: 352c2003 CR4:
> 003606e0
> [166164.933247] Call Trace:
> [166164.933264] auxdev_open+0x1b/0x40 [drm_kms_helper]
> [166164.933278] chrdev_open+0xa7/0x1c0
> [166164.933282] ? cdev_put.part.0+0x20/0x20
> [166164.933287] do_dentry_open+0x161/0x3c0
> [166164.933291] vfs_open+0x2d/0x30
> [166164.933297] path_openat+0xb27/0x10e0
> [166164.933306] ? atime_needs_update+0x73/0xd0
> [166164.933309] do_filp_open+0x91/0x100
> [166164.933313] ? __alloc_fd+0xb2/0x150
> [166164.933316] do_sys_openat2+0x210/0x2d0
> [166164.933318] do_sys_open+0x46/0x80
> [166164.933320] __x64_sys_openat+0x20/0x30
> [166164.933328] do_syscall_64+0x52/0xc0
> [166164.96] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
>
> (gdb) disassemble drm_dp_aux_dev_get_by_minor+0x29
> Dump of assembler code for function drm_dp_aux_dev_get_by_minor:
>0x00017b10 <+0>: callq 0x17b15
>0x00017b15 <+5>: push %rbp
>0x00017b16 <+6>: mov%rsp,%rbp
>0x00017b19 <+9>: push %r12
>0x00017b1b <+11>:mov%edi,%r12d
>0x00017b1e <+14>:mov$0x0,%rdi
>0x00017b25 <+21>:callq 0x17b2a
>
>0x00017b2a <+26>:mov%r12d,%esi
>0x00017b2d <+29>:mov$0x0,%rdi
>0x00017b34 <+36>:callq 0x17b39
>
>0x00017b39 <+41>:mov0x18(%rax),%edx <=
>0x00017b3c <+44>:mov%rax,%r12
>0x00017b3f <+47>:lea0x18(%rax),%rdi
>0x00017b43 <+51>:test %edx,%edx
>0x00017b45 <+53>:je 0x17b7a
>
>0x00017b47 <+55>:lea0x1(%rdx),%ecx
>0x00017b4a <+58>:mov%edx,%eax
>0x00017b4c <+60>:lock cmpxchg %ecx,(%rdi)
>0x00017b50 <+64>:jne0x17b76
>
>0x00017b52 <+66>:test %edx,%edx
>0x00017b54 <+68>:js 0x17b6d
>
>0x00017b56 <+70>:test %ecx,%ecx
>0x00017b58 <+72>:js 0x17b6d
>
>0x00017b5a <+74>:mov$0x0,%rdi
>0x00017b61 <+81>:callq 0x17b66
>
>0x00017b66 <+86>:mov%r12,%rax
>0x00017b69 <+89>:pop%r12
>0x00017b6b <+91>:pop%rbp
>0x00017b6c <+92>:retq
>0x00017b6d <+93>:xor%esi,%esi
>0x00017b6f <+95>:callq 0x17b74
>
>0x00017b74 <+100>: jmp0x17b5a
>
>0x00017b76 <+102>: mov%eax,%edx
>0x00017b78 <+104>: jmp0x17b43
>
>0x00017b7a <+106>: xor%r12d,%r12d
>0x00017b7d <+109>: jmp0x17b5a
>
> End of assembler dump.
>
> (gdb) list *drm_dp_aux_dev_get_by_minor+0x29
> 0x17b39 is in drm_dp_aux_dev_get_by_minor
> (drivers/gpu/drm/drm_dp_aux_dev.c:65).
> 60 static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_minor(unsigned
> index)
> 61 {
> 62 struct drm_dp_aux_dev *aux_dev = NULL;
> 63
