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@xxxxxxxxxxx>
Cc: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx>
Cc: Sean Paul <seanpaul@xxxxxxxxxxxx>
Cc: Fabio Estevam <festevam@xxxxxxxxx>
Cc: Deepak Rawat <drawat@xxxxxxxxxx>
Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
---
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,
--
2.29.2
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
