dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* drm: GPF in drm_getcap
@ 2016-09-09 11:56 Dmitry Vyukov
  2016-11-26 17:17 ` Dmitry Vyukov
  0 siblings, 1 reply; 21+ messages in thread
From: Dmitry Vyukov @ 2016-09-09 11:56 UTC (permalink / raw)
  To: airlied, dri-devel, LKML; +Cc: syzkaller

Hello,

The following program triggers GPF in drm_getcap:

// autogenerated by syzkaller (http://github.com/google/syzkaller)
#include <fcntl.h>
#include <stddef.h>
#include <stdint.h>
#include <sys/ioctl.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <unistd.h>

int main()
{
  int fd = open("/dev/dri/card0", O_RDONLY);
  uint64_t data[2] = {0x11, 0x80};
  ioctl(fd, 0xc010640cul /*DRM_IOCTL_GET_CAP*/, data);
  return 0;
}


general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN
Modules linked in:
CPU: 1 PID: 5745 Comm: syz-executor Not tainted 4.8.0-rc5-next-20160905+ #14
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: ffff8800310dc540 task.stack: ffff88003cbc0000
RIP: 0010:[<ffffffff834ca87b>]  [<ffffffff834ca87b>]
drm_getcap+0x34b/0x4f0 drivers/gpu/drm/drm_ioctl.c:260
RSP: 0018:ffff88003cbc7c28  EFLAGS: 00010202
RAX: 0000000000000058 RBX: ffff88003cbc7cf8 RCX: ffffc90001db0000
RDX: 000000000000005d RSI: ffff88003cbc7cf8 RDI: 00000000000002c0
RBP: ffff88003cbc7c50 R08: ffffed0007978fa1 R09: ffffed0007978fa0
R10: ffff88003cbc7d07 R11: ffffed0007978fa1 R12: fffffffffffffff0
R13: dffffc0000000000 R14: ffff88003bcc6850 R15: fffffffffffffff2
FS:  00007fcbf4e03700(0000) GS:ffff88003ed00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000006dce00 CR3: 0000000066135000 CR4: 00000000000006e0
DR0: 000000000000001e DR1: 000000000000001e DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
Stack:
 ffff88003c26db00 ffff88003cbc7cf8 ffffffff875a3000 ffffffff88cf0ee0
 fffffffffffffff2 ffff88003cbc7dc0 ffffffff834cb57c 000000000000e200
 1ffff10000000001 ffffffff875a1ba0 ffffffff882ae930 0000000000000010
Call Trace:
 [<ffffffff834cb57c>] drm_ioctl+0x54c/0xaf0 drivers/gpu/drm/drm_ioctl.c:728
 [<     inline     >] vfs_ioctl fs/ioctl.c:43
 [<ffffffff818a331c>] do_vfs_ioctl+0x18c/0x1080 fs/ioctl.c:675
 [<     inline     >] SYSC_ioctl fs/ioctl.c:690
 [<ffffffff818a429f>] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:681
 [<ffffffff86e1a8c0>] entry_SYSCALL_64_fastpath+0x23/0xc1
Code: 3c 28 00 0f 85 88 01 00 00 49 8b 44 24 10 49 39 c6 4c 8d 60 f0
74 82 e8 64 19 10 fe 49 8d bc 24 d0 02 00 00 48 89 f8 48 c1 e8 03 <42>
80 3c 28 00 0f 85 6f 01 00 00 4d 8b bc 24 d0 02 00 00 49 8d
RIP  [<ffffffff834ca87b>] drm_getcap+0x34b/0x4f0 drivers/gpu/drm/drm_ioctl.c:260
 RSP <ffff88003cbc7c28>
---[ end trace c6e1afa8cd73b880 ]---


On commit 4affa544adb8077403893e62b9e327fcf87de6f7 (Sep 8) of linux-next.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: drm: GPF in drm_getcap
  2016-09-09 11:56 drm: GPF in drm_getcap Dmitry Vyukov
@ 2016-11-26 17:17 ` Dmitry Vyukov
  2016-11-26 17:35   ` David Herrmann
  0 siblings, 1 reply; 21+ messages in thread
From: Dmitry Vyukov @ 2016-11-26 17:17 UTC (permalink / raw)
  To: airlied, dri-devel, LKML; +Cc: syzkaller

On Fri, Sep 9, 2016 at 1:56 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> Hello,
>
> The following program triggers GPF in drm_getcap:
>
> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> #include <fcntl.h>
> #include <stddef.h>
> #include <stdint.h>
> #include <sys/ioctl.h>
> #include <sys/stat.h>
> #include <sys/syscall.h>
> #include <sys/types.h>
> #include <unistd.h>
>
> int main()
> {
>   int fd = open("/dev/dri/card0", O_RDONLY);
>   uint64_t data[2] = {0x11, 0x80};
>   ioctl(fd, 0xc010640cul /*DRM_IOCTL_GET_CAP*/, data);
>   return 0;
> }
>
>
> general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN
> Modules linked in:
> CPU: 1 PID: 5745 Comm: syz-executor Not tainted 4.8.0-rc5-next-20160905+ #14
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> task: ffff8800310dc540 task.stack: ffff88003cbc0000
> RIP: 0010:[<ffffffff834ca87b>]  [<ffffffff834ca87b>]
> drm_getcap+0x34b/0x4f0 drivers/gpu/drm/drm_ioctl.c:260
> RSP: 0018:ffff88003cbc7c28  EFLAGS: 00010202
> RAX: 0000000000000058 RBX: ffff88003cbc7cf8 RCX: ffffc90001db0000
> RDX: 000000000000005d RSI: ffff88003cbc7cf8 RDI: 00000000000002c0
> RBP: ffff88003cbc7c50 R08: ffffed0007978fa1 R09: ffffed0007978fa0
> R10: ffff88003cbc7d07 R11: ffffed0007978fa1 R12: fffffffffffffff0
> R13: dffffc0000000000 R14: ffff88003bcc6850 R15: fffffffffffffff2
> FS:  00007fcbf4e03700(0000) GS:ffff88003ed00000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00000000006dce00 CR3: 0000000066135000 CR4: 00000000000006e0
> DR0: 000000000000001e DR1: 000000000000001e DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
> Stack:
>  ffff88003c26db00 ffff88003cbc7cf8 ffffffff875a3000 ffffffff88cf0ee0
>  fffffffffffffff2 ffff88003cbc7dc0 ffffffff834cb57c 000000000000e200
>  1ffff10000000001 ffffffff875a1ba0 ffffffff882ae930 0000000000000010
> Call Trace:
>  [<ffffffff834cb57c>] drm_ioctl+0x54c/0xaf0 drivers/gpu/drm/drm_ioctl.c:728
>  [<     inline     >] vfs_ioctl fs/ioctl.c:43
>  [<ffffffff818a331c>] do_vfs_ioctl+0x18c/0x1080 fs/ioctl.c:675
>  [<     inline     >] SYSC_ioctl fs/ioctl.c:690
>  [<ffffffff818a429f>] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:681
>  [<ffffffff86e1a8c0>] entry_SYSCALL_64_fastpath+0x23/0xc1
> Code: 3c 28 00 0f 85 88 01 00 00 49 8b 44 24 10 49 39 c6 4c 8d 60 f0
> 74 82 e8 64 19 10 fe 49 8d bc 24 d0 02 00 00 48 89 f8 48 c1 e8 03 <42>
> 80 3c 28 00 0f 85 6f 01 00 00 4d 8b bc 24 d0 02 00 00 49 8d
> RIP  [<ffffffff834ca87b>] drm_getcap+0x34b/0x4f0 drivers/gpu/drm/drm_ioctl.c:260
>  RSP <ffff88003cbc7c28>
> ---[ end trace c6e1afa8cd73b880 ]---
>
>
> On commit 4affa544adb8077403893e62b9e327fcf87de6f7 (Sep 8) of linux-next.

ping

Still happens on 16ae16c6e5616c084168740990fc508bda6655d4 (Nov 24).

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: drm: GPF in drm_getcap
  2016-11-26 17:17 ` Dmitry Vyukov
@ 2016-11-26 17:35   ` David Herrmann
  2016-11-26 17:50     ` Dmitry Vyukov
  0 siblings, 1 reply; 21+ messages in thread
From: David Herrmann @ 2016-11-26 17:35 UTC (permalink / raw)
  To: Dmitry Vyukov, Daniel Vetter
  Cc: syzkaller, LKML, dri-devel@lists.freedesktop.org

Hi

On Sat, Nov 26, 2016 at 6:17 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Fri, Sep 9, 2016 at 1:56 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> Hello,
>>
>> The following program triggers GPF in drm_getcap:
>>
>> // autogenerated by syzkaller (http://github.com/google/syzkaller)
>> #include <fcntl.h>
>> #include <stddef.h>
>> #include <stdint.h>
>> #include <sys/ioctl.h>
>> #include <sys/stat.h>
>> #include <sys/syscall.h>
>> #include <sys/types.h>
>> #include <unistd.h>
>>
>> int main()
>> {
>>   int fd = open("/dev/dri/card0", O_RDONLY);
>>   uint64_t data[2] = {0x11, 0x80};
>>   ioctl(fd, 0xc010640cul /*DRM_IOCTL_GET_CAP*/, data);
>>   return 0;
>> }
>>
>>
>> general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN
>> Modules linked in:
>> CPU: 1 PID: 5745 Comm: syz-executor Not tainted 4.8.0-rc5-next-20160905+ #14
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>> task: ffff8800310dc540 task.stack: ffff88003cbc0000
>> RIP: 0010:[<ffffffff834ca87b>]  [<ffffffff834ca87b>]
>> drm_getcap+0x34b/0x4f0 drivers/gpu/drm/drm_ioctl.c:260
>> RSP: 0018:ffff88003cbc7c28  EFLAGS: 00010202
>> RAX: 0000000000000058 RBX: ffff88003cbc7cf8 RCX: ffffc90001db0000
>> RDX: 000000000000005d RSI: ffff88003cbc7cf8 RDI: 00000000000002c0
>> RBP: ffff88003cbc7c50 R08: ffffed0007978fa1 R09: ffffed0007978fa0
>> R10: ffff88003cbc7d07 R11: ffffed0007978fa1 R12: fffffffffffffff0
>> R13: dffffc0000000000 R14: ffff88003bcc6850 R15: fffffffffffffff2
>> FS:  00007fcbf4e03700(0000) GS:ffff88003ed00000(0000) knlGS:0000000000000000
>> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 00000000006dce00 CR3: 0000000066135000 CR4: 00000000000006e0
>> DR0: 000000000000001e DR1: 000000000000001e DR2: 0000000000000000
>> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
>> Stack:
>>  ffff88003c26db00 ffff88003cbc7cf8 ffffffff875a3000 ffffffff88cf0ee0
>>  fffffffffffffff2 ffff88003cbc7dc0 ffffffff834cb57c 000000000000e200
>>  1ffff10000000001 ffffffff875a1ba0 ffffffff882ae930 0000000000000010
>> Call Trace:
>>  [<ffffffff834cb57c>] drm_ioctl+0x54c/0xaf0 drivers/gpu/drm/drm_ioctl.c:728
>>  [<     inline     >] vfs_ioctl fs/ioctl.c:43
>>  [<ffffffff818a331c>] do_vfs_ioctl+0x18c/0x1080 fs/ioctl.c:675
>>  [<     inline     >] SYSC_ioctl fs/ioctl.c:690
>>  [<ffffffff818a429f>] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:681
>>  [<ffffffff86e1a8c0>] entry_SYSCALL_64_fastpath+0x23/0xc1
>> Code: 3c 28 00 0f 85 88 01 00 00 49 8b 44 24 10 49 39 c6 4c 8d 60 f0
>> 74 82 e8 64 19 10 fe 49 8d bc 24 d0 02 00 00 48 89 f8 48 c1 e8 03 <42>
>> 80 3c 28 00 0f 85 6f 01 00 00 4d 8b bc 24 d0 02 00 00 49 8d
>> RIP  [<ffffffff834ca87b>] drm_getcap+0x34b/0x4f0 drivers/gpu/drm/drm_ioctl.c:260
>>  RSP <ffff88003cbc7c28>
>> ---[ end trace c6e1afa8cd73b880 ]---
>>
>>
>> On commit 4affa544adb8077403893e62b9e327fcf87de6f7 (Sep 8) of linux-next.
>
> ping
>
> Still happens on 16ae16c6e5616c084168740990fc508bda6655d4 (Nov 24).

I suspect this is because we run drm_for_each_crtc() in
drm_getcap(DRM_PAGE_FLIP_TARGET) on a legacy driver (meaning
mode_config is not initialized). @danvet, how about always
initializing mode_config to 0/empty/dummy?

Dmitry, what driver do you run this on? And is CONFIG_DRM_LEGACY enabled?

Thanks
David
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: drm: GPF in drm_getcap
  2016-11-26 17:35   ` David Herrmann
@ 2016-11-26 17:50     ` Dmitry Vyukov
  2016-11-26 18:02       ` David Herrmann
  0 siblings, 1 reply; 21+ messages in thread
From: Dmitry Vyukov @ 2016-11-26 17:50 UTC (permalink / raw)
  To: syzkaller
  Cc: Daniel Vetter, David Airlie, dri-devel@lists.freedesktop.org,
	LKML

On Sat, Nov 26, 2016 at 6:35 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Sat, Nov 26, 2016 at 6:17 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> On Fri, Sep 9, 2016 at 1:56 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>> Hello,
>>>
>>> The following program triggers GPF in drm_getcap:
>>>
>>> // autogenerated by syzkaller (http://github.com/google/syzkaller)
>>> #include <fcntl.h>
>>> #include <stddef.h>
>>> #include <stdint.h>
>>> #include <sys/ioctl.h>
>>> #include <sys/stat.h>
>>> #include <sys/syscall.h>
>>> #include <sys/types.h>
>>> #include <unistd.h>
>>>
>>> int main()
>>> {
>>>   int fd = open("/dev/dri/card0", O_RDONLY);
>>>   uint64_t data[2] = {0x11, 0x80};
>>>   ioctl(fd, 0xc010640cul /*DRM_IOCTL_GET_CAP*/, data);
>>>   return 0;
>>> }
>>>
>>>
>>> general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN
>>> Modules linked in:
>>> CPU: 1 PID: 5745 Comm: syz-executor Not tainted 4.8.0-rc5-next-20160905+ #14
>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>> task: ffff8800310dc540 task.stack: ffff88003cbc0000
>>> RIP: 0010:[<ffffffff834ca87b>]  [<ffffffff834ca87b>]
>>> drm_getcap+0x34b/0x4f0 drivers/gpu/drm/drm_ioctl.c:260
>>> RSP: 0018:ffff88003cbc7c28  EFLAGS: 00010202
>>> RAX: 0000000000000058 RBX: ffff88003cbc7cf8 RCX: ffffc90001db0000
>>> RDX: 000000000000005d RSI: ffff88003cbc7cf8 RDI: 00000000000002c0
>>> RBP: ffff88003cbc7c50 R08: ffffed0007978fa1 R09: ffffed0007978fa0
>>> R10: ffff88003cbc7d07 R11: ffffed0007978fa1 R12: fffffffffffffff0
>>> R13: dffffc0000000000 R14: ffff88003bcc6850 R15: fffffffffffffff2
>>> FS:  00007fcbf4e03700(0000) GS:ffff88003ed00000(0000) knlGS:0000000000000000
>>> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> CR2: 00000000006dce00 CR3: 0000000066135000 CR4: 00000000000006e0
>>> DR0: 000000000000001e DR1: 000000000000001e DR2: 0000000000000000
>>> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
>>> Stack:
>>>  ffff88003c26db00 ffff88003cbc7cf8 ffffffff875a3000 ffffffff88cf0ee0
>>>  fffffffffffffff2 ffff88003cbc7dc0 ffffffff834cb57c 000000000000e200
>>>  1ffff10000000001 ffffffff875a1ba0 ffffffff882ae930 0000000000000010
>>> Call Trace:
>>>  [<ffffffff834cb57c>] drm_ioctl+0x54c/0xaf0 drivers/gpu/drm/drm_ioctl.c:728
>>>  [<     inline     >] vfs_ioctl fs/ioctl.c:43
>>>  [<ffffffff818a331c>] do_vfs_ioctl+0x18c/0x1080 fs/ioctl.c:675
>>>  [<     inline     >] SYSC_ioctl fs/ioctl.c:690
>>>  [<ffffffff818a429f>] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:681
>>>  [<ffffffff86e1a8c0>] entry_SYSCALL_64_fastpath+0x23/0xc1
>>> Code: 3c 28 00 0f 85 88 01 00 00 49 8b 44 24 10 49 39 c6 4c 8d 60 f0
>>> 74 82 e8 64 19 10 fe 49 8d bc 24 d0 02 00 00 48 89 f8 48 c1 e8 03 <42>
>>> 80 3c 28 00 0f 85 6f 01 00 00 4d 8b bc 24 d0 02 00 00 49 8d
>>> RIP  [<ffffffff834ca87b>] drm_getcap+0x34b/0x4f0 drivers/gpu/drm/drm_ioctl.c:260
>>>  RSP <ffff88003cbc7c28>
>>> ---[ end trace c6e1afa8cd73b880 ]---
>>>
>>>
>>> On commit 4affa544adb8077403893e62b9e327fcf87de6f7 (Sep 8) of linux-next.
>>
>> ping
>>
>> Still happens on 16ae16c6e5616c084168740990fc508bda6655d4 (Nov 24).
>
> I suspect this is because we run drm_for_each_crtc() in
> drm_getcap(DRM_PAGE_FLIP_TARGET) on a legacy driver (meaning
> mode_config is not initialized). @danvet, how about always
> initializing mode_config to 0/empty/dummy?
>
> Dmitry, what driver do you run this on? And is CONFIG_DRM_LEGACY enabled?


CONFIG_DRM_LEGACY is enabled.

How can I understand what driver is used?
This happens inside of qemu. This is the device:
crw-rw---T 1 root video 226, 0 Nov 26 17:45 /dev/dri/card0

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: drm: GPF in drm_getcap
  2016-11-26 17:50     ` Dmitry Vyukov
@ 2016-11-26 18:02       ` David Herrmann
  2016-11-26 18:07         ` Dmitry Vyukov
  0 siblings, 1 reply; 21+ messages in thread
From: David Herrmann @ 2016-11-26 18:02 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Daniel Vetter, syzkaller, LKML, dri-devel@lists.freedesktop.org

Hi

On Sat, Nov 26, 2016 at 6:50 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Sat, Nov 26, 2016 at 6:35 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
>> Hi
>>
>> On Sat, Nov 26, 2016 at 6:17 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>> On Fri, Sep 9, 2016 at 1:56 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>>> Hello,
>>>>
>>>> The following program triggers GPF in drm_getcap:
>>>>
>>>> // autogenerated by syzkaller (http://github.com/google/syzkaller)
>>>> #include <fcntl.h>
>>>> #include <stddef.h>
>>>> #include <stdint.h>
>>>> #include <sys/ioctl.h>
>>>> #include <sys/stat.h>
>>>> #include <sys/syscall.h>
>>>> #include <sys/types.h>
>>>> #include <unistd.h>
>>>>
>>>> int main()
>>>> {
>>>>   int fd = open("/dev/dri/card0", O_RDONLY);
>>>>   uint64_t data[2] = {0x11, 0x80};
>>>>   ioctl(fd, 0xc010640cul /*DRM_IOCTL_GET_CAP*/, data);
>>>>   return 0;
>>>> }
>>>>
>>>>
>>>> general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN
>>>> Modules linked in:
>>>> CPU: 1 PID: 5745 Comm: syz-executor Not tainted 4.8.0-rc5-next-20160905+ #14
>>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>>> task: ffff8800310dc540 task.stack: ffff88003cbc0000
>>>> RIP: 0010:[<ffffffff834ca87b>]  [<ffffffff834ca87b>]
>>>> drm_getcap+0x34b/0x4f0 drivers/gpu/drm/drm_ioctl.c:260
>>>> RSP: 0018:ffff88003cbc7c28  EFLAGS: 00010202
>>>> RAX: 0000000000000058 RBX: ffff88003cbc7cf8 RCX: ffffc90001db0000
>>>> RDX: 000000000000005d RSI: ffff88003cbc7cf8 RDI: 00000000000002c0
>>>> RBP: ffff88003cbc7c50 R08: ffffed0007978fa1 R09: ffffed0007978fa0
>>>> R10: ffff88003cbc7d07 R11: ffffed0007978fa1 R12: fffffffffffffff0
>>>> R13: dffffc0000000000 R14: ffff88003bcc6850 R15: fffffffffffffff2
>>>> FS:  00007fcbf4e03700(0000) GS:ffff88003ed00000(0000) knlGS:0000000000000000
>>>> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> CR2: 00000000006dce00 CR3: 0000000066135000 CR4: 00000000000006e0
>>>> DR0: 000000000000001e DR1: 000000000000001e DR2: 0000000000000000
>>>> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
>>>> Stack:
>>>>  ffff88003c26db00 ffff88003cbc7cf8 ffffffff875a3000 ffffffff88cf0ee0
>>>>  fffffffffffffff2 ffff88003cbc7dc0 ffffffff834cb57c 000000000000e200
>>>>  1ffff10000000001 ffffffff875a1ba0 ffffffff882ae930 0000000000000010
>>>> Call Trace:
>>>>  [<ffffffff834cb57c>] drm_ioctl+0x54c/0xaf0 drivers/gpu/drm/drm_ioctl.c:728
>>>>  [<     inline     >] vfs_ioctl fs/ioctl.c:43
>>>>  [<ffffffff818a331c>] do_vfs_ioctl+0x18c/0x1080 fs/ioctl.c:675
>>>>  [<     inline     >] SYSC_ioctl fs/ioctl.c:690
>>>>  [<ffffffff818a429f>] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:681
>>>>  [<ffffffff86e1a8c0>] entry_SYSCALL_64_fastpath+0x23/0xc1
>>>> Code: 3c 28 00 0f 85 88 01 00 00 49 8b 44 24 10 49 39 c6 4c 8d 60 f0
>>>> 74 82 e8 64 19 10 fe 49 8d bc 24 d0 02 00 00 48 89 f8 48 c1 e8 03 <42>
>>>> 80 3c 28 00 0f 85 6f 01 00 00 4d 8b bc 24 d0 02 00 00 49 8d
>>>> RIP  [<ffffffff834ca87b>] drm_getcap+0x34b/0x4f0 drivers/gpu/drm/drm_ioctl.c:260
>>>>  RSP <ffff88003cbc7c28>
>>>> ---[ end trace c6e1afa8cd73b880 ]---
>>>>
>>>>
>>>> On commit 4affa544adb8077403893e62b9e327fcf87de6f7 (Sep 8) of linux-next.
>>>
>>> ping
>>>
>>> Still happens on 16ae16c6e5616c084168740990fc508bda6655d4 (Nov 24).
>>
>> I suspect this is because we run drm_for_each_crtc() in
>> drm_getcap(DRM_PAGE_FLIP_TARGET) on a legacy driver (meaning
>> mode_config is not initialized). @danvet, how about always
>> initializing mode_config to 0/empty/dummy?
>>
>> Dmitry, what driver do you run this on? And is CONFIG_DRM_LEGACY enabled?
>
>
> CONFIG_DRM_LEGACY is enabled.
>
> How can I understand what driver is used?
> This happens inside of qemu. This is the device:
> crw-rw---T 1 root video 226, 0 Nov 26 17:45 /dev/dri/card0

Usually by looking into `dmesg` and grepping for 'card0', or by inspecting:

    /sys/class/drm/card0/device/

or more importantly looking at the symlink:

    /sys/class/drm/card0/device/driver

Thanks
David
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: drm: GPF in drm_getcap
  2016-11-26 18:02       ` David Herrmann
@ 2016-11-26 18:07         ` Dmitry Vyukov
  2016-11-26 18:22           ` David Herrmann
  0 siblings, 1 reply; 21+ messages in thread
From: Dmitry Vyukov @ 2016-11-26 18:07 UTC (permalink / raw)
  To: David Herrmann
  Cc: syzkaller, Daniel Vetter, David Airlie,
	dri-devel@lists.freedesktop.org, LKML

grep "card0" dmesg:
[    5.298617] device: 'card0': device_add
[    5.298946] PM: Adding info for No Bus:card0
[    6.436178] device: 'card0': device_add
[    6.436488] PM: Adding info for No Bus:card0


# ls -l /dev/dri/card0
crw-rw---T 1 root video 226, 0 Nov 26 18:05 /dev/dri/card0

# ls -lt /sys/class/drm/card0/device/
ls: cannot access /sys/class/drm/card0/device/: No such file or directory

# ls -lt /sys/class/drm/card0/device/driver
ls: cannot access /sys/class/drm/card0/device/driver: No such file or directory


On Sat, Nov 26, 2016 at 7:02 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Sat, Nov 26, 2016 at 6:50 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> On Sat, Nov 26, 2016 at 6:35 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
>>> Hi
>>>
>>> On Sat, Nov 26, 2016 at 6:17 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>>> On Fri, Sep 9, 2016 at 1:56 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>>>> Hello,
>>>>>
>>>>> The following program triggers GPF in drm_getcap:
>>>>>
>>>>> // autogenerated by syzkaller (http://github.com/google/syzkaller)
>>>>> #include <fcntl.h>
>>>>> #include <stddef.h>
>>>>> #include <stdint.h>
>>>>> #include <sys/ioctl.h>
>>>>> #include <sys/stat.h>
>>>>> #include <sys/syscall.h>
>>>>> #include <sys/types.h>
>>>>> #include <unistd.h>
>>>>>
>>>>> int main()
>>>>> {
>>>>>   int fd = open("/dev/dri/card0", O_RDONLY);
>>>>>   uint64_t data[2] = {0x11, 0x80};
>>>>>   ioctl(fd, 0xc010640cul /*DRM_IOCTL_GET_CAP*/, data);
>>>>>   return 0;
>>>>> }
>>>>>
>>>>>
>>>>> general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN
>>>>> Modules linked in:
>>>>> CPU: 1 PID: 5745 Comm: syz-executor Not tainted 4.8.0-rc5-next-20160905+ #14
>>>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>>>> task: ffff8800310dc540 task.stack: ffff88003cbc0000
>>>>> RIP: 0010:[<ffffffff834ca87b>]  [<ffffffff834ca87b>]
>>>>> drm_getcap+0x34b/0x4f0 drivers/gpu/drm/drm_ioctl.c:260
>>>>> RSP: 0018:ffff88003cbc7c28  EFLAGS: 00010202
>>>>> RAX: 0000000000000058 RBX: ffff88003cbc7cf8 RCX: ffffc90001db0000
>>>>> RDX: 000000000000005d RSI: ffff88003cbc7cf8 RDI: 00000000000002c0
>>>>> RBP: ffff88003cbc7c50 R08: ffffed0007978fa1 R09: ffffed0007978fa0
>>>>> R10: ffff88003cbc7d07 R11: ffffed0007978fa1 R12: fffffffffffffff0
>>>>> R13: dffffc0000000000 R14: ffff88003bcc6850 R15: fffffffffffffff2
>>>>> FS:  00007fcbf4e03700(0000) GS:ffff88003ed00000(0000) knlGS:0000000000000000
>>>>> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>>> CR2: 00000000006dce00 CR3: 0000000066135000 CR4: 00000000000006e0
>>>>> DR0: 000000000000001e DR1: 000000000000001e DR2: 0000000000000000
>>>>> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
>>>>> Stack:
>>>>>  ffff88003c26db00 ffff88003cbc7cf8 ffffffff875a3000 ffffffff88cf0ee0
>>>>>  fffffffffffffff2 ffff88003cbc7dc0 ffffffff834cb57c 000000000000e200
>>>>>  1ffff10000000001 ffffffff875a1ba0 ffffffff882ae930 0000000000000010
>>>>> Call Trace:
>>>>>  [<ffffffff834cb57c>] drm_ioctl+0x54c/0xaf0 drivers/gpu/drm/drm_ioctl.c:728
>>>>>  [<     inline     >] vfs_ioctl fs/ioctl.c:43
>>>>>  [<ffffffff818a331c>] do_vfs_ioctl+0x18c/0x1080 fs/ioctl.c:675
>>>>>  [<     inline     >] SYSC_ioctl fs/ioctl.c:690
>>>>>  [<ffffffff818a429f>] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:681
>>>>>  [<ffffffff86e1a8c0>] entry_SYSCALL_64_fastpath+0x23/0xc1
>>>>> Code: 3c 28 00 0f 85 88 01 00 00 49 8b 44 24 10 49 39 c6 4c 8d 60 f0
>>>>> 74 82 e8 64 19 10 fe 49 8d bc 24 d0 02 00 00 48 89 f8 48 c1 e8 03 <42>
>>>>> 80 3c 28 00 0f 85 6f 01 00 00 4d 8b bc 24 d0 02 00 00 49 8d
>>>>> RIP  [<ffffffff834ca87b>] drm_getcap+0x34b/0x4f0 drivers/gpu/drm/drm_ioctl.c:260
>>>>>  RSP <ffff88003cbc7c28>
>>>>> ---[ end trace c6e1afa8cd73b880 ]---
>>>>>
>>>>>
>>>>> On commit 4affa544adb8077403893e62b9e327fcf87de6f7 (Sep 8) of linux-next.
>>>>
>>>> ping
>>>>
>>>> Still happens on 16ae16c6e5616c084168740990fc508bda6655d4 (Nov 24).
>>>
>>> I suspect this is because we run drm_for_each_crtc() in
>>> drm_getcap(DRM_PAGE_FLIP_TARGET) on a legacy driver (meaning
>>> mode_config is not initialized). @danvet, how about always
>>> initializing mode_config to 0/empty/dummy?
>>>
>>> Dmitry, what driver do you run this on? And is CONFIG_DRM_LEGACY enabled?
>>
>>
>> CONFIG_DRM_LEGACY is enabled.
>>
>> How can I understand what driver is used?
>> This happens inside of qemu. This is the device:
>> crw-rw---T 1 root video 226, 0 Nov 26 17:45 /dev/dri/card0
>
> Usually by looking into `dmesg` and grepping for 'card0', or by inspecting:
>
>     /sys/class/drm/card0/device/
>
> or more importantly looking at the symlink:
>
>     /sys/class/drm/card0/device/driver
>
> Thanks
> David

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: drm: GPF in drm_getcap
  2016-11-26 18:07         ` Dmitry Vyukov
@ 2016-11-26 18:22           ` David Herrmann
  2016-11-28  6:55             ` Daniel Vetter
  0 siblings, 1 reply; 21+ messages in thread
From: David Herrmann @ 2016-11-26 18:22 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Daniel Vetter, syzkaller, LKML, dri-devel@lists.freedesktop.org

Hi

On Sat, Nov 26, 2016 at 7:07 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> grep "card0" dmesg:
> [    5.298617] device: 'card0': device_add
> [    5.298946] PM: Adding info for No Bus:card0
> [    6.436178] device: 'card0': device_add
> [    6.436488] PM: Adding info for No Bus:card0
>
>
> # ls -l /dev/dri/card0
> crw-rw---T 1 root video 226, 0 Nov 26 18:05 /dev/dri/card0
>
> # ls -lt /sys/class/drm/card0/device/
> ls: cannot access /sys/class/drm/card0/device/: No such file or directory
>
> # ls -lt /sys/class/drm/card0/device/driver
> ls: cannot access /sys/class/drm/card0/device/driver: No such file or directory

Looks like vgem. Something like this should help:

    https://gist.github.com/dvdhrm/1bcdf4f3485aa1614a0198a7b90515e2

I wonder whether it would be more appropriate to return -ENOTSUPP rather than 0.

Thanks
David
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: drm: GPF in drm_getcap
  2016-11-26 18:22           ` David Herrmann
@ 2016-11-28  6:55             ` Daniel Vetter
  2016-11-28  7:14               ` Michel Dänzer
  0 siblings, 1 reply; 21+ messages in thread
From: Daniel Vetter @ 2016-11-28  6:55 UTC (permalink / raw)
  To: David Herrmann, Michel Dänzer
  Cc: Dmitry Vyukov, syzkaller, David Airlie,
	dri-devel@lists.freedesktop.org, LKML

On Sat, Nov 26, 2016 at 7:22 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> On Sat, Nov 26, 2016 at 7:07 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> grep "card0" dmesg:
>> [    5.298617] device: 'card0': device_add
>> [    5.298946] PM: Adding info for No Bus:card0
>> [    6.436178] device: 'card0': device_add
>> [    6.436488] PM: Adding info for No Bus:card0
>>
>>
>> # ls -l /dev/dri/card0
>> crw-rw---T 1 root video 226, 0 Nov 26 18:05 /dev/dri/card0
>>
>> # ls -lt /sys/class/drm/card0/device/
>> ls: cannot access /sys/class/drm/card0/device/: No such file or directory
>>
>> # ls -lt /sys/class/drm/card0/device/driver
>> ls: cannot access /sys/class/drm/card0/device/driver: No such file or directory
>
> Looks like vgem. Something like this should help:
>
>     https://gist.github.com/dvdhrm/1bcdf4f3485aa1614a0198a7b90515e2
>
> I wonder whether it would be more appropriate to return -ENOTSUPP rather than 0.

Seems a bit overkill, but can't hurt. This is most likely a
regression, probably introduced in

commit f837297ad82480024d3ad08cd84f6670bcafa862
Author: Michel Dänzer <michel.daenzer@amd.com>
Date:   Mon Aug 8 16:23:39 2016 +0900

    drm: Add DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags v2

Michel, can you pls take care of this? Either with a minimal fix, or
by adopting David's patch?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: drm: GPF in drm_getcap
  2016-11-28  6:55             ` Daniel Vetter
@ 2016-11-28  7:14               ` Michel Dänzer
  2016-11-28  8:41                 ` Dmitry Vyukov
  0 siblings, 1 reply; 21+ messages in thread
From: Michel Dänzer @ 2016-11-28  7:14 UTC (permalink / raw)
  To: Daniel Vetter, David Herrmann
  Cc: syzkaller, dri-devel@lists.freedesktop.org, Dmitry Vyukov, LKML

On 28/11/16 03:55 PM, Daniel Vetter wrote:
> On Sat, Nov 26, 2016 at 7:22 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
>> On Sat, Nov 26, 2016 at 7:07 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>> grep "card0" dmesg:
>>> [    5.298617] device: 'card0': device_add
>>> [    5.298946] PM: Adding info for No Bus:card0
>>> [    6.436178] device: 'card0': device_add
>>> [    6.436488] PM: Adding info for No Bus:card0
>>>
>>>
>>> # ls -l /dev/dri/card0
>>> crw-rw---T 1 root video 226, 0 Nov 26 18:05 /dev/dri/card0
>>>
>>> # ls -lt /sys/class/drm/card0/device/
>>> ls: cannot access /sys/class/drm/card0/device/: No such file or directory
>>>
>>> # ls -lt /sys/class/drm/card0/device/driver
>>> ls: cannot access /sys/class/drm/card0/device/driver: No such file or directory
>>
>> Looks like vgem. Something like this should help:
>>
>>     https://gist.github.com/dvdhrm/1bcdf4f3485aa1614a0198a7b90515e2
>>
>> I wonder whether it would be more appropriate to return -ENOTSUPP rather than 0.

Can't see how that would matter FWIW.


> Seems a bit overkill, but can't hurt. This is most likely a
> regression, probably introduced in
> 
> commit f837297ad82480024d3ad08cd84f6670bcafa862
> Author: Michel Dänzer <michel.daenzer@amd.com>
> Date:   Mon Aug 8 16:23:39 2016 +0900
> 
>     drm: Add DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags v2
> 
> Michel, can you pls take care of this? Either with a minimal fix, or
> by adopting David's patch?

Can't we just use David's patch as-is? If not, I think Dmitry or someone
else would be better equipped than me to extract a minimal fix from it
and test it.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: drm: GPF in drm_getcap
  2016-11-28  7:14               ` Michel Dänzer
@ 2016-11-28  8:41                 ` Dmitry Vyukov
  2016-11-30  8:30                   ` [PATCH 1/2] drm: Don't call drm_for_each_crtc with a non-KMS driver Michel Dänzer
  0 siblings, 1 reply; 21+ messages in thread
From: Dmitry Vyukov @ 2016-11-28  8:41 UTC (permalink / raw)
  To: Michel Dänzer
  Cc: Daniel Vetter, David Herrmann, syzkaller, David Airlie,
	dri-devel@lists.freedesktop.org, LKML

On Mon, Nov 28, 2016 at 8:14 AM, Michel Dänzer <michel@daenzer.net> wrote:
> On 28/11/16 03:55 PM, Daniel Vetter wrote:
>> On Sat, Nov 26, 2016 at 7:22 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
>>> On Sat, Nov 26, 2016 at 7:07 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>>> grep "card0" dmesg:
>>>> [    5.298617] device: 'card0': device_add
>>>> [    5.298946] PM: Adding info for No Bus:card0
>>>> [    6.436178] device: 'card0': device_add
>>>> [    6.436488] PM: Adding info for No Bus:card0
>>>>
>>>>
>>>> # ls -l /dev/dri/card0
>>>> crw-rw---T 1 root video 226, 0 Nov 26 18:05 /dev/dri/card0
>>>>
>>>> # ls -lt /sys/class/drm/card0/device/
>>>> ls: cannot access /sys/class/drm/card0/device/: No such file or directory
>>>>
>>>> # ls -lt /sys/class/drm/card0/device/driver
>>>> ls: cannot access /sys/class/drm/card0/device/driver: No such file or directory
>>>
>>> Looks like vgem. Something like this should help:
>>>
>>>     https://gist.github.com/dvdhrm/1bcdf4f3485aa1614a0198a7b90515e2
>>>
>>> I wonder whether it would be more appropriate to return -ENOTSUPP rather than 0.
>
> Can't see how that would matter FWIW.
>
>
>> Seems a bit overkill, but can't hurt. This is most likely a
>> regression, probably introduced in
>>
>> commit f837297ad82480024d3ad08cd84f6670bcafa862
>> Author: Michel Dänzer <michel.daenzer@amd.com>
>> Date:   Mon Aug 8 16:23:39 2016 +0900
>>
>>     drm: Add DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags v2
>>
>> Michel, can you pls take care of this? Either with a minimal fix, or
>> by adopting David's patch?
>
> Can't we just use David's patch as-is? If not, I think Dmitry or someone
> else would be better equipped than me to extract a minimal fix from it
> and test it.


I know nothing about DRM code. Reproducer is attached to the first email.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/2] drm: Don't call drm_for_each_crtc with a non-KMS driver
  2016-11-28  8:41                 ` Dmitry Vyukov
@ 2016-11-30  8:30                   ` Michel Dänzer
  2016-11-30  8:30                     ` [PATCH 2/2] drm: Return -ENOTSUPP when called for KMS cap " Michel Dänzer
  2016-11-30  9:13                     ` [PATCH 1/2] drm: Don't call drm_for_each_crtc " Daniel Vetter
  0 siblings, 2 replies; 21+ messages in thread
From: Michel Dänzer @ 2016-11-30  8:30 UTC (permalink / raw)
  To: dri-devel; +Cc: Dmitry Vyukov

From: Michel Dänzer <michel.daenzer@amd.com>

Fixes oops if userspace calls DRM_IOCTL_GET_CAP for
 DRM_CAP_PAGE_FLIP_TARGET on a non-KMS device node. (Normal userspace
doesn't do that, discovered by syzkaller)

Reported-by: Dmitry Vyukov <dvyukov@google.com>
Fixes: f837297ad824 ("drm: Add DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags v2")
Cc: stable@vger.kernel.org
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/drm_ioctl.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 0ad2c47..71c3473 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -254,10 +254,12 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
 		req->value = dev->mode_config.async_page_flip;
 		break;
 	case DRM_CAP_PAGE_FLIP_TARGET:
-		req->value = 1;
-		drm_for_each_crtc(crtc, dev) {
-			if (!crtc->funcs->page_flip_target)
-				req->value = 0;
+		if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+			req->value = 1;
+			drm_for_each_crtc(crtc, dev) {
+				if (!crtc->funcs->page_flip_target)
+					req->value = 0;
+			}
 		}
 		break;
 	case DRM_CAP_CURSOR_WIDTH:
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 2/2] drm: Return -ENOTSUPP when called for KMS cap with a non-KMS driver
  2016-11-30  8:30                   ` [PATCH 1/2] drm: Don't call drm_for_each_crtc with a non-KMS driver Michel Dänzer
@ 2016-11-30  8:30                     ` Michel Dänzer
  2016-11-30  9:07                       ` Daniel Vetter
  2016-11-30  9:13                     ` [PATCH 1/2] drm: Don't call drm_for_each_crtc " Daniel Vetter
  1 sibling, 1 reply; 21+ messages in thread
From: Michel Dänzer @ 2016-11-30  8:30 UTC (permalink / raw)
  To: dri-devel; +Cc: Dmitry Vyukov

From: Michel Dänzer <michel.daenzer@amd.com>

This is an attempt to make the previous fix a bit more robust going
forward.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/drm_ioctl.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 71c3473..32f484b 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -229,6 +229,19 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
 	struct drm_crtc *crtc;
 
 	req->value = 0;
+
+	/* Only allow non-KMS caps with non-KMS drivers */
+	switch (req->capability) {
+	case DRM_CAP_DUMB_BUFFER:
+	case DRM_CAP_VBLANK_HIGH_CRTC:
+	case DRM_CAP_PRIME:
+	case DRM_CAP_TIMESTAMP_MONOTONIC:
+		break;
+	default:
+		if (!drm_core_check_feature(dev, DRIVER_MODESET))
+			return -ENOTSUPP;
+	}
+
 	switch (req->capability) {
 	case DRM_CAP_DUMB_BUFFER:
 		if (dev->driver->dumb_create)
@@ -254,12 +267,10 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
 		req->value = dev->mode_config.async_page_flip;
 		break;
 	case DRM_CAP_PAGE_FLIP_TARGET:
-		if (drm_core_check_feature(dev, DRIVER_MODESET)) {
-			req->value = 1;
-			drm_for_each_crtc(crtc, dev) {
-				if (!crtc->funcs->page_flip_target)
-					req->value = 0;
-			}
+		req->value = 1;
+		drm_for_each_crtc(crtc, dev) {
+			if (!crtc->funcs->page_flip_target)
+				req->value = 0;
 		}
 		break;
 	case DRM_CAP_CURSOR_WIDTH:
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2] drm: Return -ENOTSUPP when called for KMS cap with a non-KMS driver
  2016-11-30  8:30                     ` [PATCH 2/2] drm: Return -ENOTSUPP when called for KMS cap " Michel Dänzer
@ 2016-11-30  9:07                       ` Daniel Vetter
  2016-11-30 17:21                         ` Alex Deucher
                                           ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Daniel Vetter @ 2016-11-30  9:07 UTC (permalink / raw)
  To: Michel Dänzer; +Cc: Dmitry Vyukov, dri-devel

On Wed, Nov 30, 2016 at 05:30:02PM +0900, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
> 
> This is an attempt to make the previous fix a bit more robust going
> forward.
> 
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 71c3473..32f484b 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -229,6 +229,19 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>  	struct drm_crtc *crtc;
>  
>  	req->value = 0;
> +
> +	/* Only allow non-KMS caps with non-KMS drivers */
> +	switch (req->capability) {
> +	case DRM_CAP_DUMB_BUFFER:

Dumb buffers are only meant to be used for kms drivers, should be
disallowed too.

> +	case DRM_CAP_VBLANK_HIGH_CRTC:

Might be good to have a comment here that we need to allow this for old
ums?

> +	case DRM_CAP_PRIME:
> +	case DRM_CAP_TIMESTAMP_MONOTONIC:

This is pretty new, I don't think any of the old ums drivers was ever
updated to use it. Should probably disallow it too.
> +		break;
> +	default:
> +		if (!drm_core_check_feature(dev, DRIVER_MODESET))
> +			return -ENOTSUPP;
> +	}

And one code org bikeshed: I don't like the duplicated switch, could we
instead split it it into two disjoint sets like this?

	switch (req->capability) {
	case DRM_CAP_PRIME:
		req->value |= dev->driver->prime_fd_to_handle ? DRM_PRIME_CAP_IMPORT : 0;
		req->value |= dev->driver->prime_handle_to_fd ? DRM_PRIME_CAP_EXPORT : 0;
		break;
	... all other non-modeset caps ...
	}

	if (!drm_core_check_feature(dev, DRIVER_MODESET))
		return -ENOTSUPP;

	switch (req->capability) {
	... handle remaining caps needed for DRIVER_MODSET ...
	default:
		return -EINVAL;
	}

That way it would be a bit more obvious that people who add a new cap need
to make a decision where to put it (and by default put it in the bottom
pile).
-Daniel

>  	switch (req->capability) {
>  	case DRM_CAP_DUMB_BUFFER:
>  		if (dev->driver->dumb_create)
> @@ -254,12 +267,10 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>  		req->value = dev->mode_config.async_page_flip;
>  		break;
>  	case DRM_CAP_PAGE_FLIP_TARGET:
> -		if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> -			req->value = 1;
> -			drm_for_each_crtc(crtc, dev) {
> -				if (!crtc->funcs->page_flip_target)
> -					req->value = 0;
> -			}
> +		req->value = 1;
> +		drm_for_each_crtc(crtc, dev) {
> +			if (!crtc->funcs->page_flip_target)
> +				req->value = 0;
>  		}
>  		break;
>  	case DRM_CAP_CURSOR_WIDTH:
> -- 
> 2.10.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/2] drm: Don't call drm_for_each_crtc with a non-KMS driver
  2016-11-30  8:30                   ` [PATCH 1/2] drm: Don't call drm_for_each_crtc with a non-KMS driver Michel Dänzer
  2016-11-30  8:30                     ` [PATCH 2/2] drm: Return -ENOTSUPP when called for KMS cap " Michel Dänzer
@ 2016-11-30  9:13                     ` Daniel Vetter
  1 sibling, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2016-11-30  9:13 UTC (permalink / raw)
  To: Michel Dänzer; +Cc: Dmitry Vyukov, dri-devel

On Wed, Nov 30, 2016 at 05:30:01PM +0900, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
> 
> Fixes oops if userspace calls DRM_IOCTL_GET_CAP for
>  DRM_CAP_PAGE_FLIP_TARGET on a non-KMS device node. (Normal userspace
> doesn't do that, discovered by syzkaller)
> 
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Fixes: f837297ad824 ("drm: Add DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags v2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>

Applied to drm-misc-fixes, thanks.
-Daniel

> ---
>  drivers/gpu/drm/drm_ioctl.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 0ad2c47..71c3473 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -254,10 +254,12 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>  		req->value = dev->mode_config.async_page_flip;
>  		break;
>  	case DRM_CAP_PAGE_FLIP_TARGET:
> -		req->value = 1;
> -		drm_for_each_crtc(crtc, dev) {
> -			if (!crtc->funcs->page_flip_target)
> -				req->value = 0;
> +		if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> +			req->value = 1;
> +			drm_for_each_crtc(crtc, dev) {
> +				if (!crtc->funcs->page_flip_target)
> +					req->value = 0;
> +			}
>  		}
>  		break;
>  	case DRM_CAP_CURSOR_WIDTH:
> -- 
> 2.10.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2] drm: Return -ENOTSUPP when called for KMS cap with a non-KMS driver
  2016-11-30  9:07                       ` Daniel Vetter
@ 2016-11-30 17:21                         ` Alex Deucher
  2016-12-01  7:35                         ` Michel Dänzer
  2016-12-01  7:37                         ` [PATCH v2] " Michel Dänzer
  2 siblings, 0 replies; 21+ messages in thread
From: Alex Deucher @ 2016-11-30 17:21 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Michel Dänzer, Maling list - DRI developers, Dmitry Vyukov

On Wed, Nov 30, 2016 at 4:07 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Nov 30, 2016 at 05:30:02PM +0900, Michel Dänzer wrote:
>> From: Michel Dänzer <michel.daenzer@amd.com>
>>
>> This is an attempt to make the previous fix a bit more robust going
>> forward.
>>
>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>> ---
>>  drivers/gpu/drm/drm_ioctl.c | 23 +++++++++++++++++------
>>  1 file changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
>> index 71c3473..32f484b 100644
>> --- a/drivers/gpu/drm/drm_ioctl.c
>> +++ b/drivers/gpu/drm/drm_ioctl.c
>> @@ -229,6 +229,19 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>>       struct drm_crtc *crtc;
>>
>>       req->value = 0;
>> +
>> +     /* Only allow non-KMS caps with non-KMS drivers */
>> +     switch (req->capability) {
>> +     case DRM_CAP_DUMB_BUFFER:
>
> Dumb buffers are only meant to be used for kms drivers, should be
> disallowed too.
>
>> +     case DRM_CAP_VBLANK_HIGH_CRTC:
>
> Might be good to have a comment here that we need to allow this for old
> ums?
>

I don't think we need this for UMS.  It was added for evergreen and we
only supported this feature on KMS.

Alex

>> +     case DRM_CAP_PRIME:
>> +     case DRM_CAP_TIMESTAMP_MONOTONIC:
>
> This is pretty new, I don't think any of the old ums drivers was ever
> updated to use it. Should probably disallow it too.
>> +             break;
>> +     default:
>> +             if (!drm_core_check_feature(dev, DRIVER_MODESET))
>> +                     return -ENOTSUPP;
>> +     }
>
> And one code org bikeshed: I don't like the duplicated switch, could we
> instead split it it into two disjoint sets like this?
>
>         switch (req->capability) {
>         case DRM_CAP_PRIME:
>                 req->value |= dev->driver->prime_fd_to_handle ? DRM_PRIME_CAP_IMPORT : 0;
>                 req->value |= dev->driver->prime_handle_to_fd ? DRM_PRIME_CAP_EXPORT : 0;
>                 break;
>         ... all other non-modeset caps ...
>         }
>
>         if (!drm_core_check_feature(dev, DRIVER_MODESET))
>                 return -ENOTSUPP;
>
>         switch (req->capability) {
>         ... handle remaining caps needed for DRIVER_MODSET ...
>         default:
>                 return -EINVAL;
>         }
>
> That way it would be a bit more obvious that people who add a new cap need
> to make a decision where to put it (and by default put it in the bottom
> pile).
> -Daniel
>
>>       switch (req->capability) {
>>       case DRM_CAP_DUMB_BUFFER:
>>               if (dev->driver->dumb_create)
>> @@ -254,12 +267,10 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>>               req->value = dev->mode_config.async_page_flip;
>>               break;
>>       case DRM_CAP_PAGE_FLIP_TARGET:
>> -             if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>> -                     req->value = 1;
>> -                     drm_for_each_crtc(crtc, dev) {
>> -                             if (!crtc->funcs->page_flip_target)
>> -                                     req->value = 0;
>> -                     }
>> +             req->value = 1;
>> +             drm_for_each_crtc(crtc, dev) {
>> +                     if (!crtc->funcs->page_flip_target)
>> +                             req->value = 0;
>>               }
>>               break;
>>       case DRM_CAP_CURSOR_WIDTH:
>> --
>> 2.10.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2] drm: Return -ENOTSUPP when called for KMS cap with a non-KMS driver
  2016-11-30  9:07                       ` Daniel Vetter
  2016-11-30 17:21                         ` Alex Deucher
@ 2016-12-01  7:35                         ` Michel Dänzer
  2016-12-01  7:37                         ` [PATCH v2] " Michel Dänzer
  2 siblings, 0 replies; 21+ messages in thread
From: Michel Dänzer @ 2016-12-01  7:35 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel, Dmitry Vyukov

On 30/11/16 06:07 PM, Daniel Vetter wrote:
> On Wed, Nov 30, 2016 at 05:30:02PM +0900, Michel Dänzer wrote:
>> From: Michel Dänzer <michel.daenzer@amd.com>
>>
>> This is an attempt to make the previous fix a bit more robust going
>> forward.
>>
>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>> ---
>>  drivers/gpu/drm/drm_ioctl.c | 23 +++++++++++++++++------
>>  1 file changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
>> index 71c3473..32f484b 100644
>> --- a/drivers/gpu/drm/drm_ioctl.c
>> +++ b/drivers/gpu/drm/drm_ioctl.c
>> @@ -229,6 +229,19 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>>  	struct drm_crtc *crtc;
>>  
>>  	req->value = 0;
>> +
>> +	/* Only allow non-KMS caps with non-KMS drivers */
>> +	switch (req->capability) {
>> +	case DRM_CAP_DUMB_BUFFER:
> 
> Dumb buffers are only meant to be used for kms drivers, should be
> disallowed too.
> 
>> +	case DRM_CAP_VBLANK_HIGH_CRTC:
> 
> Might be good to have a comment here that we need to allow this for old
> ums?

This is effectively KMS-only as well, per Alex (thanks!).


>> +	case DRM_CAP_PRIME:
>> +	case DRM_CAP_TIMESTAMP_MONOTONIC:
> 
> This is pretty new, I don't think any of the old ums drivers was ever
> updated to use it. Should probably disallow it too.

DRM_CAP_TIMESTAMP_MONOTONIC is driver independent, I don't see why it
wouldn't work fine with UMS drivers. OTOH, I don't think DRM_CAP_PRIME
can work with UMS.


>> +		break;
>> +	default:
>> +		if (!drm_core_check_feature(dev, DRIVER_MODESET))
>> +			return -ENOTSUPP;
>> +	}
> 
> And one code org bikeshed: I don't like the duplicated switch, could we
> instead split it it into two disjoint sets like this?
> 
> 	switch (req->capability) {
> 	case DRM_CAP_PRIME:
> 		req->value |= dev->driver->prime_fd_to_handle ? DRM_PRIME_CAP_IMPORT : 0;
> 		req->value |= dev->driver->prime_handle_to_fd ? DRM_PRIME_CAP_EXPORT : 0;
> 		break;
> 	... all other non-modeset caps ...
> 	}
> 
> 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
> 		return -ENOTSUPP;
> 
> 	switch (req->capability) {
> 	... handle remaining caps needed for DRIVER_MODSET ...
> 	default:
> 		return -EINVAL;
> 	}
> 
> That way it would be a bit more obvious that people who add a new cap need
> to make a decision where to put it (and by default put it in the bottom
> pile).

Your pseudo-code wouldn't work correctly, but I get your idea. :)


v2 addressing review feedback coming up soon.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2] drm: Return -ENOTSUPP when called for KMS cap with a non-KMS driver
  2016-11-30  9:07                       ` Daniel Vetter
  2016-11-30 17:21                         ` Alex Deucher
  2016-12-01  7:35                         ` Michel Dänzer
@ 2016-12-01  7:37                         ` Michel Dänzer
  2016-12-01 14:46                           ` Alex Deucher
  2016-12-01 15:21                           ` Sean Paul
  2 siblings, 2 replies; 21+ messages in thread
From: Michel Dänzer @ 2016-12-01  7:37 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Dmitry Vyukov, dri-devel

From: Michel Dänzer <michel.daenzer@amd.com>

This is an attempt to make the previous fix a bit more robust going
forward.

v2:
* Only allow DRM_CAP_TIMESTAMP_MONOTONIC with UMS drivers (Daniel
  Vetter, Alex Deucher)
* Different logic to keep DRM_CAP_TIMESTAMP_MONOTONIC separate from
  the other caps (Daniel Vetter)

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/drm_ioctl.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 71c3473..706d5aa 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -229,6 +229,17 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
 	struct drm_crtc *crtc;
 
 	req->value = 0;
+
+	/* Only one cap makes sense with a UMS driver: */
+	if (req->capability == DRM_CAP_TIMESTAMP_MONOTONIC) {
+		req->value = drm_timestamp_monotonic;
+		return 0;
+	}
+
+	/* Other caps only work with KMS drivers */
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		return -ENOTSUPP;
+
 	switch (req->capability) {
 	case DRM_CAP_DUMB_BUFFER:
 		if (dev->driver->dumb_create)
@@ -247,19 +258,14 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
 		req->value |= dev->driver->prime_fd_to_handle ? DRM_PRIME_CAP_IMPORT : 0;
 		req->value |= dev->driver->prime_handle_to_fd ? DRM_PRIME_CAP_EXPORT : 0;
 		break;
-	case DRM_CAP_TIMESTAMP_MONOTONIC:
-		req->value = drm_timestamp_monotonic;
-		break;
 	case DRM_CAP_ASYNC_PAGE_FLIP:
 		req->value = dev->mode_config.async_page_flip;
 		break;
 	case DRM_CAP_PAGE_FLIP_TARGET:
-		if (drm_core_check_feature(dev, DRIVER_MODESET)) {
-			req->value = 1;
-			drm_for_each_crtc(crtc, dev) {
-				if (!crtc->funcs->page_flip_target)
-					req->value = 0;
-			}
+		req->value = 1;
+		drm_for_each_crtc(crtc, dev) {
+			if (!crtc->funcs->page_flip_target)
+				req->value = 0;
 		}
 		break;
 	case DRM_CAP_CURSOR_WIDTH:
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] drm: Return -ENOTSUPP when called for KMS cap with a non-KMS driver
  2016-12-01  7:37                         ` [PATCH v2] " Michel Dänzer
@ 2016-12-01 14:46                           ` Alex Deucher
  2016-12-05  8:05                             ` Daniel Vetter
  2016-12-01 15:21                           ` Sean Paul
  1 sibling, 1 reply; 21+ messages in thread
From: Alex Deucher @ 2016-12-01 14:46 UTC (permalink / raw)
  To: Michel Dänzer; +Cc: Maling list - DRI developers, Dmitry Vyukov

On Thu, Dec 1, 2016 at 2:37 AM, Michel Dänzer <michel@daenzer.net> wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> This is an attempt to make the previous fix a bit more robust going
> forward.
>
> v2:
> * Only allow DRM_CAP_TIMESTAMP_MONOTONIC with UMS drivers (Daniel
>   Vetter, Alex Deucher)
> * Different logic to keep DRM_CAP_TIMESTAMP_MONOTONIC separate from
>   the other caps (Daniel Vetter)
>
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/drm_ioctl.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 71c3473..706d5aa 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -229,6 +229,17 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>         struct drm_crtc *crtc;
>
>         req->value = 0;
> +
> +       /* Only one cap makes sense with a UMS driver: */
> +       if (req->capability == DRM_CAP_TIMESTAMP_MONOTONIC) {
> +               req->value = drm_timestamp_monotonic;
> +               return 0;
> +       }
> +
> +       /* Other caps only work with KMS drivers */
> +       if (!drm_core_check_feature(dev, DRIVER_MODESET))
> +               return -ENOTSUPP;
> +
>         switch (req->capability) {
>         case DRM_CAP_DUMB_BUFFER:
>                 if (dev->driver->dumb_create)
> @@ -247,19 +258,14 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>                 req->value |= dev->driver->prime_fd_to_handle ? DRM_PRIME_CAP_IMPORT : 0;
>                 req->value |= dev->driver->prime_handle_to_fd ? DRM_PRIME_CAP_EXPORT : 0;
>                 break;
> -       case DRM_CAP_TIMESTAMP_MONOTONIC:
> -               req->value = drm_timestamp_monotonic;
> -               break;
>         case DRM_CAP_ASYNC_PAGE_FLIP:
>                 req->value = dev->mode_config.async_page_flip;
>                 break;
>         case DRM_CAP_PAGE_FLIP_TARGET:
> -               if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> -                       req->value = 1;
> -                       drm_for_each_crtc(crtc, dev) {
> -                               if (!crtc->funcs->page_flip_target)
> -                                       req->value = 0;
> -                       }
> +               req->value = 1;
> +               drm_for_each_crtc(crtc, dev) {
> +                       if (!crtc->funcs->page_flip_target)
> +                               req->value = 0;
>                 }
>                 break;
>         case DRM_CAP_CURSOR_WIDTH:
> --
> 2.10.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] drm: Return -ENOTSUPP when called for KMS cap with a non-KMS driver
  2016-12-01  7:37                         ` [PATCH v2] " Michel Dänzer
  2016-12-01 14:46                           ` Alex Deucher
@ 2016-12-01 15:21                           ` Sean Paul
  2016-12-01 15:23                             ` Daniel Vetter
  1 sibling, 1 reply; 21+ messages in thread
From: Sean Paul @ 2016-12-01 15:21 UTC (permalink / raw)
  To: Michel Dänzer; +Cc: dri-devel, Dmitry Vyukov

On Thu, Dec 1, 2016 at 2:37 AM, Michel Dänzer <michel@daenzer.net> wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> This is an attempt to make the previous fix a bit more robust going
> forward.

It takes a bit of work to locate the "previous fix" now that it's gone
through drm-misc-fixes. Can you update with a proper reference to the
first patch?

Additionally, it's probably easiest if we hold off on merging this
until your first patch finds its way into drm-misc-next.

Thanks,

Sean


>
> v2:
> * Only allow DRM_CAP_TIMESTAMP_MONOTONIC with UMS drivers (Daniel
>   Vetter, Alex Deucher)
> * Different logic to keep DRM_CAP_TIMESTAMP_MONOTONIC separate from
>   the other caps (Daniel Vetter)
>
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 71c3473..706d5aa 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -229,6 +229,17 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>         struct drm_crtc *crtc;
>
>         req->value = 0;
> +
> +       /* Only one cap makes sense with a UMS driver: */
> +       if (req->capability == DRM_CAP_TIMESTAMP_MONOTONIC) {
> +               req->value = drm_timestamp_monotonic;
> +               return 0;
> +       }
> +
> +       /* Other caps only work with KMS drivers */
> +       if (!drm_core_check_feature(dev, DRIVER_MODESET))
> +               return -ENOTSUPP;
> +
>         switch (req->capability) {
>         case DRM_CAP_DUMB_BUFFER:
>                 if (dev->driver->dumb_create)
> @@ -247,19 +258,14 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>                 req->value |= dev->driver->prime_fd_to_handle ? DRM_PRIME_CAP_IMPORT : 0;
>                 req->value |= dev->driver->prime_handle_to_fd ? DRM_PRIME_CAP_EXPORT : 0;
>                 break;
> -       case DRM_CAP_TIMESTAMP_MONOTONIC:
> -               req->value = drm_timestamp_monotonic;
> -               break;
>         case DRM_CAP_ASYNC_PAGE_FLIP:
>                 req->value = dev->mode_config.async_page_flip;
>                 break;
>         case DRM_CAP_PAGE_FLIP_TARGET:
> -               if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> -                       req->value = 1;
> -                       drm_for_each_crtc(crtc, dev) {
> -                               if (!crtc->funcs->page_flip_target)
> -                                       req->value = 0;
> -                       }
> +               req->value = 1;
> +               drm_for_each_crtc(crtc, dev) {
> +                       if (!crtc->funcs->page_flip_target)
> +                               req->value = 0;
>                 }
>                 break;
>         case DRM_CAP_CURSOR_WIDTH:
> --
> 2.10.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] drm: Return -ENOTSUPP when called for KMS cap with a non-KMS driver
  2016-12-01 15:21                           ` Sean Paul
@ 2016-12-01 15:23                             ` Daniel Vetter
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2016-12-01 15:23 UTC (permalink / raw)
  To: Sean Paul; +Cc: dri-devel, Michel Dänzer, Dmitry Vyukov

On Thu, Dec 01, 2016 at 10:21:28AM -0500, Sean Paul wrote:
> On Thu, Dec 1, 2016 at 2:37 AM, Michel Dänzer <michel@daenzer.net> wrote:
> > From: Michel Dänzer <michel.daenzer@amd.com>
> >
> > This is an attempt to make the previous fix a bit more robust going
> > forward.
> 
> It takes a bit of work to locate the "previous fix" now that it's gone
> through drm-misc-fixes. Can you update with a proper reference to the
> first patch?
> 
> Additionally, it's probably easiest if we hold off on merging this
> until your first patch finds its way into drm-misc-next.

I'll bother Dave for a backmerge as soon as 4.9/4.9-rc8 is out, and then
we can apply Michel's cleanup on top.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] drm: Return -ENOTSUPP when called for KMS cap with a non-KMS driver
  2016-12-01 14:46                           ` Alex Deucher
@ 2016-12-05  8:05                             ` Daniel Vetter
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2016-12-05  8:05 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Maling list - DRI developers, Michel Dänzer, Dmitry Vyukov

On Thu, Dec 01, 2016 at 09:46:06AM -0500, Alex Deucher wrote:
> On Thu, Dec 1, 2016 at 2:37 AM, Michel Dänzer <michel@daenzer.net> wrote:
> > From: Michel Dänzer <michel.daenzer@amd.com>
> >
> > This is an attempt to make the previous fix a bit more robust going
> > forward.
> >
> > v2:
> > * Only allow DRM_CAP_TIMESTAMP_MONOTONIC with UMS drivers (Daniel
> >   Vetter, Alex Deucher)
> > * Different logic to keep DRM_CAP_TIMESTAMP_MONOTONIC separate from
> >   the other caps (Daniel Vetter)
> >
> > Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> 
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Applied to drm-misc, thanks.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/drm_ioctl.c | 24 +++++++++++++++---------
> >  1 file changed, 15 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > index 71c3473..706d5aa 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -229,6 +229,17 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
> >         struct drm_crtc *crtc;
> >
> >         req->value = 0;
> > +
> > +       /* Only one cap makes sense with a UMS driver: */
> > +       if (req->capability == DRM_CAP_TIMESTAMP_MONOTONIC) {
> > +               req->value = drm_timestamp_monotonic;
> > +               return 0;
> > +       }
> > +
> > +       /* Other caps only work with KMS drivers */
> > +       if (!drm_core_check_feature(dev, DRIVER_MODESET))
> > +               return -ENOTSUPP;
> > +
> >         switch (req->capability) {
> >         case DRM_CAP_DUMB_BUFFER:
> >                 if (dev->driver->dumb_create)
> > @@ -247,19 +258,14 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
> >                 req->value |= dev->driver->prime_fd_to_handle ? DRM_PRIME_CAP_IMPORT : 0;
> >                 req->value |= dev->driver->prime_handle_to_fd ? DRM_PRIME_CAP_EXPORT : 0;
> >                 break;
> > -       case DRM_CAP_TIMESTAMP_MONOTONIC:
> > -               req->value = drm_timestamp_monotonic;
> > -               break;
> >         case DRM_CAP_ASYNC_PAGE_FLIP:
> >                 req->value = dev->mode_config.async_page_flip;
> >                 break;
> >         case DRM_CAP_PAGE_FLIP_TARGET:
> > -               if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> > -                       req->value = 1;
> > -                       drm_for_each_crtc(crtc, dev) {
> > -                               if (!crtc->funcs->page_flip_target)
> > -                                       req->value = 0;
> > -                       }
> > +               req->value = 1;
> > +               drm_for_each_crtc(crtc, dev) {
> > +                       if (!crtc->funcs->page_flip_target)
> > +                               req->value = 0;
> >                 }
> >                 break;
> >         case DRM_CAP_CURSOR_WIDTH:
> > --
> > 2.10.2
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2016-12-05  8:05 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-09 11:56 drm: GPF in drm_getcap Dmitry Vyukov
2016-11-26 17:17 ` Dmitry Vyukov
2016-11-26 17:35   ` David Herrmann
2016-11-26 17:50     ` Dmitry Vyukov
2016-11-26 18:02       ` David Herrmann
2016-11-26 18:07         ` Dmitry Vyukov
2016-11-26 18:22           ` David Herrmann
2016-11-28  6:55             ` Daniel Vetter
2016-11-28  7:14               ` Michel Dänzer
2016-11-28  8:41                 ` Dmitry Vyukov
2016-11-30  8:30                   ` [PATCH 1/2] drm: Don't call drm_for_each_crtc with a non-KMS driver Michel Dänzer
2016-11-30  8:30                     ` [PATCH 2/2] drm: Return -ENOTSUPP when called for KMS cap " Michel Dänzer
2016-11-30  9:07                       ` Daniel Vetter
2016-11-30 17:21                         ` Alex Deucher
2016-12-01  7:35                         ` Michel Dänzer
2016-12-01  7:37                         ` [PATCH v2] " Michel Dänzer
2016-12-01 14:46                           ` Alex Deucher
2016-12-05  8:05                             ` Daniel Vetter
2016-12-01 15:21                           ` Sean Paul
2016-12-01 15:23                             ` Daniel Vetter
2016-11-30  9:13                     ` [PATCH 1/2] drm: Don't call drm_for_each_crtc " Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).