[PATCH] drm: qxl: Don't alloc fbdev if emulation is not supported

[PATCH] drm: qxl: Don't alloc fbdev if emulation is not supported

[Gabriel Krisman Bertazi]

If fbdev emulation is disabled, the QXL shutdown path will try to clean
a framebuffer that wasn't initialized, hitting the Oops below.  The
problem is that even when FBDEV_EMULATION is disabled we allocate the
qfbdev strutucture, but we don't initialize it.  The fix is to stop
allocating the memory, since it won't be used.  This allows the existing
verification in the cleanup hook to do it's job preventing the oops.

Now that we don't allocate the unused fbdev structure, we need to be
careful when dereferencing it in the PM suspend hook.

[   24.284684] BUG: unable to handle kernel NULL pointer dereference at 00000000000002e0
[   24.285627] IP: mutex_lock+0x18/0x30
[   24.286049] PGD 78cdf067
[   24.286050] PUD 7940f067
[   24.286344] PMD 0
[   24.286649]
[   24.287072] Oops: 0002 [#1] SMP
[   24.287422] Modules linked in: qxl
[   24.287806] CPU: 0 PID: 2328 Comm: bash Not tainted 4.10.0-rc5+ #97
[   24.288515] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.9.3-20161025_171302-gandalf 04/01/2014
[   24.289681] task: ffff88007c4c0000 task.stack: ffffc90001b58000
[   24.290354] RIP: 0010:mutex_lock+0x18/0x30
[   24.290812] RSP: 0018:ffffc90001b5bcb0 EFLAGS: 00010246
[   24.291401] RAX: 0000000000000000 RBX: 00000000000002e0 RCX: 0000000000000000
[   24.292209] RDX: ffff88007c4c0000 RSI: 0000000000000001 RDI: 00000000000002e0
[   24.292987] RBP: ffffc90001b5bcb8 R08: fffffffffffffffe R09: 0000000000000001
[   24.293797] R10: ffff880078d80b80 R11: 0000000000011400 R12: 0000000000000000
[   24.294601] R13: 00000000000002e0 R14: ffffffffa0009c28 R15: 0000000000000060
[   24.295439] FS:  00007f30e3acbb40(0000) GS:ffff88007fc00000(0000) knlGS:0000000000000000
[   24.296364] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   24.296997] CR2: 00000000000002e0 CR3: 0000000078c7b000 CR4: 00000000000006f0
[   24.297813] Call Trace:
[   24.298097]  drm_framebuffer_cleanup+0x1f/0x70
[   24.298612]  qxl_fbdev_fini+0x68/0x90 [qxl]
[   24.299074]  qxl_modeset_fini+0xd/0x30 [qxl]
[   24.299562]  qxl_pci_remove+0x22/0x50 [qxl]
[   24.300025]  pci_device_remove+0x34/0xb0
[   24.300507]  device_release_driver_internal+0x150/0x200
[   24.301082]  device_release_driver+0xd/0x10
[   24.301587]  unbind_store+0x108/0x150
[   24.301993]  drv_attr_store+0x20/0x30
[   24.302402]  sysfs_kf_write+0x32/0x40
[   24.302827]  kernfs_fop_write+0x108/0x190
[   24.303269]  __vfs_write+0x23/0x120
[   24.303678]  ? security_file_permission+0x36/0xb0
[   24.304193]  ? rw_verify_area+0x49/0xb0
[   24.304636]  vfs_write+0xb0/0x190
[   24.305004]  SyS_write+0x41/0xa0
[   24.305362]  entry_SYSCALL_64_fastpath+0x1a/0xa9
[   24.305887] RIP: 0033:0x7f30e31d9620
[   24.306285] RSP: 002b:00007ffc54b47e68 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[   24.307128] RAX: ffffffffffffffda RBX: 00007f30e3497600 RCX: 00007f30e31d9620
[   24.307928] RDX: 000000000000000d RSI: 0000000000da2008 RDI: 0000000000000001
[   24.308727] RBP: 000000000070bc60 R08: 00007f30e3498760 R09: 00007f30e3acbb40
[   24.309504] R10: 0000000000000073 R11: 0000000000000246 R12: 0000000000000001
[   24.310295] R13: 0000000000000000 R14: 0000000000000000 R15: 00007ffc54b47f34
[   24.311095] Code: 0e 01 e9 7b fe ff ff 66 90 66 2e 0f 1f 84 00 00 00 00 00
55 48 89 e5 53 48 89 fb e8 83 e8 ff ff 65 48 8b 14 25 40 c4 00 00 31 c0 <3e>
48 0f b1 13 48 85 c0 74 08 48 89 df e8 66 fd ff ff 5b 5d c3
[   24.313182] RIP: mutex_lock+0x18/0x30 RSP: ffffc90001b5bcb0
[   24.313811] CR2: 00000000000002e0
[   24.314208] ---[ end trace 29669c1593cae14b ]---

Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
---
 drivers/gpu/drm/qxl/qxl_fb.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c
index 35124737666e..14e2a49a4dcf 100644
--- a/drivers/gpu/drm/qxl/qxl_fb.c
+++ b/drivers/gpu/drm/qxl/qxl_fb.c
@@ -368,9 +368,11 @@ static const struct drm_fb_helper_funcs qxl_fb_helper_funcs = {
 
 int qxl_fbdev_init(struct qxl_device *qdev)
 {
+	int ret = 0;
+
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 	struct qxl_fbdev *qfbdev;
 	int bpp_sel = 32; /* TODO: parameter from somewhere? */
-	int ret;
 
 	qfbdev = kzalloc(sizeof(struct qxl_fbdev), GFP_KERNEL);
 	if (!qfbdev)
@@ -403,6 +405,8 @@ int qxl_fbdev_init(struct qxl_device *qdev)
 	drm_fb_helper_fini(&qfbdev->helper);
 free:
 	kfree(qfbdev);
+#endif
+
 	return ret;
 }
 
@@ -418,6 +422,9 @@ void qxl_fbdev_fini(struct qxl_device *qdev)
 
 void qxl_fbdev_set_suspend(struct qxl_device *qdev, int state)
 {
+	if (!qdev->mode_info.qfbdev)
+		return;
+
 	drm_fb_helper_set_suspend(&qdev->mode_info.qfbdev->helper, state);
 }
 
-- 
2.11.0

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

Re: [PATCH] drm: qxl: Don't alloc fbdev if emulation is not supported

[Daniel Vetter]

On Mon, Feb 27, 2017 at 05:33:30PM -0300, Gabriel Krisman Bertazi wrote:
> If fbdev emulation is disabled, the QXL shutdown path will try to clean
> a framebuffer that wasn't initialized, hitting the Oops below.  The
> problem is that even when FBDEV_EMULATION is disabled we allocate the
> qfbdev strutucture, but we don't initialize it.  The fix is to stop
> allocating the memory, since it won't be used.  This allows the existing
> verification in the cleanup hook to do it's job preventing the oops.
> 
> Now that we don't allocate the unused fbdev structure, we need to be
> careful when dereferencing it in the PM suspend hook.
> 
> [   24.284684] BUG: unable to handle kernel NULL pointer dereference at 00000000000002e0
> [   24.285627] IP: mutex_lock+0x18/0x30
> [   24.286049] PGD 78cdf067
> [   24.286050] PUD 7940f067
> [   24.286344] PMD 0
> [   24.286649]
> [   24.287072] Oops: 0002 [#1] SMP
> [   24.287422] Modules linked in: qxl
> [   24.287806] CPU: 0 PID: 2328 Comm: bash Not tainted 4.10.0-rc5+ #97
> [   24.288515] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.9.3-20161025_171302-gandalf 04/01/2014
> [   24.289681] task: ffff88007c4c0000 task.stack: ffffc90001b58000
> [   24.290354] RIP: 0010:mutex_lock+0x18/0x30
> [   24.290812] RSP: 0018:ffffc90001b5bcb0 EFLAGS: 00010246
> [   24.291401] RAX: 0000000000000000 RBX: 00000000000002e0 RCX: 0000000000000000
> [   24.292209] RDX: ffff88007c4c0000 RSI: 0000000000000001 RDI: 00000000000002e0
> [   24.292987] RBP: ffffc90001b5bcb8 R08: fffffffffffffffe R09: 0000000000000001
> [   24.293797] R10: ffff880078d80b80 R11: 0000000000011400 R12: 0000000000000000
> [   24.294601] R13: 00000000000002e0 R14: ffffffffa0009c28 R15: 0000000000000060
> [   24.295439] FS:  00007f30e3acbb40(0000) GS:ffff88007fc00000(0000) knlGS:0000000000000000
> [   24.296364] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   24.296997] CR2: 00000000000002e0 CR3: 0000000078c7b000 CR4: 00000000000006f0
> [   24.297813] Call Trace:
> [   24.298097]  drm_framebuffer_cleanup+0x1f/0x70
> [   24.298612]  qxl_fbdev_fini+0x68/0x90 [qxl]
> [   24.299074]  qxl_modeset_fini+0xd/0x30 [qxl]
> [   24.299562]  qxl_pci_remove+0x22/0x50 [qxl]
> [   24.300025]  pci_device_remove+0x34/0xb0
> [   24.300507]  device_release_driver_internal+0x150/0x200
> [   24.301082]  device_release_driver+0xd/0x10
> [   24.301587]  unbind_store+0x108/0x150
> [   24.301993]  drv_attr_store+0x20/0x30
> [   24.302402]  sysfs_kf_write+0x32/0x40
> [   24.302827]  kernfs_fop_write+0x108/0x190
> [   24.303269]  __vfs_write+0x23/0x120
> [   24.303678]  ? security_file_permission+0x36/0xb0
> [   24.304193]  ? rw_verify_area+0x49/0xb0
> [   24.304636]  vfs_write+0xb0/0x190
> [   24.305004]  SyS_write+0x41/0xa0
> [   24.305362]  entry_SYSCALL_64_fastpath+0x1a/0xa9
> [   24.305887] RIP: 0033:0x7f30e31d9620
> [   24.306285] RSP: 002b:00007ffc54b47e68 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> [   24.307128] RAX: ffffffffffffffda RBX: 00007f30e3497600 RCX: 00007f30e31d9620
> [   24.307928] RDX: 000000000000000d RSI: 0000000000da2008 RDI: 0000000000000001
> [   24.308727] RBP: 000000000070bc60 R08: 00007f30e3498760 R09: 00007f30e3acbb40
> [   24.309504] R10: 0000000000000073 R11: 0000000000000246 R12: 0000000000000001
> [   24.310295] R13: 0000000000000000 R14: 0000000000000000 R15: 00007ffc54b47f34
> [   24.311095] Code: 0e 01 e9 7b fe ff ff 66 90 66 2e 0f 1f 84 00 00 00 00 00
> 55 48 89 e5 53 48 89 fb e8 83 e8 ff ff 65 48 8b 14 25 40 c4 00 00 31 c0 <3e>
> 48 0f b1 13 48 85 c0 74 08 48 89 df e8 66 fd ff ff 5b 5d c3
> [   24.313182] RIP: mutex_lock+0x18/0x30 RSP: ffffc90001b5bcb0
> [   24.313811] CR2: 00000000000002e0
> [   24.314208] ---[ end trace 29669c1593cae14b ]---
> 
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>

Hm, I bit silly that we need #ifdef here, all this stuff is meant to Just
Work. Can't we just fix the oops with suitable checks in the fini paths
instead?

Also 0day has hit some oops in bochs, it might suffer from similar bugs.
-Daniel

> ---
>  drivers/gpu/drm/qxl/qxl_fb.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c
> index 35124737666e..14e2a49a4dcf 100644
> --- a/drivers/gpu/drm/qxl/qxl_fb.c
> +++ b/drivers/gpu/drm/qxl/qxl_fb.c
> @@ -368,9 +368,11 @@ static const struct drm_fb_helper_funcs qxl_fb_helper_funcs = {
>  
>  int qxl_fbdev_init(struct qxl_device *qdev)
>  {
> +	int ret = 0;
> +
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
>  	struct qxl_fbdev *qfbdev;
>  	int bpp_sel = 32; /* TODO: parameter from somewhere? */
> -	int ret;
>  
>  	qfbdev = kzalloc(sizeof(struct qxl_fbdev), GFP_KERNEL);
>  	if (!qfbdev)
> @@ -403,6 +405,8 @@ int qxl_fbdev_init(struct qxl_device *qdev)
>  	drm_fb_helper_fini(&qfbdev->helper);
>  free:
>  	kfree(qfbdev);
> +#endif
> +
>  	return ret;
>  }
>  
> @@ -418,6 +422,9 @@ void qxl_fbdev_fini(struct qxl_device *qdev)
>  
>  void qxl_fbdev_set_suspend(struct qxl_device *qdev, int state)
>  {
> +	if (!qdev->mode_info.qfbdev)
> +		return;
> +
>  	drm_fb_helper_set_suspend(&qdev->mode_info.qfbdev->helper, state);
>  }
>  
> -- 
> 2.11.0
> 
> _______________________________________________
> 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

[PATCH] drm: qxl: Don't clean up uninitialized fbdev framebuffer

[Gabriel Krisman Bertazi]

Daniel Vetter <daniel@ffwll.ch> writes:

Hi Daniel, thanks again for your review.

> Hm, I bit silly that we need #ifdef here, all this stuff is meant to Just
> Work. Can't we just fix the oops with suitable checks in the fini paths
> instead?

Heh.  I expected someone would bring up this objection.  My rationale is
that on the driver side, and for many drivers, fbdev emulation code is
always executed, even if it is only calling a bunch of NOP functions on
the drm core, which is a bit silly.  I wonder if struct drm_driver could
have a new callback for fbdev_initialization, which we could avoid
calling in drm_core (maybe at registering time) if FBDEV_EMULATION is
disabled. (well, this would go a bit in the opposite direction of having
open coded probe sequences).

Anyway, for the Oops I mentioned, can you consider the patch below,
instead?

>
> Also 0day has hit some oops in bochs, it might suffer from similar bugs.

I can take a look at that too.

-- >8 --
Subject: [PATCH] drm: qxl: Don't clean up uninitialized fbdev framebuffer

If fbdev emulation is disabled, the QXL shutdown path still tries to
clean the framebuffer, which wasn't initialized, hitting the Oops below.
We can check for the qfb->obj element to ensure the fb was initialized,
because the qfb always has a bo object associated with it since
initialization time.

[   24.284684] BUG: unable to handle kernel NULL pointer dereference at 00000000000002e0
[   24.285627] IP: mutex_lock+0x18/0x30
[   24.286049] PGD 78cdf067
[   24.286050] PUD 7940f067
[   24.286344] PMD 0
[   24.286649]
[   24.287072] Oops: 0002 [#1] SMP
[   24.287422] Modules linked in: qxl
[   24.287806] CPU: 0 PID: 2328 Comm: bash Not tainted 4.10.0-rc5+ #97
[   24.288515] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.9.3-20161025_171302-gandalf 04/01/2014
[   24.289681] task: ffff88007c4c0000 task.stack: ffffc90001b58000
[   24.290354] RIP: 0010:mutex_lock+0x18/0x30
[   24.290812] RSP: 0018:ffffc90001b5bcb0 EFLAGS: 00010246
[   24.291401] RAX: 0000000000000000 RBX: 00000000000002e0 RCX: 0000000000000000
[   24.292209] RDX: ffff88007c4c0000 RSI: 0000000000000001 RDI: 00000000000002e0
[   24.292987] RBP: ffffc90001b5bcb8 R08: fffffffffffffffe R09: 0000000000000001
[   24.293797] R10: ffff880078d80b80 R11: 0000000000011400 R12: 0000000000000000
[   24.294601] R13: 00000000000002e0 R14: ffffffffa0009c28 R15: 0000000000000060
[   24.295439] FS:  00007f30e3acbb40(0000) GS:ffff88007fc00000(0000) knlGS:0000000000000000
[   24.296364] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   24.296997] CR2: 00000000000002e0 CR3: 0000000078c7b000 CR4: 00000000000006f0
[   24.297813] Call Trace:
[   24.298097]  drm_framebuffer_cleanup+0x1f/0x70
[   24.298612]  qxl_fbdev_fini+0x68/0x90 [qxl]
[   24.299074]  qxl_modeset_fini+0xd/0x30 [qxl]
[   24.299562]  qxl_pci_remove+0x22/0x50 [qxl]
[   24.300025]  pci_device_remove+0x34/0xb0
[   24.300507]  device_release_driver_internal+0x150/0x200
[   24.301082]  device_release_driver+0xd/0x10
[   24.301587]  unbind_store+0x108/0x150
[   24.301993]  drv_attr_store+0x20/0x30
[   24.302402]  sysfs_kf_write+0x32/0x40
[   24.302827]  kernfs_fop_write+0x108/0x190
[   24.303269]  __vfs_write+0x23/0x120
[   24.303678]  ? security_file_permission+0x36/0xb0
[   24.304193]  ? rw_verify_area+0x49/0xb0
[   24.304636]  vfs_write+0xb0/0x190
[   24.305004]  SyS_write+0x41/0xa0
[   24.305362]  entry_SYSCALL_64_fastpath+0x1a/0xa9
[   24.305887] RIP: 0033:0x7f30e31d9620
[   24.306285] RSP: 002b:00007ffc54b47e68 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[   24.307128] RAX: ffffffffffffffda RBX: 00007f30e3497600 RCX: 00007f30e31d9620
[   24.307928] RDX: 000000000000000d RSI: 0000000000da2008 RDI: 0000000000000001
[   24.308727] RBP: 000000000070bc60 R08: 00007f30e3498760 R09: 00007f30e3acbb40
[   24.309504] R10: 0000000000000073 R11: 0000000000000246 R12: 0000000000000001
[   24.310295] R13: 0000000000000000 R14: 0000000000000000 R15: 00007ffc54b47f34
[   24.311095] Code: 0e 01 e9 7b fe ff ff 66 90 66 2e 0f 1f 84 00 00 00 00 00
55 48 89 e5 53 48 89 fb e8 83 e8 ff ff 65 48 8b 14 25 40 c4 00 00 31 c0 <3e>
48 0f b1 13 48 85 c0 74 08 48 89 df e8 66 fd ff ff 5b 5d c3
[   24.313182] RIP: mutex_lock+0x18/0x30 RSP: ffffc90001b5bcb0
[   24.313811] CR2: 00000000000002e0
[   24.314208] ---[ end trace 29669c1593cae14b ]---

Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
---
 drivers/gpu/drm/qxl/qxl_fb.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c
index 35124737666e..4d6c311e8e5e 100644
--- a/drivers/gpu/drm/qxl/qxl_fb.c
+++ b/drivers/gpu/drm/qxl/qxl_fb.c
@@ -351,13 +351,16 @@ static int qxl_fbdev_destroy(struct drm_device *dev, struct qxl_fbdev *qfbdev)
 
 	drm_fb_helper_unregister_fbi(&qfbdev->helper);
 
-	if (qfb->obj) {
+	if (qfb->obj)
 		qxlfb_destroy_pinned_object(qfb->obj);
-		qfb->obj = NULL;
-	}
+
 	drm_fb_helper_fini(&qfbdev->helper);
 	vfree(qfbdev->shadow);
-	drm_framebuffer_cleanup(&qfb->base);
+
+	if (qfb->obj)
+		drm_framebuffer_cleanup(&qfb->base);
+
+	qfb->obj = NULL;
 
 	return 0;
 }
-- 
2.11.0

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

Re: [PATCH] drm: qxl: Don't clean up uninitialized fbdev framebuffer

[Daniel Vetter]

On Wed, Mar 01, 2017 at 06:50:08PM -0300, Gabriel Krisman Bertazi wrote:
> Daniel Vetter <daniel@ffwll.ch> writes:
> 
> Hi Daniel, thanks again for your review.
> 
> > Hm, I bit silly that we need #ifdef here, all this stuff is meant to Just
> > Work. Can't we just fix the oops with suitable checks in the fini paths
> > instead?
> 
> Heh.  I expected someone would bring up this objection.  My rationale is
> that on the driver side, and for many drivers, fbdev emulation code is
> always executed, even if it is only calling a bunch of NOP functions on
> the drm core, which is a bit silly.  I wonder if struct drm_driver could
> have a new callback for fbdev_initialization, which we could avoid
> calling in drm_core (maybe at registering time) if FBDEV_EMULATION is
> disabled. (well, this would go a bit in the opposite direction of having
> open coded probe sequences).
> 
> Anyway, for the Oops I mentioned, can you consider the patch below,
> instead?

lgtm.
-Daniel

> 
> >
> > Also 0day has hit some oops in bochs, it might suffer from similar bugs.
> 
> I can take a look at that too.
> 
> -- >8 --
> Subject: [PATCH] drm: qxl: Don't clean up uninitialized fbdev framebuffer
> 
> If fbdev emulation is disabled, the QXL shutdown path still tries to
> clean the framebuffer, which wasn't initialized, hitting the Oops below.
> We can check for the qfb->obj element to ensure the fb was initialized,
> because the qfb always has a bo object associated with it since
> initialization time.
> 
> [   24.284684] BUG: unable to handle kernel NULL pointer dereference at 00000000000002e0
> [   24.285627] IP: mutex_lock+0x18/0x30
> [   24.286049] PGD 78cdf067
> [   24.286050] PUD 7940f067
> [   24.286344] PMD 0
> [   24.286649]
> [   24.287072] Oops: 0002 [#1] SMP
> [   24.287422] Modules linked in: qxl
> [   24.287806] CPU: 0 PID: 2328 Comm: bash Not tainted 4.10.0-rc5+ #97
> [   24.288515] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.9.3-20161025_171302-gandalf 04/01/2014
> [   24.289681] task: ffff88007c4c0000 task.stack: ffffc90001b58000
> [   24.290354] RIP: 0010:mutex_lock+0x18/0x30
> [   24.290812] RSP: 0018:ffffc90001b5bcb0 EFLAGS: 00010246
> [   24.291401] RAX: 0000000000000000 RBX: 00000000000002e0 RCX: 0000000000000000
> [   24.292209] RDX: ffff88007c4c0000 RSI: 0000000000000001 RDI: 00000000000002e0
> [   24.292987] RBP: ffffc90001b5bcb8 R08: fffffffffffffffe R09: 0000000000000001
> [   24.293797] R10: ffff880078d80b80 R11: 0000000000011400 R12: 0000000000000000
> [   24.294601] R13: 00000000000002e0 R14: ffffffffa0009c28 R15: 0000000000000060
> [   24.295439] FS:  00007f30e3acbb40(0000) GS:ffff88007fc00000(0000) knlGS:0000000000000000
> [   24.296364] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   24.296997] CR2: 00000000000002e0 CR3: 0000000078c7b000 CR4: 00000000000006f0
> [   24.297813] Call Trace:
> [   24.298097]  drm_framebuffer_cleanup+0x1f/0x70
> [   24.298612]  qxl_fbdev_fini+0x68/0x90 [qxl]
> [   24.299074]  qxl_modeset_fini+0xd/0x30 [qxl]
> [   24.299562]  qxl_pci_remove+0x22/0x50 [qxl]
> [   24.300025]  pci_device_remove+0x34/0xb0
> [   24.300507]  device_release_driver_internal+0x150/0x200
> [   24.301082]  device_release_driver+0xd/0x10
> [   24.301587]  unbind_store+0x108/0x150
> [   24.301993]  drv_attr_store+0x20/0x30
> [   24.302402]  sysfs_kf_write+0x32/0x40
> [   24.302827]  kernfs_fop_write+0x108/0x190
> [   24.303269]  __vfs_write+0x23/0x120
> [   24.303678]  ? security_file_permission+0x36/0xb0
> [   24.304193]  ? rw_verify_area+0x49/0xb0
> [   24.304636]  vfs_write+0xb0/0x190
> [   24.305004]  SyS_write+0x41/0xa0
> [   24.305362]  entry_SYSCALL_64_fastpath+0x1a/0xa9
> [   24.305887] RIP: 0033:0x7f30e31d9620
> [   24.306285] RSP: 002b:00007ffc54b47e68 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> [   24.307128] RAX: ffffffffffffffda RBX: 00007f30e3497600 RCX: 00007f30e31d9620
> [   24.307928] RDX: 000000000000000d RSI: 0000000000da2008 RDI: 0000000000000001
> [   24.308727] RBP: 000000000070bc60 R08: 00007f30e3498760 R09: 00007f30e3acbb40
> [   24.309504] R10: 0000000000000073 R11: 0000000000000246 R12: 0000000000000001
> [   24.310295] R13: 0000000000000000 R14: 0000000000000000 R15: 00007ffc54b47f34
> [   24.311095] Code: 0e 01 e9 7b fe ff ff 66 90 66 2e 0f 1f 84 00 00 00 00 00
> 55 48 89 e5 53 48 89 fb e8 83 e8 ff ff 65 48 8b 14 25 40 c4 00 00 31 c0 <3e>
> 48 0f b1 13 48 85 c0 74 08 48 89 df e8 66 fd ff ff 5b 5d c3
> [   24.313182] RIP: mutex_lock+0x18/0x30 RSP: ffffc90001b5bcb0
> [   24.313811] CR2: 00000000000002e0
> [   24.314208] ---[ end trace 29669c1593cae14b ]---
> 
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
> ---
>  drivers/gpu/drm/qxl/qxl_fb.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c
> index 35124737666e..4d6c311e8e5e 100644
> --- a/drivers/gpu/drm/qxl/qxl_fb.c
> +++ b/drivers/gpu/drm/qxl/qxl_fb.c
> @@ -351,13 +351,16 @@ static int qxl_fbdev_destroy(struct drm_device *dev, struct qxl_fbdev *qfbdev)
>  
>  	drm_fb_helper_unregister_fbi(&qfbdev->helper);
>  
> -	if (qfb->obj) {
> +	if (qfb->obj)
>  		qxlfb_destroy_pinned_object(qfb->obj);
> -		qfb->obj = NULL;
> -	}
> +
>  	drm_fb_helper_fini(&qfbdev->helper);
>  	vfree(qfbdev->shadow);
> -	drm_framebuffer_cleanup(&qfb->base);
> +
> +	if (qfb->obj)
> +		drm_framebuffer_cleanup(&qfb->base);

btw if you're looking for more cleanup work: Embeddeding drm_framebuffer
like this is deprecated, since it wreaks the refcounting. There's comments
around the relevant functions, and iirc we also have a todo somewhere.
-Daniel

> +
> +	qfb->obj = NULL;
>  
>  	return 0;
>  }
> -- 
> 2.11.0
> 

-- 
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