Re: [Intel-gfx] [PATCH] drm: Make integer overflow checking cover universal cursor updates (v2)

2015-04-14 Thread Daniel Vetter 
On Mon, Apr 13, 2015 at 11:06:13AM -0700, Matt Roper wrote:
 Our legacy SetPlane updates perform integer overflow checking on a
 plane's destination rectangle in drm_mode_setplane(), and atomic updates
 handled as part of a drm_atomic_state transaction do the same checking
 in drm_atomic_plane_check().  However legacy cursor updates that get
 routed through universal plane interfaces may bypass this overflow
 checking if the driver's .update_plane is serviced by the transitional
 plane helpers rather than the full atomic plane helpers.
 
 Move the check for destination rectangle integer overflow from the
 drm_mode_setplane() to __setplane_internal() so that it also covers
 cursor operations.
 
 This fixes an issue first noticed with i915 commit:
 
 commit ff42e093e9c9c17a6e1d6aab24875a36795f926e
 Author: Daniel Vetter daniel.vet...@ffwll.ch
 Date:   Mon Mar 2 16:35:20 2015 +0100
 
 Revert drm/i915: Switch planes from transitional helpers to full
 atomic helpers
 
 The above revert switched us from full atomic helpers back to the
 transitional helpers, and in doing so we lost the overflow checking here
 for universal cursor updates.  Even though such extreme cursor positions
 are unlikely to actually happen in the wild, we still don't want there
 to be a change of behavior when drivers switch from transitional helpers
 to full helpers.
 
 v2: Move check from setplane ioctl to setplane_internal rather than
 adding an additional copy of the checks to the transitional plane
 helpers.  (Daniel)
 
 Cc: Daniel Vetter dan...@ffwll.ch
 Testcase: igt/kms_cursor_crc
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84269
 Signed-off-by: Matt Roper matthew.d.ro...@intel.com

Applied to drm-misc, thanks.
-Daniel

 ---
  drivers/gpu/drm/drm_crtc.c | 22 +++---
  1 file changed, 11 insertions(+), 11 deletions(-)
 
 diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
 index b914882..160647a 100644
 --- a/drivers/gpu/drm/drm_crtc.c
 +++ b/drivers/gpu/drm/drm_crtc.c
 @@ -2507,6 +2507,17 @@ static int __setplane_internal(struct drm_plane *plane,
   goto out;
   }
  
 + /* Give drivers some help against integer overflows */
 + if (crtc_w  INT_MAX ||
 + crtc_x  INT_MAX - (int32_t) crtc_w ||
 + crtc_h  INT_MAX ||
 + crtc_y  INT_MAX - (int32_t) crtc_h) {
 + DRM_DEBUG_KMS(Invalid CRTC coordinates %ux%u+%d+%d\n,
 +   crtc_w, crtc_h, crtc_x, crtc_y);
 + return -ERANGE;
 + }
 +
 +
   fb_width = fb-width  16;
   fb_height = fb-height  16;
  
 @@ -2591,17 +2602,6 @@ int drm_mode_setplane(struct drm_device *dev, void 
 *data,
   if (!drm_core_check_feature(dev, DRIVER_MODESET))
   return -EINVAL;
  
 - /* Give drivers some help against integer overflows */
 - if (plane_req-crtc_w  INT_MAX ||
 - plane_req-crtc_x  INT_MAX - (int32_t) plane_req-crtc_w ||
 - plane_req-crtc_h  INT_MAX ||
 - plane_req-crtc_y  INT_MAX - (int32_t) plane_req-crtc_h) {
 - DRM_DEBUG_KMS(Invalid CRTC coordinates %ux%u+%d+%d\n,
 -   plane_req-crtc_w, plane_req-crtc_h,
 -   plane_req-crtc_x, plane_req-crtc_y);
 - return -ERANGE;
 - }
 -
   /*
* First, find the plane, crtc, and fb objects.  If not available,
* we don't bother to call the driver.
 -- 
 1.8.5.1
 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm: Make integer overflow checking cover universal cursor updates (v2)

2015-04-13 Thread Matt Roper 
Our legacy SetPlane updates perform integer overflow checking on a
plane's destination rectangle in drm_mode_setplane(), and atomic updates
handled as part of a drm_atomic_state transaction do the same checking
in drm_atomic_plane_check().  However legacy cursor updates that get
routed through universal plane interfaces may bypass this overflow
checking if the driver's .update_plane is serviced by the transitional
plane helpers rather than the full atomic plane helpers.

Move the check for destination rectangle integer overflow from the
drm_mode_setplane() to __setplane_internal() so that it also covers
cursor operations.

This fixes an issue first noticed with i915 commit:

commit ff42e093e9c9c17a6e1d6aab24875a36795f926e
Author: Daniel Vetter daniel.vet...@ffwll.ch
Date:   Mon Mar 2 16:35:20 2015 +0100

Revert drm/i915: Switch planes from transitional helpers to full
atomic helpers

The above revert switched us from full atomic helpers back to the
transitional helpers, and in doing so we lost the overflow checking here
for universal cursor updates.  Even though such extreme cursor positions
are unlikely to actually happen in the wild, we still don't want there
to be a change of behavior when drivers switch from transitional helpers
to full helpers.

v2: Move check from setplane ioctl to setplane_internal rather than
adding an additional copy of the checks to the transitional plane
helpers.  (Daniel)

Cc: Daniel Vetter dan...@ffwll.ch
Testcase: igt/kms_cursor_crc
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84269
Signed-off-by: Matt Roper matthew.d.ro...@intel.com
---
 drivers/gpu/drm/drm_crtc.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index b914882..160647a 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2507,6 +2507,17 @@ static int __setplane_internal(struct drm_plane *plane,
goto out;
}
 
+   /* Give drivers some help against integer overflows */
+   if (crtc_w  INT_MAX ||
+   crtc_x  INT_MAX - (int32_t) crtc_w ||
+   crtc_h  INT_MAX ||
+   crtc_y  INT_MAX - (int32_t) crtc_h) {
+   DRM_DEBUG_KMS(Invalid CRTC coordinates %ux%u+%d+%d\n,
+ crtc_w, crtc_h, crtc_x, crtc_y);
+   return -ERANGE;
+   }
+
+
fb_width = fb-width  16;
fb_height = fb-height  16;
 
@@ -2591,17 +2602,6 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EINVAL;
 
-   /* Give drivers some help against integer overflows */
-   if (plane_req-crtc_w  INT_MAX ||
-   plane_req-crtc_x  INT_MAX - (int32_t) plane_req-crtc_w ||
-   plane_req-crtc_h  INT_MAX ||
-   plane_req-crtc_y  INT_MAX - (int32_t) plane_req-crtc_h) {
-   DRM_DEBUG_KMS(Invalid CRTC coordinates %ux%u+%d+%d\n,
- plane_req-crtc_w, plane_req-crtc_h,
- plane_req-crtc_x, plane_req-crtc_y);
-   return -ERANGE;
-   }
-
/*
 * First, find the plane, crtc, and fb objects.  If not available,
 * we don't bother to call the driver.
-- 
1.8.5.1

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

Re: [Intel-gfx] [PATCH] drm: Make integer overflow checking cover universal cursor updates (v2)

2015-04-13 Thread shuang . he 
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: 
shuang...@intel.com)
Task id: 6187
-Summary-
Platform  Delta  drm-intel-nightly  Series Applied
PNV -8  276/276  268/276
ILK  301/301  301/301
SNB -22  316/316  294/316
IVB -1  328/328  327/328
BYT  285/285  285/285
HSW  394/394  394/394
BDW  321/321  321/321
-Detailed-
Platform  Testdrm-intel-nightly  Series 
Applied
*PNV  igt@gem_tiled_pread_pwrite  PASS(4)  FAIL(1)PASS(1)
 PNV  igt@gem_userptr_blits@coherency-sync  CRASH(2)PASS(4)  CRASH(2)
 PNV  igt@gem_userptr_blits@coherency-unsync  CRASH(2)PASS(5)  CRASH(2)
*PNV  igt@gem_userptr_blits@forked-sync-swapping-mempressure-interruptible  
PASS(2)  FAIL(1)PASS(1)
 PNV  igt@gen3_render_linear_blits  FAIL(4)PASS(8)  FAIL(2)
 PNV  igt@gen3_render_mixed_blits  FAIL(5)PASS(7)  FAIL(2)
 PNV  igt@gen3_render_tiledx_blits  FAIL(5)PASS(8)  FAIL(2)
 PNV  igt@gen3_render_tiledy_blits  FAIL(4)PASS(7)  FAIL(2)
 SNB  igt@kms_cursor_crc@cursor-size-change  NSPT(2)PASS(1)  NSPT(2)
 SNB  igt@kms_flip_event_leak  NSPT(2)PASS(1)  NSPT(2)
 SNB  igt@kms_mmio_vs_cs_flip@setcrtc_vs_cs_flip  NSPT(3)PASS(1)  
NSPT(2)
 SNB  igt@kms_mmio_vs_cs_flip@setplane_vs_cs_flip  NSPT(3)PASS(1)  
NSPT(2)
 SNB  igt@kms_rotation_crc@primary-rotation  NSPT(3)PASS(1)  NSPT(2)
 SNB  igt@kms_rotation_crc@sprite-rotation  NSPT(3)PASS(3)  NSPT(2)
 SNB  igt@pm_rpm@cursor  NSPT(3)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@cursor-dpms  NSPT(3)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@dpms-mode-unset-non-lpsp  NSPT(3)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@dpms-non-lpsp  NSPT(3)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@drm-resources-equal  NSPT(3)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@fences  NSPT(3)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@fences-dpms  NSPT(3)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@gem-execbuf  NSPT(3)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@gem-mmap-cpu  NSPT(3)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@gem-mmap-gtt  NSPT(3)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@gem-pread  NSPT(3)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@i2c  NSPT(3)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@modeset-non-lpsp  NSPT(3)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@modeset-non-lpsp-stress-no-wait  NSPT(3)PASS(1)  
NSPT(2)
 SNB  igt@pm_rpm@pci-d3-state  NSPT(3)PASS(1)  NSPT(2)
 SNB  igt@pm_rpm@rte  NSPT(3)PASS(1)  NSPT(2)
 IVB  igt@gem_pwrite_pread@uncached-copy-performance  DMESG_WARN(1)PASS(8)  
DMESG_WARN(1)PASS(1)
(dmesg patch 
applied)drm:i915_hangcheck_elapsed[i915]]*ERROR*Hangcheck_timer_elapsed...blitter_ring_idle@Hangcheck
 timer elapsed... blitter ring idle
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx