[PATCH v2 2/2] drm/i915: Validate userspace-provided color management LUT's (v2)

[Shankar, Uma]

>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces at lists.freedesktop.org] On Behalf Of
>Matt Roper
>Sent: Friday, December 14, 2018 3:25 AM
>To: dri-devel at lists.freedesktop.org; intel-gfx at lists.freedesktop.org
>Cc: Shankar, Uma <uma.shankar at intel.com>; Sharma, Swati2
><swati2.sharma at intel.com>
>Subject: [PATCH v2 2/2] drm/i915: Validate userspace-provided color
>management LUT's (v2)
>
>We currently program userspace-provided gamma and degamma LUT's into our
>hardware without really checking to see whether they satisfy our hardware's
>rules.  We should try to catch tables that are invalid for our hardware early and
>reject the atomic transaction.
>
>All of our platforms that accept a degamma LUT expect that the entries in the
>LUT are always flat or increasing, never decreasing.  Also, our GLK and ICL
>platforms only accept degamma tables with r=g=b entries; so we should also add
>the relevant checks for that in anticipation of degamma support landing for those
>platforms.
>
>v2:
> - Use new API (single check function with bitmask of tests to apply)
> - Call helper for our gamma table as well (with no additional tests
>   specified) so that the table size will be validated.

Looks ok to me.
Reviewed-By: Uma Shankar <uma.shankar at intel.com>

>Cc: Uma Shankar <uma.shankar at intel.com>
>Cc: Swati Sharma <swati2.sharma at intel.com>
>Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>---
> drivers/gpu/drm/i915/intel_color.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/intel_color.c
>b/drivers/gpu/drm/i915/intel_color.c
>index 37fd9ddf762e..5ad4459a5f3c 100644
>--- a/drivers/gpu/drm/i915/intel_color.c
>+++ b/drivers/gpu/drm/i915/intel_color.c
>@@ -609,10 +609,29 @@ int intel_color_check(struct intel_crtc_state
>*crtc_state)  {
> 	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> 	size_t gamma_length, degamma_length;
>+	uint32_t tests = DRM_COLOR_LUT_INCREASING;
>
> 	degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size;
> 	gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size;
>
>+	/*
>+	 * All of our platforms mandate that the degamma curve be
>+	 * non-decreasing.  Additionally, GLK and gen11 only accept a single
>+	 * value for red, green, and blue in the degamma table.  Make sure
>+	 * userspace didn't try to pass us something we can't handle.
>+	 *
>+	 * We don't have any extra hardware constraints on the gamma table,
>+	 * so we just test that it's a proper size multiple
>+	 * (tablesize % entrysize == 0).
>+	 */
>+	if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 11)
>+		tests |= DRM_COLOR_LUT_EQUAL_CHANNELS;
>+
>+	if (drm_color_lut_check(crtc_state->base.degamma_lut, tests) != 0)
>+		return -EINVAL;
>+	if (drm_color_lut_check(crtc_state->base.gamma_lut, 0) != 0)
>+		return -EINVAL;
>+
> 	/*
> 	 * We allow both degamma & gamma luts at the right size or
> 	 * NULL.
>--
>2.14.4
>
>_______________________________________________
>dri-devel mailing list
>dri-devel at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel