[Intel-gfx] [PATCH] drm/i915: Fix dereferencing invalid connectors in is_crtc_connector_off()

2013-07-17 Thread Chris Wilson
In commit e3de42b68478a8c95dd27520e9adead2af9477a5
Author: Imre Deak imre.d...@intel.com
Date:   Fri May 3 19:44:07 2013 +0200

drm/i915: force full modeset if the connector is in DPMS OFF mode

a new function was added that walked over the set of connectors to see
if any of the currently associated CRTC was switched off. This function
walked an array of connectors, rather than the array of pointers to
connectors contained in the drm_mode_set - i.e. it was dereferencing far
past the end of the first connector. This only becomes an issue if we
attempt to use a clone mode (i.e. more than one connector per CRTC) such
that set-num_connectors  1.

Reported-by: Timo Aaltonen tjaal...@ubuntu.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65927
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Imre Deak imre.d...@intel.com
Cc: Egbert Eich e...@suse.de
Cc: Jesse Barnes jbar...@virtuousgeek.org
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: sta...@vger.kernel.org
---
 drivers/gpu/drm/i915/intel_display.c | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 9a10e71..6790600 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8658,15 +8658,20 @@ static void intel_set_config_restore_state(struct 
drm_device *dev,
 }
 
 static bool
-is_crtc_connector_off(struct drm_crtc *crtc, struct drm_connector *connectors,
- int num_connectors)
+is_crtc_connector_off(struct drm_mode_set *set)
 {
int i;
 
-   for (i = 0; i  num_connectors; i++)
-   if (connectors[i].encoder 
-   connectors[i].encoder-crtc == crtc 
-   connectors[i].dpms != DRM_MODE_DPMS_ON)
+   if (set-num_connectors == 0)
+   return false;
+
+   if (WARN_ON(set-connectors == NULL))
+   return false;
+
+   for (i = 0; i  set-num_connectors; i++)
+   if (set-connectors[i]-encoder 
+   set-connectors[i]-encoder-crtc == set-crtc 
+   set-connectors[i]-dpms != DRM_MODE_DPMS_ON)
return true;
 
return false;
@@ -8679,10 +8684,8 @@ intel_set_config_compute_mode_changes(struct 
drm_mode_set *set,
 
/* We should be able to check here if the fb has the same properties
 * and then just flip_or_move it */
-   if (set-connectors != NULL 
-   is_crtc_connector_off(set-crtc, *set-connectors,
- set-num_connectors)) {
-   config-mode_changed = true;
+   if (is_crtc_connector_off(set)) {
+   config-mode_changed = true;
} else if (set-crtc-fb != set-fb) {
/* If we have no fb then treat it as a full mode set */
if (set-crtc-fb == NULL) {
-- 
1.8.3.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Fix dereferencing invalid connectors in is_crtc_connector_off()

2013-07-17 Thread Daniel Vetter
On Wed, Jul 17, 2013 at 12:14:40PM +0100, Chris Wilson wrote:
 In commit e3de42b68478a8c95dd27520e9adead2af9477a5
 Author: Imre Deak imre.d...@intel.com
 Date:   Fri May 3 19:44:07 2013 +0200
 
 drm/i915: force full modeset if the connector is in DPMS OFF mode
 
 a new function was added that walked over the set of connectors to see
 if any of the currently associated CRTC was switched off. This function
 walked an array of connectors, rather than the array of pointers to
 connectors contained in the drm_mode_set - i.e. it was dereferencing far
 past the end of the first connector. This only becomes an issue if we
 attempt to use a clone mode (i.e. more than one connector per CRTC) such
 that set-num_connectors  1.
 
 Reported-by: Timo Aaltonen tjaal...@ubuntu.com
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65927
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Imre Deak imre.d...@intel.com
 Cc: Egbert Eich e...@suse.de
 Cc: Jesse Barnes jbar...@virtuousgeek.org
 Cc: Daniel Vetter daniel.vet...@ffwll.ch
 Cc: sta...@vger.kernel.org

Oh dear ...

Picked up for -fixes, thanks for the patch.
-Daniel
 ---
  drivers/gpu/drm/i915/intel_display.c | 23 +--
  1 file changed, 13 insertions(+), 10 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 9a10e71..6790600 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -8658,15 +8658,20 @@ static void intel_set_config_restore_state(struct 
 drm_device *dev,
  }
  
  static bool
 -is_crtc_connector_off(struct drm_crtc *crtc, struct drm_connector 
 *connectors,
 -   int num_connectors)
 +is_crtc_connector_off(struct drm_mode_set *set)
  {
   int i;
  
 - for (i = 0; i  num_connectors; i++)
 - if (connectors[i].encoder 
 - connectors[i].encoder-crtc == crtc 
 - connectors[i].dpms != DRM_MODE_DPMS_ON)
 + if (set-num_connectors == 0)
 + return false;
 +
 + if (WARN_ON(set-connectors == NULL))
 + return false;
 +
 + for (i = 0; i  set-num_connectors; i++)
 + if (set-connectors[i]-encoder 
 + set-connectors[i]-encoder-crtc == set-crtc 
 + set-connectors[i]-dpms != DRM_MODE_DPMS_ON)
   return true;
  
   return false;
 @@ -8679,10 +8684,8 @@ intel_set_config_compute_mode_changes(struct 
 drm_mode_set *set,
  
   /* We should be able to check here if the fb has the same properties
* and then just flip_or_move it */
 - if (set-connectors != NULL 
 - is_crtc_connector_off(set-crtc, *set-connectors,
 -   set-num_connectors)) {
 - config-mode_changed = true;
 + if (is_crtc_connector_off(set)) {
 + config-mode_changed = true;
   } else if (set-crtc-fb != set-fb) {
   /* If we have no fb then treat it as a full mode set */
   if (set-crtc-fb == NULL) {
 -- 
 1.8.3.2
 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx