| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f211044d-c797-4e10-a06e-10dce071f704@suse.de>
Date: Thu, 11 Dec 2025 08:46:26 +0100
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Harry Wentland <harry.wentland@....com>,
Leo Li <sunpeng.li@....com>, Rodrigo Siqueira <siqueira@...lia.com>,
Alex Deucher <alexander.deucher@....com>,
Christian König <christian.koenig@....com>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
amd-gfx@...ts.freedesktop.org
Subject: Re: [PATCH RESEND 2/3] drm/atomic: add max_size check to
drm_property_replace_blob_from_id()
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@....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)
Powered by blists - more mailing lists