* [PATCH] drm/core: Fix ordering in drm_mode_config_cleanup.
@ 2016-03-22 14:42 Maarten Lankhorst
2016-03-22 14:58 ` Ville Syrjälä
2016-03-22 17:07 ` ✓ Fi.CI.BAT: success for " Patchwork
0 siblings, 2 replies; 6+ messages in thread
From: Maarten Lankhorst @ 2016-03-22 14:42 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
__drm_atomic_helper_plane_destroy_state calls
drm_framebuffer_unreference, which means that if drm_framebuffer_free
is called before plane->destroy freed memory will be accessed.
A similar case happens for the blob list, which was freed before the
crtc state was, resulting in the unreference_blob from crtc_destroy_state
pointing to garbage memory causing another opportunity for a GPF.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/drm_crtc.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 51c5a00ffdff..5a13b1afccbe 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5958,6 +5958,15 @@ void drm_mode_config_cleanup(struct drm_device *dev)
drm_property_destroy(dev, property);
}
+ list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list,
+ head) {
+ plane->funcs->destroy(plane);
+ }
+
+ list_for_each_entry_safe(crtc, ct, &dev->mode_config.crtc_list, head) {
+ crtc->funcs->destroy(crtc);
+ }
+
list_for_each_entry_safe(blob, bt, &dev->mode_config.property_blob_list,
head_global) {
drm_property_unreference_blob(blob);
@@ -5976,15 +5985,6 @@ void drm_mode_config_cleanup(struct drm_device *dev)
drm_framebuffer_free(&fb->refcount);
}
- list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list,
- head) {
- plane->funcs->destroy(plane);
- }
-
- list_for_each_entry_safe(crtc, ct, &dev->mode_config.crtc_list, head) {
- crtc->funcs->destroy(crtc);
- }
-
ida_destroy(&dev->mode_config.connector_ida);
idr_destroy(&dev->mode_config.tile_idr);
idr_destroy(&dev->mode_config.crtc_idr);
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] drm/core: Fix ordering in drm_mode_config_cleanup.
2016-03-22 14:42 [PATCH] drm/core: Fix ordering in drm_mode_config_cleanup Maarten Lankhorst
@ 2016-03-22 14:58 ` Ville Syrjälä
2016-03-22 15:08 ` Maarten Lankhorst
2016-03-22 17:07 ` ✓ Fi.CI.BAT: success for " Patchwork
1 sibling, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2016-03-22 14:58 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx, dri-devel
On Tue, Mar 22, 2016 at 03:42:14PM +0100, Maarten Lankhorst wrote:
> __drm_atomic_helper_plane_destroy_state calls
> drm_framebuffer_unreference, which means that if drm_framebuffer_free
> is called before plane->destroy freed memory will be accessed.
>
> A similar case happens for the blob list, which was freed before the
> crtc state was, resulting in the unreference_blob from crtc_destroy_state
> pointing to garbage memory causing another opportunity for a GPF.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/drm_crtc.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 51c5a00ffdff..5a13b1afccbe 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -5958,6 +5958,15 @@ void drm_mode_config_cleanup(struct drm_device *dev)
> drm_property_destroy(dev, property);
> }
And what about props? Any chance of explosion due to those being gone?
>
> + list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list,
> + head) {
> + plane->funcs->destroy(plane);
> + }
> +
> + list_for_each_entry_safe(crtc, ct, &dev->mode_config.crtc_list, head) {
> + crtc->funcs->destroy(crtc);
> + }
> +
> list_for_each_entry_safe(blob, bt, &dev->mode_config.property_blob_list,
> head_global) {
> drm_property_unreference_blob(blob);
> @@ -5976,15 +5985,6 @@ void drm_mode_config_cleanup(struct drm_device *dev)
> drm_framebuffer_free(&fb->refcount);
> }
>
> - list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list,
> - head) {
> - plane->funcs->destroy(plane);
> - }
> -
> - list_for_each_entry_safe(crtc, ct, &dev->mode_config.crtc_list, head) {
> - crtc->funcs->destroy(crtc);
> - }
> -
> ida_destroy(&dev->mode_config.connector_ida);
> idr_destroy(&dev->mode_config.tile_idr);
> idr_destroy(&dev->mode_config.crtc_idr);
> --
> 2.1.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/core: Fix ordering in drm_mode_config_cleanup.
2016-03-22 14:58 ` Ville Syrjälä
@ 2016-03-22 15:08 ` Maarten Lankhorst
2016-04-01 20:04 ` Ville Syrjälä
0 siblings, 1 reply; 6+ messages in thread
From: Maarten Lankhorst @ 2016-03-22 15:08 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, dri-devel
Op 22-03-16 om 15:58 schreef Ville Syrjälä:
> On Tue, Mar 22, 2016 at 03:42:14PM +0100, Maarten Lankhorst wrote:
>> __drm_atomic_helper_plane_destroy_state calls
>> drm_framebuffer_unreference, which means that if drm_framebuffer_free
>> is called before plane->destroy freed memory will be accessed.
>>
>> A similar case happens for the blob list, which was freed before the
>> crtc state was, resulting in the unreference_blob from crtc_destroy_state
>> pointing to garbage memory causing another opportunity for a GPF.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>> drivers/gpu/drm/drm_crtc.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index 51c5a00ffdff..5a13b1afccbe 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -5958,6 +5958,15 @@ void drm_mode_config_cleanup(struct drm_device *dev)
>> drm_property_destroy(dev, property);
>> }
> And what about props? Any chance of explosion due to those being gone?
>
Not as far as I'm aware of.
If you use something like a crtc_x property, only the value gets written to crtc_state, the value is an int that would still be valid.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/core: Fix ordering in drm_mode_config_cleanup.
2016-03-22 15:08 ` Maarten Lankhorst
@ 2016-04-01 20:04 ` Ville Syrjälä
2016-04-12 10:43 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2016-04-01 20:04 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx, dri-devel
On Tue, Mar 22, 2016 at 04:08:39PM +0100, Maarten Lankhorst wrote:
> Op 22-03-16 om 15:58 schreef Ville Syrjälä:
> > On Tue, Mar 22, 2016 at 03:42:14PM +0100, Maarten Lankhorst wrote:
> >> __drm_atomic_helper_plane_destroy_state calls
> >> drm_framebuffer_unreference, which means that if drm_framebuffer_free
> >> is called before plane->destroy freed memory will be accessed.
> >>
> >> A similar case happens for the blob list, which was freed before the
> >> crtc state was, resulting in the unreference_blob from crtc_destroy_state
> >> pointing to garbage memory causing another opportunity for a GPF.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >> drivers/gpu/drm/drm_crtc.c | 18 +++++++++---------
> >> 1 file changed, 9 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> >> index 51c5a00ffdff..5a13b1afccbe 100644
> >> --- a/drivers/gpu/drm/drm_crtc.c
> >> +++ b/drivers/gpu/drm/drm_crtc.c
> >> @@ -5958,6 +5958,15 @@ void drm_mode_config_cleanup(struct drm_device *dev)
> >> drm_property_destroy(dev, property);
> >> }
> > And what about props? Any chance of explosion due to those being gone?
> >
> Not as far as I'm aware of.
>
> If you use something like a crtc_x property, only the value gets written to crtc_state, the value is an int that would still be valid.
I was too lazy to confirm this for all drivers. But at least i915 looked
clean on that front. So
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> ~Maarten
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/core: Fix ordering in drm_mode_config_cleanup.
2016-04-01 20:04 ` Ville Syrjälä
@ 2016-04-12 10:43 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2016-04-12 10:43 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, dri-devel
On Fri, Apr 01, 2016 at 11:04:10PM +0300, Ville Syrjälä wrote:
> On Tue, Mar 22, 2016 at 04:08:39PM +0100, Maarten Lankhorst wrote:
> > Op 22-03-16 om 15:58 schreef Ville Syrjälä:
> > > On Tue, Mar 22, 2016 at 03:42:14PM +0100, Maarten Lankhorst wrote:
> > >> __drm_atomic_helper_plane_destroy_state calls
> > >> drm_framebuffer_unreference, which means that if drm_framebuffer_free
> > >> is called before plane->destroy freed memory will be accessed.
> > >>
> > >> A similar case happens for the blob list, which was freed before the
> > >> crtc state was, resulting in the unreference_blob from crtc_destroy_state
> > >> pointing to garbage memory causing another opportunity for a GPF.
> > >>
> > >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > >> ---
> > >> drivers/gpu/drm/drm_crtc.c | 18 +++++++++---------
> > >> 1 file changed, 9 insertions(+), 9 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > >> index 51c5a00ffdff..5a13b1afccbe 100644
> > >> --- a/drivers/gpu/drm/drm_crtc.c
> > >> +++ b/drivers/gpu/drm/drm_crtc.c
> > >> @@ -5958,6 +5958,15 @@ void drm_mode_config_cleanup(struct drm_device *dev)
> > >> drm_property_destroy(dev, property);
> > >> }
> > > And what about props? Any chance of explosion due to those being gone?
> > >
> > Not as far as I'm aware of.
> >
> > If you use something like a crtc_x property, only the value gets written to crtc_state, the value is an int that would still be valid.
>
> I was too lazy to confirm this for all drivers. But at least i915 looked
> clean on that front. So
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Applied to drm-misc, thanks.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* ✓ Fi.CI.BAT: success for drm/core: Fix ordering in drm_mode_config_cleanup.
2016-03-22 14:42 [PATCH] drm/core: Fix ordering in drm_mode_config_cleanup Maarten Lankhorst
2016-03-22 14:58 ` Ville Syrjälä
@ 2016-03-22 17:07 ` Patchwork
1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2016-03-22 17:07 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
== Series Details ==
Series: drm/core: Fix ordering in drm_mode_config_cleanup.
URL : https://patchwork.freedesktop.org/series/4758/
State : success
== Summary ==
Series 4758v1 drm/core: Fix ordering in drm_mode_config_cleanup.
http://patchwork.freedesktop.org/api/1.0/series/4758/revisions/1/mbox/
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-c:
dmesg-warn -> PASS (bsw-nuc-2)
Test pm_rpm:
Subgroup basic-pci-d3-state:
dmesg-warn -> PASS (byt-nuc)
Subgroup basic-rte:
pass -> DMESG-WARN (byt-nuc) UNSTABLE
bdw-nuci7 total:192 pass:180 dwarn:0 dfail:0 fail:0 skip:12
bdw-ultra total:192 pass:171 dwarn:0 dfail:0 fail:0 skip:21
bsw-nuc-2 total:192 pass:154 dwarn:1 dfail:0 fail:0 skip:37
byt-nuc total:192 pass:156 dwarn:1 dfail:0 fail:0 skip:35
hsw-brixbox total:192 pass:170 dwarn:0 dfail:0 fail:0 skip:22
hsw-gt2 total:192 pass:175 dwarn:0 dfail:0 fail:0 skip:17
ivb-t430s total:192 pass:167 dwarn:0 dfail:0 fail:0 skip:25
skl-i5k-2 total:192 pass:169 dwarn:0 dfail:0 fail:0 skip:23
skl-i7k-2 total:192 pass:169 dwarn:0 dfail:0 fail:0 skip:23
skl-nuci5 total:192 pass:181 dwarn:0 dfail:0 fail:0 skip:11
snb-dellxps total:192 pass:158 dwarn:0 dfail:0 fail:0 skip:34
snb-x220t total:192 pass:158 dwarn:0 dfail:0 fail:1 skip:33
Results at /archive/results/CI_IGT_test/Patchwork_1679/
83ed25fa1b956275542da63eb98dc8fd2291329d drm-intel-nightly: 2016y-03m-22d-15h-20m-55s UTC integration manifest
2fb96a0c0489a7c3c751ae69e7d13bf503d2ce25 drm/core: Fix ordering in drm_mode_config_cleanup.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-12 10:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-22 14:42 [PATCH] drm/core: Fix ordering in drm_mode_config_cleanup Maarten Lankhorst
2016-03-22 14:58 ` Ville Syrjälä
2016-03-22 15:08 ` Maarten Lankhorst
2016-04-01 20:04 ` Ville Syrjälä
2016-04-12 10:43 ` Daniel Vetter
2016-03-22 17:07 ` ✓ Fi.CI.BAT: success for " Patchwork
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).