Userspace can set a damage clip with a negative coordinate, negative width or height or larger than the plane. This invalid values could cause issues in some HW or even worst enable security flaws.

v2: - add debug messages to let userspace know why atomic commit failed due invalid damage clips

Cc: Simon Ser contact@emersion.fr Cc: Gwan-gyeong Mun gwan-gyeong.mun@intel.com Cc: Sean Paul seanpaul@chromium.org Cc: Fabio Estevam festevam@gmail.com Cc: Deepak Rawat drawat@vmware.com Cc: dri-devel@lists.freedesktop.org Signed-off-by: José Roberto de Souza jose.souza@intel.com --- drivers/gpu/drm/drm_atomic_helper.c | 4 +- drivers/gpu/drm/drm_damage_helper.c | 59 ++++++++++++++++++++++++----- include/drm/drm_damage_helper.h | 4 +- 3 files changed, 55 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index ba1507036f26..c6b341ecae2c 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -897,7 +897,9 @@ drm_atomic_helper_check_planes(struct drm_device *dev,

drm_atomic_helper_plane_changed(state, old_plane_state, new_plane_state, plane);

- drm_atomic_helper_check_plane_damage(state, new_plane_state); + ret = drm_atomic_helper_check_plane_damage(state, new_plane_state); + if (ret) + return ret;

if (!funcs || !funcs->atomic_check) continue; diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c index 3a4126dc2520..69a557aaa8cf 100644 --- a/drivers/gpu/drm/drm_damage_helper.c +++ b/drivers/gpu/drm/drm_damage_helper.c @@ -33,6 +33,7 @@ #include <drm/drm_atomic.h> #include <drm/drm_damage_helper.h> #include <drm/drm_device.h> +#include <drm/drm_print.h>

/** * DOC: overview @@ -104,36 +105,76 @@ void drm_plane_enable_fb_damage_clips(struct drm_plane *plane) EXPORT_SYMBOL(drm_plane_enable_fb_damage_clips);

/** - * drm_atomic_helper_check_plane_damage - Verify plane damage on atomic_check. + * drm_atomic_helper_check_plane_damage - Verify plane damage clips on + * atomic_check. * @state: The driver state object. - * @plane_state: Plane state for which to verify damage. + * @plane_state: Plane state for which to verify damage clips. * - * This helper function makes sure that damage from plane state is discarded - * for full modeset. If there are more reasons a driver would want to do a full - * plane update rather than processing individual damage regions, then those - * cases should be taken care of here. + * This helper checks if all damage clips has valid values and makes sure that + * damage clips from plane state is discarded for full modeset. If there are + * more reasons a driver would want to do a full plane update rather than + * processing individual damage regions, then those cases should be taken care + * of here. * * Note that &drm_plane_state.fb_damage_clips == NULL in plane state means that * full plane update should happen. It also ensure helper iterator will return * &drm_plane_state.src as damage. + * + * Return: Zero on success, negative errno on failure. */ -void drm_atomic_helper_check_plane_damage(struct drm_atomic_state *state, - struct drm_plane_state *plane_state) +int drm_atomic_helper_check_plane_damage(struct drm_atomic_state *state, + struct drm_plane_state *plane_state) { + struct drm_mode_rect *damage_clips; struct drm_crtc_state *crtc_state; + unsigned int num_clips, w, h; + + num_clips = drm_plane_get_damage_clips_count(plane_state); + if (!num_clips) + return 0;

if (plane_state->crtc) { crtc_state = drm_atomic_get_new_crtc_state(state, plane_state->crtc);

if (WARN_ON(!crtc_state)) - return; + return 0;

if (drm_atomic_crtc_needs_modeset(crtc_state)) { drm_property_blob_put(plane_state->fb_damage_clips); plane_state->fb_damage_clips = NULL; + return 0; + } + } + + w = drm_rect_width(&plane_state->src) >> 16; + h = drm_rect_height(&plane_state->src) >> 16; + damage_clips = drm_plane_get_damage_clips(plane_state); + + for (; num_clips; num_clips--, damage_clips++) { + if (damage_clips->x1 < 0 || damage_clips->x2 < 0 || + damage_clips->y1 < 0 || damage_clips->y2 < 0) { + drm_dbg_atomic(state->dev, + "Invalid damage clip, negative coordinate\n"); + return -EINVAL; + } + + if (damage_clips->x2 < damage_clips->x1 || + damage_clips->y2 < damage_clips->y1) { + drm_dbg_atomic(state->dev, + "Invalid damage clip, negative width or height\n"); + return -EINVAL; + } + + if ((damage_clips->x2 - damage_clips->x1) > w || + (damage_clips->y2 - damage_clips->y1) > h) { + drm_dbg_atomic(state->dev, + "Invalid damage clip, width or height larger than plane\n"); + return -EINVAL; } } + + return 0; } EXPORT_SYMBOL(drm_atomic_helper_check_plane_damage);

diff --git a/include/drm/drm_damage_helper.h b/include/drm/drm_damage_helper.h index 40c34a5bf149..5e344d1a2b22 100644 --- a/include/drm/drm_damage_helper.h +++ b/include/drm/drm_damage_helper.h @@ -65,8 +65,8 @@ struct drm_atomic_helper_damage_iter { };

void drm_plane_enable_fb_damage_clips(struct drm_plane *plane); -void drm_atomic_helper_check_plane_damage(struct drm_atomic_state *state, - struct drm_plane_state *plane_state); +int drm_atomic_helper_check_plane_damage(struct drm_atomic_state *state, + struct drm_plane_state *plane_state); int drm_atomic_helper_dirtyfb(struct drm_framebuffer *fb, struct drm_file *file_priv, unsigned int flags, unsigned int color, struct drm_clip_rect *clips,