On Fri, Apr 03, 2015 at 02:27:46PM -0700, Matt Roper wrote: > Add tests for destination rectangle integer overflow before calling the > driver's check function. This will ensure that the transitional plane > helpers match the behavior of the full atomic helpers by always > returning -ERANGE for planes positioned beyond INT_MAX. > > Note that the legacy SetPlane ioctl itself also includes similar tests > for integer overflow, so the only case where this check really matters > is when legacy cursor operations get routed through the universal plane > interface internally. > > This issue was 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. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84269 > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> Since this is just for the cursor ioctl ... shouldn't we instead move these checks from drm_mode_setplane to __setplane_internal? That way other drivers also can rely upon these guarantees when implementing universal cursor support and we won't have a mismatch in what kind of plane properties can sneak through to drivers. -Daniel > --- > drivers/gpu/drm/drm_plane_helper.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c > index 33807e0..1e9e105 100644 > --- a/drivers/gpu/drm/drm_plane_helper.c > +++ b/drivers/gpu/drm/drm_plane_helper.c > @@ -417,6 +417,20 @@ int drm_plane_helper_commit(struct drm_plane *plane, > for (i = 0; i < 2; i++) > crtc_funcs[i] = crtc[i] ? crtc[i]->helper_private : NULL; > > + /* > + * Give drivers some help against integer overflows (and match the > + * behavior of the full atomic helpers). > + */ > + if (plane_state->crtc_w > INT_MAX || > + plane_state->crtc_x > INT_MAX - (int32_t) plane_state->crtc_w || > + plane_state->crtc_h > INT_MAX || > + plane_state->crtc_y > INT_MAX - (int32_t) plane_state->crtc_h) { > + DRM_DEBUG_ATOMIC("Invalid CRTC coordinates %ux%u+%d+%d\n", > + plane_state->crtc_w, plane_state->crtc_h, > + plane_state->crtc_x, plane_state->crtc_y); > + return -ERANGE; > + } > + > if (plane_funcs->atomic_check) { > ret = plane_funcs->atomic_check(plane, plane_state); > if (ret) > -- > 1.8.5.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
