From: "José Roberto de Souza" <jose.souza@intel.com> To: intel-gfx@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org, "Gwan-gyeong Mun" <gwan-gyeong.mun@intel.com>, "Sean Paul" <seanpaul@chromium.org>, "José Roberto de Souza" <jose.souza@intel.com>, "Deepak Rawat" <drawat@vmware.com> Subject: [PATCH v5 1/6] drm/damage_helper: Check if damage clips has valid values Date: Sun, 13 Dec 2020 10:39:25 -0800 [thread overview] Message-ID: <20201213183930.349592-1-jose.souza@intel.com> (raw) 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, -- 2.29.2 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: "José Roberto de Souza" <jose.souza@intel.com> To: intel-gfx@lists.freedesktop.org Cc: Simon Ser <contact@emersion.fr>, dri-devel@lists.freedesktop.org, Sean Paul <seanpaul@chromium.org>, Deepak Rawat <drawat@vmware.com>, Fabio Estevam <festevam@gmail.com> Subject: [Intel-gfx] [PATCH v5 1/6] drm/damage_helper: Check if damage clips has valid values Date: Sun, 13 Dec 2020 10:39:25 -0800 [thread overview] Message-ID: <20201213183930.349592-1-jose.souza@intel.com> (raw) 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, -- 2.29.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next reply other threads:[~2020-12-13 18:39 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-13 18:39 José Roberto de Souza [this message] 2020-12-13 18:39 ` [Intel-gfx] [PATCH v5 1/6] drm/damage_helper: Check if damage clips has valid values José Roberto de Souza 2020-12-13 18:39 ` [Intel-gfx] [PATCH v5 2/6] drm/i915/display: Check plane damage clips José Roberto de Souza 2020-12-14 9:33 ` Mun, Gwan-gyeong 2020-12-13 18:39 ` [Intel-gfx] [PATCH v5 3/6] drm/i915/display/psr: Use plane damage clips to calculate damaged area José Roberto de Souza 2020-12-14 11:00 ` Mun, Gwan-gyeong 2020-12-14 13:58 ` Souza, Jose 2020-12-13 18:39 ` [Intel-gfx] [PATCH v5 4/6] drm/i915/display: Split and export main surface calculation from skl_check_main_surface() José Roberto de Souza 2020-12-13 18:39 ` [Intel-gfx] [PATCH v5 5/6] drm/i915/display/psr: Program plane's calculated offset to plane SF register José Roberto de Souza 2020-12-13 18:39 ` [Intel-gfx] [PATCH v5 6/6] HAX/DO_NOT_MERGE_IT: drm/i915/display: Enable PSR2 selective fetch for testing José Roberto de Souza 2020-12-13 18:51 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [v5,1/6] drm/damage_helper: Check if damage clips has valid values Patchwork 2020-12-13 18:54 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork 2020-12-13 19:18 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork 2020-12-13 21:52 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork 2020-12-14 8:55 ` [PATCH v5 1/6] " Simon Ser 2020-12-14 8:55 ` [Intel-gfx] " Simon Ser 2020-12-14 9:27 ` Mun, Gwan-gyeong 2020-12-14 9:27 ` [Intel-gfx] " Mun, Gwan-gyeong 2020-12-14 9:55 ` Daniel Vetter 2020-12-14 9:55 ` Daniel Vetter
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20201213183930.349592-1-jose.souza@intel.com \ --to=jose.souza@intel.com \ --cc=drawat@vmware.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=gwan-gyeong.mun@intel.com \ --cc=intel-gfx@lists.freedesktop.org \ --cc=seanpaul@chromium.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.