* [PATCH] drm: fix a use-after-free when GPU acceleration disabled
@ 2013-05-21 6:23 Huacai Chen
2013-05-21 7:59 ` Michel Dänzer
2013-05-21 15:30 ` Paul Menzel
0 siblings, 2 replies; 5+ messages in thread
From: Huacai Chen @ 2013-05-21 6:23 UTC (permalink / raw)
To: David Airlie
Cc: dri-devel, Fuxin Zhang, Alex Deucher, Michel Dänzer,
Huacai Chen, Binbin Zhou, stable
When GPU acceleration is disabled, drm_vblank_cleanup() will free the
vblank-related data, such as vblank_refcount, vblank_inmodeset, etc.
But we found that drm_vblank_post_modeset() may be called after the
cleanup, which use vblank_refcount and vblank_inmodeset. And this will
cause a kernel panic.
Fix this by return immediately if dev->num_crtcs is zero. This is the
same thing that drm_vblank_pre_modeset() does.
Call trace of a drm_vblank_post_modeset() after drm_vblank_cleanup():
[ 62.628906] [<ffffffff804868d0>] drm_vblank_post_modeset+0x34/0xb4
[ 62.628906] [<ffffffff804c7008>] atombios_crtc_dpms+0xb4/0x174
[ 62.628906] [<ffffffff804c70e0>] atombios_crtc_commit+0x18/0x38
[ 62.628906] [<ffffffff8047f038>] drm_crtc_helper_set_mode+0x304/0x3cc
[ 62.628906] [<ffffffff8047f92c>] drm_crtc_helper_set_config+0x6d8/0x988
[ 62.628906] [<ffffffff8047dd40>] drm_fb_helper_set_par+0x94/0x104
[ 62.628906] [<ffffffff80439d14>] fbcon_init+0x424/0x57c
[ 62.628906] [<ffffffff8046a638>] visual_init+0xb8/0x118
[ 62.628906] [<ffffffff8046b9f8>] take_over_console+0x238/0x384
[ 62.628906] [<ffffffff80436df8>] fbcon_takeover+0x7c/0xdc
[ 62.628906] [<ffffffff8024fa20>] notifier_call_chain+0x44/0x94
[ 62.628906] [<ffffffff8024fcbc>] __blocking_notifier_call_chain+0x48/0x68
[ 62.628906] [<ffffffff8042d990>] register_framebuffer+0x228/0x260
[ 62.628906] [<ffffffff8047e010>] drm_fb_helper_single_fb_probe+0x260/0x314
[ 62.628906] [<ffffffff8047e2c4>] drm_fb_helper_initial_config+0x200/0x234
[ 62.628906] [<ffffffff804e5560>] radeon_fbdev_init+0xd4/0xf4
[ 62.628906] [<ffffffff804e0e08>] radeon_modeset_init+0x9bc/0xa18
[ 62.628906] [<ffffffff804bfc14>] radeon_driver_load_kms+0xdc/0x12c
[ 62.628906] [<ffffffff8048b548>] drm_get_pci_dev+0x148/0x238
[ 62.628906] [<ffffffff80423564>] local_pci_probe+0x5c/0xd0
[ 62.628906] [<ffffffff80241ac4>] work_for_cpu_fn+0x1c/0x30
[ 62.628906] [<ffffffff802427c8>] process_one_work+0x274/0x3bc
[ 62.628906] [<ffffffff80242934>] process_scheduled_works+0x24/0x44
[ 62.628906] [<ffffffff8024515c>] worker_thread+0x31c/0x3f4
[ 62.628906] [<ffffffff802497a8>] kthread+0x88/0x90
[ 62.628906] [<ffffffff80206794>] kernel_thread_helper+0x10/0x18
Signed-off-by: Huacai Chen <chenhc@lemote.com>
Signed-off-by: Binbin Zhou <zhoubb@lemote.com>
Cc: <stable@vger.kernel.org>
---
drivers/gpu/drm/drm_irq.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index c798eea..c7a1348 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -981,7 +981,7 @@ EXPORT_SYMBOL(drm_vblank_off);
*/
void drm_vblank_pre_modeset(struct drm_device *dev, int crtc)
{
- /* vblank is not initialized (IRQ not installed ?) */
+ /* vblank is not initialized (IRQ not installed ?), or has been freed */
if (!dev->num_crtcs)
return;
/*
@@ -1003,6 +1003,10 @@ void drm_vblank_post_modeset(struct drm_device *dev, int crtc)
{
unsigned long irqflags;
+ /* vblank is not initialized (IRQ not installed ?), or has been freed */
+ if (!dev->num_crtcs)
+ return;
+
if (dev->vblank_inmodeset[crtc]) {
spin_lock_irqsave(&dev->vbl_lock, irqflags);
dev->vblank_disable_allowed = 1;
--
1.7.7.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm: fix a use-after-free when GPU acceleration disabled
2013-05-21 6:23 [PATCH] drm: fix a use-after-free when GPU acceleration disabled Huacai Chen
@ 2013-05-21 7:59 ` Michel Dänzer
2013-05-21 15:30 ` Paul Menzel
1 sibling, 0 replies; 5+ messages in thread
From: Michel Dänzer @ 2013-05-21 7:59 UTC (permalink / raw)
To: Huacai Chen; +Cc: dri-devel
On Die, 2013-05-21 at 14:23 +0800, Huacai Chen wrote:
> When GPU acceleration is disabled, drm_vblank_cleanup() will free the
> vblank-related data, such as vblank_refcount, vblank_inmodeset, etc.
> But we found that drm_vblank_post_modeset() may be called after the
> cleanup, which use vblank_refcount and vblank_inmodeset. And this will
> cause a kernel panic.
>
> Fix this by return immediately if dev->num_crtcs is zero. This is the
> same thing that drm_vblank_pre_modeset() does.
>
> Call trace of a drm_vblank_post_modeset() after drm_vblank_cleanup():
> [ 62.628906] [<ffffffff804868d0>] drm_vblank_post_modeset+0x34/0xb4
> [ 62.628906] [<ffffffff804c7008>] atombios_crtc_dpms+0xb4/0x174
> [ 62.628906] [<ffffffff804c70e0>] atombios_crtc_commit+0x18/0x38
> [ 62.628906] [<ffffffff8047f038>] drm_crtc_helper_set_mode+0x304/0x3cc
> [ 62.628906] [<ffffffff8047f92c>] drm_crtc_helper_set_config+0x6d8/0x988
> [ 62.628906] [<ffffffff8047dd40>] drm_fb_helper_set_par+0x94/0x104
> [ 62.628906] [<ffffffff80439d14>] fbcon_init+0x424/0x57c
> [ 62.628906] [<ffffffff8046a638>] visual_init+0xb8/0x118
> [ 62.628906] [<ffffffff8046b9f8>] take_over_console+0x238/0x384
> [ 62.628906] [<ffffffff80436df8>] fbcon_takeover+0x7c/0xdc
> [ 62.628906] [<ffffffff8024fa20>] notifier_call_chain+0x44/0x94
> [ 62.628906] [<ffffffff8024fcbc>] __blocking_notifier_call_chain+0x48/0x68
> [ 62.628906] [<ffffffff8042d990>] register_framebuffer+0x228/0x260
> [ 62.628906] [<ffffffff8047e010>] drm_fb_helper_single_fb_probe+0x260/0x314
> [ 62.628906] [<ffffffff8047e2c4>] drm_fb_helper_initial_config+0x200/0x234
> [ 62.628906] [<ffffffff804e5560>] radeon_fbdev_init+0xd4/0xf4
> [ 62.628906] [<ffffffff804e0e08>] radeon_modeset_init+0x9bc/0xa18
> [ 62.628906] [<ffffffff804bfc14>] radeon_driver_load_kms+0xdc/0x12c
> [ 62.628906] [<ffffffff8048b548>] drm_get_pci_dev+0x148/0x238
> [ 62.628906] [<ffffffff80423564>] local_pci_probe+0x5c/0xd0
> [ 62.628906] [<ffffffff80241ac4>] work_for_cpu_fn+0x1c/0x30
> [ 62.628906] [<ffffffff802427c8>] process_one_work+0x274/0x3bc
> [ 62.628906] [<ffffffff80242934>] process_scheduled_works+0x24/0x44
> [ 62.628906] [<ffffffff8024515c>] worker_thread+0x31c/0x3f4
> [ 62.628906] [<ffffffff802497a8>] kthread+0x88/0x90
> [ 62.628906] [<ffffffff80206794>] kernel_thread_helper+0x10/0x18
>
> Signed-off-by: Huacai Chen <chenhc@lemote.com>
> Signed-off-by: Binbin Zhou <zhoubb@lemote.com>
> Cc: <stable@vger.kernel.org>
> ---
> drivers/gpu/drm/drm_irq.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index c798eea..c7a1348 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -981,7 +981,7 @@ EXPORT_SYMBOL(drm_vblank_off);
> */
> void drm_vblank_pre_modeset(struct drm_device *dev, int crtc)
> {
> - /* vblank is not initialized (IRQ not installed ?) */
> + /* vblank is not initialized (IRQ not installed ?), or has been freed */
> if (!dev->num_crtcs)
> return;
> /*
> @@ -1003,6 +1003,10 @@ void drm_vblank_post_modeset(struct drm_device *dev, int crtc)
> {
> unsigned long irqflags;
>
> + /* vblank is not initialized (IRQ not installed ?), or has been freed */
> + if (!dev->num_crtcs)
> + return;
> +
> if (dev->vblank_inmodeset[crtc]) {
> spin_lock_irqsave(&dev->vbl_lock, irqflags);
> dev->vblank_disable_allowed = 1;
The comment changes seem superfluous to me, but either way:
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Debian, X and DRI developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm: fix a use-after-free when GPU acceleration disabled
2013-05-21 6:23 [PATCH] drm: fix a use-after-free when GPU acceleration disabled Huacai Chen
2013-05-21 7:59 ` Michel Dänzer
@ 2013-05-21 15:30 ` Paul Menzel
2013-06-03 8:29 ` Huacai Chen
1 sibling, 1 reply; 5+ messages in thread
From: Paul Menzel @ 2013-05-21 15:30 UTC (permalink / raw)
To: Huacai Chen
Cc: David Airlie, Binbin Zhou, Michel Dänzer, dri-devel, stable,
Fuxin Zhang, Alex Deucher
[-- Attachment #1: Type: text/plain, Size: 2753 bytes --]
Am Dienstag, den 21.05.2013, 14:23 +0800 schrieb Huacai Chen:
> When GPU acceleration is disabled, drm_vblank_cleanup() will free the
> vblank-related data, such as vblank_refcount, vblank_inmodeset, etc.
> But we found that drm_vblank_post_modeset() may be called after the
> cleanup, which use vblank_refcount and vblank_inmodeset. And this will
> cause a kernel panic.
>
> Fix this by return immediately if dev->num_crtcs is zero. This is the
> same thing that drm_vblank_pre_modeset() does.
>
> Call trace of a drm_vblank_post_modeset() after drm_vblank_cleanup():
> [ 62.628906] [<ffffffff804868d0>] drm_vblank_post_modeset+0x34/0xb4
> [ 62.628906] [<ffffffff804c7008>] atombios_crtc_dpms+0xb4/0x174
> [ 62.628906] [<ffffffff804c70e0>] atombios_crtc_commit+0x18/0x38
> [ 62.628906] [<ffffffff8047f038>] drm_crtc_helper_set_mode+0x304/0x3cc
> [ 62.628906] [<ffffffff8047f92c>] drm_crtc_helper_set_config+0x6d8/0x988
> [ 62.628906] [<ffffffff8047dd40>] drm_fb_helper_set_par+0x94/0x104
> [ 62.628906] [<ffffffff80439d14>] fbcon_init+0x424/0x57c
> [ 62.628906] [<ffffffff8046a638>] visual_init+0xb8/0x118
> [ 62.628906] [<ffffffff8046b9f8>] take_over_console+0x238/0x384
> [ 62.628906] [<ffffffff80436df8>] fbcon_takeover+0x7c/0xdc
> [ 62.628906] [<ffffffff8024fa20>] notifier_call_chain+0x44/0x94
> [ 62.628906] [<ffffffff8024fcbc>] __blocking_notifier_call_chain+0x48/0x68
> [ 62.628906] [<ffffffff8042d990>] register_framebuffer+0x228/0x260
> [ 62.628906] [<ffffffff8047e010>] drm_fb_helper_single_fb_probe+0x260/0x314
> [ 62.628906] [<ffffffff8047e2c4>] drm_fb_helper_initial_config+0x200/0x234
> [ 62.628906] [<ffffffff804e5560>] radeon_fbdev_init+0xd4/0xf4
> [ 62.628906] [<ffffffff804e0e08>] radeon_modeset_init+0x9bc/0xa18
> [ 62.628906] [<ffffffff804bfc14>] radeon_driver_load_kms+0xdc/0x12c
> [ 62.628906] [<ffffffff8048b548>] drm_get_pci_dev+0x148/0x238
> [ 62.628906] [<ffffffff80423564>] local_pci_probe+0x5c/0xd0
> [ 62.628906] [<ffffffff80241ac4>] work_for_cpu_fn+0x1c/0x30
> [ 62.628906] [<ffffffff802427c8>] process_one_work+0x274/0x3bc
> [ 62.628906] [<ffffffff80242934>] process_scheduled_works+0x24/0x44
> [ 62.628906] [<ffffffff8024515c>] worker_thread+0x31c/0x3f4
> [ 62.628906] [<ffffffff802497a8>] kthread+0x88/0x90
> [ 62.628906] [<ffffffff80206794>] kernel_thread_helper+0x10/0x18
Nice commit message!
> Signed-off-by: Huacai Chen <chenhc@lemote.com>
> Signed-off-by: Binbin Zhou <zhoubb@lemote.com>
> Cc: <stable@vger.kernel.org>
> ---
> drivers/gpu/drm/drm_irq.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
[…]
Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
Thanks,
Paul
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm: fix a use-after-free when GPU acceleration disabled
2013-05-21 15:30 ` Paul Menzel
@ 2013-06-03 8:29 ` Huacai Chen
2013-06-03 9:14 ` Dave Airlie
0 siblings, 1 reply; 5+ messages in thread
From: Huacai Chen @ 2013-06-03 8:29 UTC (permalink / raw)
To: Paul Menzel
Cc: Binbin Zhou, Michel Dänzer, dri-devel, stable, Fuxin Zhang,
Alex Deucher
[-- Attachment #1.1: Type: text/plain, Size: 3258 bytes --]
Hi, Dave
Why this patch hasn't been applied in your tree? I think you are the right
maintainer.
Huacai Chen
On Tue, May 21, 2013 at 11:30 PM, Paul Menzel <
paulepanter@users.sourceforge.net> wrote:
> Am Dienstag, den 21.05.2013, 14:23 +0800 schrieb Huacai Chen:
> > When GPU acceleration is disabled, drm_vblank_cleanup() will free the
> > vblank-related data, such as vblank_refcount, vblank_inmodeset, etc.
> > But we found that drm_vblank_post_modeset() may be called after the
> > cleanup, which use vblank_refcount and vblank_inmodeset. And this will
> > cause a kernel panic.
> >
> > Fix this by return immediately if dev->num_crtcs is zero. This is the
> > same thing that drm_vblank_pre_modeset() does.
> >
> > Call trace of a drm_vblank_post_modeset() after drm_vblank_cleanup():
> > [ 62.628906] [<ffffffff804868d0>] drm_vblank_post_modeset+0x34/0xb4
> > [ 62.628906] [<ffffffff804c7008>] atombios_crtc_dpms+0xb4/0x174
> > [ 62.628906] [<ffffffff804c70e0>] atombios_crtc_commit+0x18/0x38
> > [ 62.628906] [<ffffffff8047f038>] drm_crtc_helper_set_mode+0x304/0x3cc
> > [ 62.628906] [<ffffffff8047f92c>]
> drm_crtc_helper_set_config+0x6d8/0x988
> > [ 62.628906] [<ffffffff8047dd40>] drm_fb_helper_set_par+0x94/0x104
> > [ 62.628906] [<ffffffff80439d14>] fbcon_init+0x424/0x57c
> > [ 62.628906] [<ffffffff8046a638>] visual_init+0xb8/0x118
> > [ 62.628906] [<ffffffff8046b9f8>] take_over_console+0x238/0x384
> > [ 62.628906] [<ffffffff80436df8>] fbcon_takeover+0x7c/0xdc
> > [ 62.628906] [<ffffffff8024fa20>] notifier_call_chain+0x44/0x94
> > [ 62.628906] [<ffffffff8024fcbc>]
> __blocking_notifier_call_chain+0x48/0x68
> > [ 62.628906] [<ffffffff8042d990>] register_framebuffer+0x228/0x260
> > [ 62.628906] [<ffffffff8047e010>]
> drm_fb_helper_single_fb_probe+0x260/0x314
> > [ 62.628906] [<ffffffff8047e2c4>]
> drm_fb_helper_initial_config+0x200/0x234
> > [ 62.628906] [<ffffffff804e5560>] radeon_fbdev_init+0xd4/0xf4
> > [ 62.628906] [<ffffffff804e0e08>] radeon_modeset_init+0x9bc/0xa18
> > [ 62.628906] [<ffffffff804bfc14>] radeon_driver_load_kms+0xdc/0x12c
> > [ 62.628906] [<ffffffff8048b548>] drm_get_pci_dev+0x148/0x238
> > [ 62.628906] [<ffffffff80423564>] local_pci_probe+0x5c/0xd0
> > [ 62.628906] [<ffffffff80241ac4>] work_for_cpu_fn+0x1c/0x30
> > [ 62.628906] [<ffffffff802427c8>] process_one_work+0x274/0x3bc
> > [ 62.628906] [<ffffffff80242934>] process_scheduled_works+0x24/0x44
> > [ 62.628906] [<ffffffff8024515c>] worker_thread+0x31c/0x3f4
> > [ 62.628906] [<ffffffff802497a8>] kthread+0x88/0x90
> > [ 62.628906] [<ffffffff80206794>] kernel_thread_helper+0x10/0x18
>
> Nice commit message!
>
> > Signed-off-by: Huacai Chen <chenhc@lemote.com>
> > Signed-off-by: Binbin Zhou <zhoubb@lemote.com>
> > Cc: <stable@vger.kernel.org>
> > ---
> > drivers/gpu/drm/drm_irq.c | 6 +++++-
> > 1 files changed, 5 insertions(+), 1 deletions(-)
>
> […]
>
> Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
>
>
> Thanks,
>
> Paul
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
[-- Attachment #1.2: Type: text/html, Size: 4395 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm: fix a use-after-free when GPU acceleration disabled
2013-06-03 8:29 ` Huacai Chen
@ 2013-06-03 9:14 ` Dave Airlie
0 siblings, 0 replies; 5+ messages in thread
From: Dave Airlie @ 2013-06-03 9:14 UTC (permalink / raw)
To: Huacai Chen
Cc: Binbin Zhou, Michel Dänzer, dri-devel, Fuxin Zhang,
Alex Deucher, Paul Menzel
On Mon, Jun 3, 2013 at 6:29 PM, Huacai Chen <chenhc@lemote.com> wrote:
> Hi, Dave
>
> Why this patch hasn't been applied in your tree? I think you are the right
> maintainer.
>
Because I missed it, i've stuck in my fixes queue now.
thanks for pinging.
Dave.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-06-03 9:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-21 6:23 [PATCH] drm: fix a use-after-free when GPU acceleration disabled Huacai Chen
2013-05-21 7:59 ` Michel Dänzer
2013-05-21 15:30 ` Paul Menzel
2013-06-03 8:29 ` Huacai Chen
2013-06-03 9:14 ` Dave Airlie
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.