diff mbox series

[RFC,v4,07/42] drm/vkms: Avoid reading beyond LUT array

Message ID 20240226211100.100108-8-harry.wentland@amd.com (mailing list archive)
State New, archived
Headers show
Series Color Pipeline API w/ VKMS | expand

Commit Message

Harry Wentland Feb. 26, 2024, 9:10 p.m. UTC
When the floor LUT index (drm_fixp2int(lut_index) is the last
index of the array the ceil LUT index will point to an entry
beyond the array. Make sure we guard against it and use the
value of the floor LUT index.

v3:
 - Drop bits from commit description that didn't contribute
   anything of value

Fixes: db1f254f2cfaf ("drm/vkms: Add support to 1D gamma LUT")
Signed-off-by: Harry Wentland <harry.wentland@amd.com>
Cc: Arthur Grillo <arthurgrillo@riseup.net>
Reviewed-by: Melissa Wen <mwen@igalia.com>
---
 drivers/gpu/drm/vkms/vkms_composer.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Comments

Melissa Wen March 12, 2024, 6:54 p.m. UTC | #1
On 02/26, Harry Wentland wrote:
> When the floor LUT index (drm_fixp2int(lut_index) is the last
> index of the array the ceil LUT index will point to an entry
> beyond the array. Make sure we guard against it and use the
> value of the floor LUT index.
> 
> v3:
>  - Drop bits from commit description that didn't contribute
>    anything of value
> 
> Fixes: db1f254f2cfaf ("drm/vkms: Add support to 1D gamma LUT")
> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> Cc: Arthur Grillo <arthurgrillo@riseup.net>
> Reviewed-by: Melissa Wen <mwen@igalia.com>

Same. Already applied upstream:
https://cgit.freedesktop.org/drm/drm-misc/commit/drivers/gpu/drm?id=2fee84030d12d9fddfa874e4562d71761a129277

I guess you are working on top of asdn branch and it's okay to me.
I'm just mentioning to avoid confusion.

Melissa

> ---
>  drivers/gpu/drm/vkms/vkms_composer.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
> index d178f2a400f6..b90e446d5954 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -123,6 +123,8 @@ static u16 apply_lut_to_channel_value(const struct vkms_color_lut *lut, u16 chan
>  				      enum lut_channel channel)
>  {
>  	s64 lut_index = get_lut_index(lut, channel_value);
> +	u16 *floor_lut_value, *ceil_lut_value;
> +	u16 floor_channel_value, ceil_channel_value;
>  
>  	/*
>  	 * This checks if `struct drm_color_lut` has any gap added by the compiler
> @@ -130,11 +132,15 @@ static u16 apply_lut_to_channel_value(const struct vkms_color_lut *lut, u16 chan
>  	 */
>  	static_assert(sizeof(struct drm_color_lut) == sizeof(__u16) * 4);
>  
> -	u16 *floor_lut_value = (__u16 *)&lut->base[drm_fixp2int(lut_index)];
> -	u16 *ceil_lut_value = (__u16 *)&lut->base[drm_fixp2int_ceil(lut_index)];
> +	floor_lut_value = (__u16 *)&lut->base[drm_fixp2int(lut_index)];
> +	if (drm_fixp2int(lut_index) == (lut->lut_length - 1))
> +		/* We're at the end of the LUT array, use same value for ceil and floor */
> +		ceil_lut_value = floor_lut_value;
> +	else
> +		ceil_lut_value = (__u16 *)&lut->base[drm_fixp2int_ceil(lut_index)];
>  
> -	u16 floor_channel_value = floor_lut_value[channel];
> -	u16 ceil_channel_value = ceil_lut_value[channel];
> +	floor_channel_value = floor_lut_value[channel];
> +	ceil_channel_value = ceil_lut_value[channel];
>  
>  	return lerp_u16(floor_channel_value, ceil_channel_value,
>  			lut_index & DRM_FIXED_DECIMAL_MASK);
> -- 
> 2.44.0
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index d178f2a400f6..b90e446d5954 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -123,6 +123,8 @@  static u16 apply_lut_to_channel_value(const struct vkms_color_lut *lut, u16 chan
 				      enum lut_channel channel)
 {
 	s64 lut_index = get_lut_index(lut, channel_value);
+	u16 *floor_lut_value, *ceil_lut_value;
+	u16 floor_channel_value, ceil_channel_value;
 
 	/*
 	 * This checks if `struct drm_color_lut` has any gap added by the compiler
@@ -130,11 +132,15 @@  static u16 apply_lut_to_channel_value(const struct vkms_color_lut *lut, u16 chan
 	 */
 	static_assert(sizeof(struct drm_color_lut) == sizeof(__u16) * 4);
 
-	u16 *floor_lut_value = (__u16 *)&lut->base[drm_fixp2int(lut_index)];
-	u16 *ceil_lut_value = (__u16 *)&lut->base[drm_fixp2int_ceil(lut_index)];
+	floor_lut_value = (__u16 *)&lut->base[drm_fixp2int(lut_index)];
+	if (drm_fixp2int(lut_index) == (lut->lut_length - 1))
+		/* We're at the end of the LUT array, use same value for ceil and floor */
+		ceil_lut_value = floor_lut_value;
+	else
+		ceil_lut_value = (__u16 *)&lut->base[drm_fixp2int_ceil(lut_index)];
 
-	u16 floor_channel_value = floor_lut_value[channel];
-	u16 ceil_channel_value = ceil_lut_value[channel];
+	floor_channel_value = floor_lut_value[channel];
+	ceil_channel_value = ceil_lut_value[channel];
 
 	return lerp_u16(floor_channel_value, ceil_channel_value,
 			lut_index & DRM_FIXED_DECIMAL_MASK);