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.vetter@xxxxxxxx> > 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 <daniel@xxxxxxxx> > Testcase: igt/kms_cursor_crc > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84269 > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
