[PATCH RESEND 0/3] drm/atomic: restrict the size of of gamma / degamma LUTs

[PATCH RESEND 0/3] drm/atomic: restrict the size of of gamma / degamma LUTs

[Dmitry Baryshkov]

While picking up the Gamma correction patch for the msm driver I noticed
that kms_color@invalid-gamma-lut-sizes and
kms_color@invalid-degamma-lut-sizes tests fail. These tests attempt
submitting LUT tables greater than the size specified by the
corresponding property. The issue doesn't seem to be specific to msm
driver only. Add generic check that LUT size is not greater than the
size passed to drm_crtc_enable_color_mgmt().

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
Dmitry Baryshkov (3):
      drm/mode_object: add drm_object_immutable_property_get_value()
      drm/atomic: add max_size check to drm_property_replace_blob_from_id()
      drm/atomic: verify that gamma/degamma LUTs are not too big

 .../drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c    |  5 +++++
 drivers/gpu/drm/drm_atomic_uapi.c                  | 23 ++++++++++++++++++--
 drivers/gpu/drm/drm_mode_object.c                  | 25 ++++++++++++++++++++++
 drivers/gpu/drm/drm_property.c                     | 11 ++++++++++
 include/drm/drm_mode_object.h                      |  3 +++
 include/drm/drm_property.h                         |  1 +
 6 files changed, 66 insertions(+), 2 deletions(-)
---
base-commit: ea07a751fbf3743490a463ce6cc5828e6dc3c660
change-id: 20251114-drm-fix-lut-checks-4bb325e24110

Best regards,
-- 
With best wishes
Dmitry

[PATCH RESEND 1/3] drm/mode_object: add drm_object_immutable_property_get_value()

[Dmitry Baryshkov]

We have a helper to get property values for non-atomic drivers and
another one default property values for atomic drivers. In some cases we
need the ability to get value of immutable property, no matter what kind
of driver it is. Implement new property-related helper,
drm_object_immutable_property_get_value(), which lets the caller to get
the value of the immutable property.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/drm_mode_object.c | 25 +++++++++++++++++++++++++
 include/drm/drm_mode_object.h     |  3 +++
 2 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
index e943205a2394..ead8eeafd012 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -384,6 +384,31 @@ int drm_object_property_get_default_value(struct drm_mode_object *obj,
 }
 EXPORT_SYMBOL(drm_object_property_get_default_value);
 
+/**
+ * drm_object_immutable_property_get_value - retrieve the value of a property
+ * @obj: drm mode object to get property value from
+ * @property: property to retrieve
+ * @val: storage for the property value
+ *
+ * This function retrieves the softare state of the given immutable property for the given
+ * property.
+ *
+ * This function can be called directtly by both atomic and non-atomic drivers.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drm_object_immutable_property_get_value(struct drm_mode_object *obj,
+					    struct drm_property *property,
+					    uint64_t *val)
+{
+	if (WARN_ON(!(property->flags & DRM_MODE_PROP_IMMUTABLE)))
+		return -EINVAL;
+
+	return __drm_object_property_get_prop_value(obj, property, val);
+}
+EXPORT_SYMBOL(drm_object_immutable_property_get_value);
+
 /* helper for getconnector and getproperties ioctls */
 int drm_mode_object_get_properties(struct drm_mode_object *obj, bool atomic,
 				   uint32_t __user *prop_ptr,
diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
index c68edbd126d0..44a0d6f8d01f 100644
--- a/include/drm/drm_mode_object.h
+++ b/include/drm/drm_mode_object.h
@@ -133,6 +133,9 @@ int drm_object_property_get_value(struct drm_mode_object *obj,
 int drm_object_property_get_default_value(struct drm_mode_object *obj,
 					  struct drm_property *property,
 					  uint64_t *val);
+int drm_object_immutable_property_get_value(struct drm_mode_object *obj,
+					    struct drm_property *property,
+					    uint64_t *val);
 
 void drm_object_attach_property(struct drm_mode_object *obj,
 				struct drm_property *property,

-- 
2.47.3

[PATCH RESEND 2/3] drm/atomic: add max_size check to drm_property_replace_blob_from_id()

[Dmitry Baryshkov]

The function drm_property_replace_blob_from_id() allows checking whether
the blob size is equal to a predefined value. In case of variable-size
properties (like the gamma / degamma LUTs) we might want to check for
the blob size against the maximum, allowing properties of the size
lesser than the max supported by the hardware. Extend the function in
order to support such checks.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c |  5 +++++
 drivers/gpu/drm/drm_atomic_uapi.c                       |  7 +++++--
 drivers/gpu/drm/drm_property.c                          | 11 +++++++++++
 include/drm/drm_property.h                              |  1 +
 4 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index e027798ece03..d19631b5d9e1 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1677,6 +1677,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
 							&dm_plane_state->degamma_lut,
 							val, -1,
 							sizeof(struct drm_color_lut),
+							0,
 							&replaced);
 		dm_plane_state->base.color_mgmt_changed |= replaced;
 		return ret;
@@ -1695,6 +1696,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
 							&dm_plane_state->ctm,
 							val,
 							sizeof(struct drm_color_ctm_3x4), -1,
+							0,
 							&replaced);
 		dm_plane_state->base.color_mgmt_changed |= replaced;
 		return ret;
@@ -1703,6 +1705,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
 							&dm_plane_state->shaper_lut,
 							val, -1,
 							sizeof(struct drm_color_lut),
+							0,
 							&replaced);
 		dm_plane_state->base.color_mgmt_changed |= replaced;
 		return ret;
@@ -1716,6 +1719,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
 							&dm_plane_state->lut3d,
 							val, -1,
 							sizeof(struct drm_color_lut),
+							0,
 							&replaced);
 		dm_plane_state->base.color_mgmt_changed |= replaced;
 		return ret;
@@ -1724,6 +1728,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
 							&dm_plane_state->blend_lut,
 							val, -1,
 							sizeof(struct drm_color_lut),
+							0,
 							&replaced);
 		dm_plane_state->base.color_mgmt_changed |= replaced;
 		return ret;
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 85dbdaa4a2e2..7d2076f1006e 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -388,6 +388,7 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 					&state->degamma_lut,
 					val,
 					-1, sizeof(struct drm_color_lut),
+					0,
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
@@ -395,7 +396,7 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		ret = drm_property_replace_blob_from_id(dev,
 					&state->ctm,
 					val,
-					sizeof(struct drm_color_ctm), -1,
+					sizeof(struct drm_color_ctm), -1, 0,
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
@@ -404,6 +405,7 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 					&state->gamma_lut,
 					val,
 					-1, sizeof(struct drm_color_lut),
+					0,
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
@@ -546,6 +548,7 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 					val,
 					-1,
 					sizeof(struct drm_mode_rect),
+					0,
 					&replaced);
 		return ret;
 	} else if (property == plane->scaling_filter_property) {
@@ -741,7 +744,7 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
 		ret = drm_property_replace_blob_from_id(dev,
 				&state->hdr_output_metadata,
 				val,
-				sizeof(struct hdr_output_metadata), -1,
+				sizeof(struct hdr_output_metadata), -1, 0,
 				&replaced);
 		return ret;
 	} else if (property == config->aspect_ratio_property) {
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
index 596272149a35..5befe443135d 100644
--- a/drivers/gpu/drm/drm_property.c
+++ b/drivers/gpu/drm/drm_property.c
@@ -759,6 +759,7 @@ EXPORT_SYMBOL(drm_property_replace_blob);
  * @blob_id: the id of the new blob to replace with
  * @expected_size: expected size of the blob property
  * @expected_elem_size: expected size of an element in the blob property
+ * @max_size: the maximum size of the blob property for variable-size blobs
  * @replaced: if the blob was in fact replaced
  *
  * Look up the new blob from id, take its reference, check expected sizes of
@@ -773,6 +774,7 @@ int drm_property_replace_blob_from_id(struct drm_device *dev,
 					 uint64_t blob_id,
 					 ssize_t expected_size,
 					 ssize_t expected_elem_size,
+					 ssize_t max_size,
 					 bool *replaced)
 {
 	struct drm_property_blob *new_blob = NULL;
@@ -801,6 +803,15 @@ int drm_property_replace_blob_from_id(struct drm_device *dev,
 			drm_property_blob_put(new_blob);
 			return -EINVAL;
 		}
+
+		if (max_size > 0 &&
+		    new_blob->length > max_size) {
+			drm_dbg_atomic(dev,
+				       "[BLOB:%d] length %zu greater than max %zu\n",
+				       new_blob->base.id, new_blob->length, max_size);
+			drm_property_blob_put(new_blob);
+			return -EINVAL;
+		}
 	}
 
 	*replaced |= drm_property_replace_blob(blob, new_blob);
diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
index 082f29156b3e..aa49b5a42bb5 100644
--- a/include/drm/drm_property.h
+++ b/include/drm/drm_property.h
@@ -284,6 +284,7 @@ int drm_property_replace_blob_from_id(struct drm_device *dev,
 				      uint64_t blob_id,
 				      ssize_t expected_size,
 				      ssize_t expected_elem_size,
+				      ssize_t max_size,
 				      bool *replaced);
 int drm_property_replace_global_blob(struct drm_device *dev,
 				     struct drm_property_blob **replace,

-- 
2.47.3

[PATCH RESEND 3/3] drm/atomic: verify that gamma/degamma LUTs are not too big

[Dmitry Baryshkov]

The kernel specifies LUT table sizes in a separate property, however it
doesn't enforce it as a maximum. Some drivers implement max suze check
on their own in the atomic_check path. Other drivers simply ignore the
issue. Perform LUT size validation in the generic place.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 7d2076f1006e..178d983b2378 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -384,11 +384,19 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 	} else if (property == config->prop_vrr_enabled) {
 		state->vrr_enabled = val;
 	} else if (property == config->degamma_lut_property) {
+		u64 lut_size;
+
+		ret = drm_object_immutable_property_get_value(&crtc->base,
+							      config->degamma_lut_size_property,
+							      &lut_size);
+		if (ret)
+			return ret;
+
 		ret = drm_property_replace_blob_from_id(dev,
 					&state->degamma_lut,
 					val,
 					-1, sizeof(struct drm_color_lut),
-					0,
+					sizeof(struct drm_color_lut) * lut_size,
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
@@ -401,11 +409,19 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		state->color_mgmt_changed |= replaced;
 		return ret;
 	} else if (property == config->gamma_lut_property) {
+		u64 lut_size;
+
+		ret = drm_object_immutable_property_get_value(&crtc->base,
+							      config->gamma_lut_size_property,
+							      &lut_size);
+		if (ret)
+			return ret;
+
 		ret = drm_property_replace_blob_from_id(dev,
 					&state->gamma_lut,
 					val,
 					-1, sizeof(struct drm_color_lut),
-					0,
+					sizeof(struct drm_color_lut) * lut_size,
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;

-- 
2.47.3

Re: [PATCH RESEND 1/3] drm/mode_object: add drm_object_immutable_property_get_value()

[Thomas Zimmermann]

Hi

Am 10.12.25 um 20:42 schrieb Dmitry Baryshkov:
> We have a helper to get property values for non-atomic drivers and
> another one default property values for atomic drivers. In some cases we
> need the ability to get value of immutable property, no matter what kind
> of driver it is. Implement new property-related helper,
> drm_object_immutable_property_get_value(), which lets the caller to get
> the value of the immutable property.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>   drivers/gpu/drm/drm_mode_object.c | 25 +++++++++++++++++++++++++
>   include/drm/drm_mode_object.h     |  3 +++
>   2 files changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index e943205a2394..ead8eeafd012 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -384,6 +384,31 @@ int drm_object_property_get_default_value(struct drm_mode_object *obj,
>   }
>   EXPORT_SYMBOL(drm_object_property_get_default_value);
>   
> +/**
> + * drm_object_immutable_property_get_value - retrieve the value of a property
> + * @obj: drm mode object to get property value from
> + * @property: property to retrieve
> + * @val: storage for the property value
> + *
> + * This function retrieves the softare state of the given immutable property for the given
> + * property.

Sounds odd. Rather for "for the given mode object" ?

> + *
> + * This function can be called directtly by both atomic and non-atomic drivers.

'directly' or just remove the word from the sentence.

> + *
> + * Returns:
> + * Zero on success, error code on failure.
> + */
> +int drm_object_immutable_property_get_value(struct drm_mode_object *obj,
> +					    struct drm_property *property,
> +					    uint64_t *val)
> +{
> +	if (WARN_ON(!(property->flags & DRM_MODE_PROP_IMMUTABLE)))

Rather use drm_WARN_ON(property->dev, ...) here.

Best regards
Thomas

> +		return -EINVAL;
> +
> +	return __drm_object_property_get_prop_value(obj, property, val);
> +}
> +EXPORT_SYMBOL(drm_object_immutable_property_get_value);
> +
>   /* helper for getconnector and getproperties ioctls */
>   int drm_mode_object_get_properties(struct drm_mode_object *obj, bool atomic,
>   				   uint32_t __user *prop_ptr,
> diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
> index c68edbd126d0..44a0d6f8d01f 100644
> --- a/include/drm/drm_mode_object.h
> +++ b/include/drm/drm_mode_object.h
> @@ -133,6 +133,9 @@ int drm_object_property_get_value(struct drm_mode_object *obj,
>   int drm_object_property_get_default_value(struct drm_mode_object *obj,
>   					  struct drm_property *property,
>   					  uint64_t *val);
> +int drm_object_immutable_property_get_value(struct drm_mode_object *obj,
> +					    struct drm_property *property,
> +					    uint64_t *val);
>   
>   void drm_object_attach_property(struct drm_mode_object *obj,
>   				struct drm_property *property,
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH RESEND 2/3] drm/atomic: add max_size check to drm_property_replace_blob_from_id()

[Thomas Zimmermann]

Hi

Am 10.12.25 um 20:42 schrieb Dmitry Baryshkov:
> The function drm_property_replace_blob_from_id() allows checking whether
> the blob size is equal to a predefined value. In case of variable-size
> properties (like the gamma / degamma LUTs) we might want to check for
> the blob size against the maximum, allowing properties of the size
> lesser than the max supported by the hardware. Extend the function in
> order to support such checks.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c |  5 +++++
>   drivers/gpu/drm/drm_atomic_uapi.c                       |  7 +++++--
>   drivers/gpu/drm/drm_property.c                          | 11 +++++++++++
>   include/drm/drm_property.h                              |  1 +
>   4 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index e027798ece03..d19631b5d9e1 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -1677,6 +1677,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
>   							&dm_plane_state->degamma_lut,
>   							val, -1,
>   							sizeof(struct drm_color_lut),
> +							0,

The rest of the API uses -1 for an invalid/unknown argument. I'd stick 
with that instead of using 0.

>   							&replaced);
>   		dm_plane_state->base.color_mgmt_changed |= replaced;
>   		return ret;
> @@ -1695,6 +1696,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
>   							&dm_plane_state->ctm,
>   							val,
>   							sizeof(struct drm_color_ctm_3x4), -1,
> +							0,
>   							&replaced);
>   		dm_plane_state->base.color_mgmt_changed |= replaced;
>   		return ret;
> @@ -1703,6 +1705,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
>   							&dm_plane_state->shaper_lut,
>   							val, -1,
>   							sizeof(struct drm_color_lut),
> +							0,
>   							&replaced);
>   		dm_plane_state->base.color_mgmt_changed |= replaced;
>   		return ret;
> @@ -1716,6 +1719,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
>   							&dm_plane_state->lut3d,
>   							val, -1,
>   							sizeof(struct drm_color_lut),
> +							0,
>   							&replaced);
>   		dm_plane_state->base.color_mgmt_changed |= replaced;
>   		return ret;
> @@ -1724,6 +1728,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
>   							&dm_plane_state->blend_lut,
>   							val, -1,
>   							sizeof(struct drm_color_lut),
> +							0,
>   							&replaced);
>   		dm_plane_state->base.color_mgmt_changed |= replaced;
>   		return ret;
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 85dbdaa4a2e2..7d2076f1006e 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -388,6 +388,7 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>   					&state->degamma_lut,
>   					val,
>   					-1, sizeof(struct drm_color_lut),
> +					0,
>   					&replaced);
>   		state->color_mgmt_changed |= replaced;
>   		return ret;
> @@ -395,7 +396,7 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>   		ret = drm_property_replace_blob_from_id(dev,
>   					&state->ctm,
>   					val,
> -					sizeof(struct drm_color_ctm), -1,
> +					sizeof(struct drm_color_ctm), -1, 0,
>   					&replaced);
>   		state->color_mgmt_changed |= replaced;
>   		return ret;
> @@ -404,6 +405,7 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>   					&state->gamma_lut,
>   					val,
>   					-1, sizeof(struct drm_color_lut),
> +					0,
>   					&replaced);
>   		state->color_mgmt_changed |= replaced;
>   		return ret;
> @@ -546,6 +548,7 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>   					val,
>   					-1,
>   					sizeof(struct drm_mode_rect),
> +					0,
>   					&replaced);
>   		return ret;
>   	} else if (property == plane->scaling_filter_property) {
> @@ -741,7 +744,7 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>   		ret = drm_property_replace_blob_from_id(dev,
>   				&state->hdr_output_metadata,
>   				val,
> -				sizeof(struct hdr_output_metadata), -1,
> +				sizeof(struct hdr_output_metadata), -1, 0,
>   				&replaced);
>   		return ret;
>   	} else if (property == config->aspect_ratio_property) {
> diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
> index 596272149a35..5befe443135d 100644
> --- a/drivers/gpu/drm/drm_property.c
> +++ b/drivers/gpu/drm/drm_property.c
> @@ -759,6 +759,7 @@ EXPORT_SYMBOL(drm_property_replace_blob);
>    * @blob_id: the id of the new blob to replace with
>    * @expected_size: expected size of the blob property
>    * @expected_elem_size: expected size of an element in the blob property
> + * @max_size: the maximum size of the blob property for variable-size blobs
>    * @replaced: if the blob was in fact replaced
>    *
>    * Look up the new blob from id, take its reference, check expected sizes of
> @@ -773,6 +774,7 @@ int drm_property_replace_blob_from_id(struct drm_device *dev,
>   					 uint64_t blob_id,
>   					 ssize_t expected_size,
>   					 ssize_t expected_elem_size,
> +					 ssize_t max_size,

I'd put max_size before expected_size, so that the size of individual 
elements isn't located in the middle of them.

>   					 bool *replaced)
>   {
>   	struct drm_property_blob *new_blob = NULL;
> @@ -801,6 +803,15 @@ int drm_property_replace_blob_from_id(struct drm_device *dev,
>   			drm_property_blob_put(new_blob);
>   			return -EINVAL;
>   		}
> +
> +		if (max_size > 0 &&
> +		    new_blob->length > max_size) {
> +			drm_dbg_atomic(dev,
> +				       "[BLOB:%d] length %zu greater than max %zu\n",
> +				       new_blob->base.id, new_blob->length, max_size);
> +			drm_property_blob_put(new_blob);
> +			return -EINVAL;
> +		}

I'd first test for max_size before testing for expected size.

And shouldn't you also test for (max_size % expected_elem_size == 0)?

Best regards
Thomas

>   	}
>   
>   	*replaced |= drm_property_replace_blob(blob, new_blob);
> diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
> index 082f29156b3e..aa49b5a42bb5 100644
> --- a/include/drm/drm_property.h
> +++ b/include/drm/drm_property.h
> @@ -284,6 +284,7 @@ int drm_property_replace_blob_from_id(struct drm_device *dev,
>   				      uint64_t blob_id,
>   				      ssize_t expected_size,
>   				      ssize_t expected_elem_size,
> +				      ssize_t max_size,
>   				      bool *replaced);
>   int drm_property_replace_global_blob(struct drm_device *dev,
>   				     struct drm_property_blob **replace,
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Re: [PATCH RESEND 2/3] drm/atomic: add max_size check to drm_property_replace_blob_from_id()

[Dmitry Baryshkov]

On Thu, Dec 11, 2025 at 08:46:26AM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 10.12.25 um 20:42 schrieb Dmitry Baryshkov:
> > The function drm_property_replace_blob_from_id() allows checking whether
> > the blob size is equal to a predefined value. In case of variable-size
> > properties (like the gamma / degamma LUTs) we might want to check for
> > the blob size against the maximum, allowing properties of the size
> > lesser than the max supported by the hardware. Extend the function in
> > order to support such checks.
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > ---
> >   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c |  5 +++++
> >   drivers/gpu/drm/drm_atomic_uapi.c                       |  7 +++++--
> >   drivers/gpu/drm/drm_property.c                          | 11 +++++++++++
> >   include/drm/drm_property.h                              |  1 +
> >   4 files changed, 22 insertions(+), 2 deletions(-)
> > 
> > @@ -801,6 +803,15 @@ int drm_property_replace_blob_from_id(struct drm_device *dev,
> >   			drm_property_blob_put(new_blob);
> >   			return -EINVAL;
> >   		}
> > +
> > +		if (max_size > 0 &&
> > +		    new_blob->length > max_size) {
> > +			drm_dbg_atomic(dev,
> > +				       "[BLOB:%d] length %zu greater than max %zu\n",
> > +				       new_blob->base.id, new_blob->length, max_size);
> > +			drm_property_blob_put(new_blob);
> > +			return -EINVAL;
> > +		}
> 
> I'd first test for max_size before testing for expected size.
> 
> And shouldn't you also test for (max_size % expected_elem_size == 0)?

No, why? We are testing the values passed from the userspace, not the
kernel itself. E.g. we also don't have a test that (expected_size %
expected_elem_size == 0).

> 
> Best regards
> Thomas
> 
> >   	}
> >   	*replaced |= drm_property_replace_blob(blob, new_blob);
> > diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
> > index 082f29156b3e..aa49b5a42bb5 100644
> > --- a/include/drm/drm_property.h
> > +++ b/include/drm/drm_property.h
> > @@ -284,6 +284,7 @@ int drm_property_replace_blob_from_id(struct drm_device *dev,
> >   				      uint64_t blob_id,
> >   				      ssize_t expected_size,
> >   				      ssize_t expected_elem_size,
> > +				      ssize_t max_size,
> >   				      bool *replaced);
> >   int drm_property_replace_global_blob(struct drm_device *dev,
> >   				     struct drm_property_blob **replace,
> > 
> 
> -- 
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
> GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
> 
> 

-- 
With best wishes
Dmitry

Re: [PATCH RESEND 2/3] drm/atomic: add max_size check to drm_property_replace_blob_from_id()

[Thomas Zimmermann]

Hi

Am 28.12.25 um 01:53 schrieb Dmitry Baryshkov:
> On Thu, Dec 11, 2025 at 08:46:26AM +0100, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 10.12.25 um 20:42 schrieb Dmitry Baryshkov:
>>> The function drm_property_replace_blob_from_id() allows checking whether
>>> the blob size is equal to a predefined value. In case of variable-size
>>> properties (like the gamma / degamma LUTs) we might want to check for
>>> the blob size against the maximum, allowing properties of the size
>>> lesser than the max supported by the hardware. Extend the function in
>>> order to support such checks.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>>> ---
>>>    drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c |  5 +++++
>>>    drivers/gpu/drm/drm_atomic_uapi.c                       |  7 +++++--
>>>    drivers/gpu/drm/drm_property.c                          | 11 +++++++++++
>>>    include/drm/drm_property.h                              |  1 +
>>>    4 files changed, 22 insertions(+), 2 deletions(-)
>>>
>>> @@ -801,6 +803,15 @@ int drm_property_replace_blob_from_id(struct drm_device *dev,
>>>    			drm_property_blob_put(new_blob);
>>>    			return -EINVAL;
>>>    		}
>>> +
>>> +		if (max_size > 0 &&
>>> +		    new_blob->length > max_size) {
>>> +			drm_dbg_atomic(dev,
>>> +				       "[BLOB:%d] length %zu greater than max %zu\n",
>>> +				       new_blob->base.id, new_blob->length, max_size);
>>> +			drm_property_blob_put(new_blob);
>>> +			return -EINVAL;
>>> +		}
>> I'd first test for max_size before testing for expected size.
>>
>> And shouldn't you also test for (max_size % expected_elem_size == 0)?
> No, why? We are testing the values passed from the userspace, not the
> kernel itself. E.g. we also don't have a test that (expected_size %
> expected_elem_size == 0).

Well, OK.

Best regards
Thomas

>
>> Best regards
>> Thomas
>>
>>>    	}
>>>    	*replaced |= drm_property_replace_blob(blob, new_blob);
>>> diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
>>> index 082f29156b3e..aa49b5a42bb5 100644
>>> --- a/include/drm/drm_property.h
>>> +++ b/include/drm/drm_property.h
>>> @@ -284,6 +284,7 @@ int drm_property_replace_blob_from_id(struct drm_device *dev,
>>>    				      uint64_t blob_id,
>>>    				      ssize_t expected_size,
>>>    				      ssize_t expected_elem_size,
>>> +				      ssize_t max_size,
>>>    				      bool *replaced);
>>>    int drm_property_replace_global_blob(struct drm_device *dev,
>>>    				     struct drm_property_blob **replace,
>>>
>> -- 
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
>> GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
>>
>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)