linux-kernel - Re: [PATCH v2 2/3] DRM: Create new Content Protection connector property

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bc723ffd-73cd-0339-c2aa-42b3db5a3560@linaro.org>
Date:   Fri, 21 Apr 2023 20:07:00 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Mark Yacoub <markyacoub@...omium.org>,
        dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>
Cc:     seanpaul@...omium.org, dianders@...omium.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] DRM: Create new Content Protection connector
 property

On 21/04/2023 19:27, Mark Yacoub wrote:
> From: Mark Yacoub <markyacoub@...omium.org>

Nit: is there a reason for this header? My first impression is that it 
matches your outgoing name & email address and as such is not necessary.

Nit#2: subject should mention 'Key', as you are creating a property for 
the key.

> 
> [Why]
> To enable Protected Content, some drivers require a key to be injected
> from user space to enable HDCP on the connector.
> 
> [How]
> Create new "Content Protection Property" of type "Blob"

Generic observation is that the ability to inject HDCP keys manually 
seems to be quite unique to your hardware. As such, I think the debugfs 
or sysfs suits better in comparison to the DRM property.

> 
> Signed-off-by: Mark Yacoub <markyacoub@...omium.org>
> ---
>   drivers/gpu/drm/drm_atomic_uapi.c | 9 +++++++++
>   include/drm/drm_connector.h       | 6 ++++++
>   include/drm/drm_mode_config.h     | 6 ++++++
>   3 files changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index d867e7f9f2cd5..e20bc57cdb05c 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -749,6 +749,11 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>   		state->content_protection = val;
>   	} else if (property == config->hdcp_content_type_property) {
>   		state->hdcp_content_type = val;
> +	} else if (property == config->content_protection_key_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(
> +			dev, &state->content_protection_key, val, -1, -1,
> +			&replaced);
> +		return ret;
>   	} else if (property == connector->colorspace_property) {
>   		state->colorspace = val;
>   	} else if (property == config->writeback_fb_id_property) {
> @@ -843,6 +848,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>   		*val = state->content_protection;
>   	} else if (property == config->hdcp_content_type_property) {
>   		*val = state->hdcp_content_type;
> +	} else if (property == config->content_protection_key_property) {
> +		*val = state->content_protection_key ?
> +			       state->content_protection_key->base.id :
> +			       0;
>   	} else if (property == config->writeback_fb_id_property) {
>   		/* Writeback framebuffer is one-shot, write and forget */
>   		*val = 0;
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 7b5048516185c..2fbe51272bfeb 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -896,6 +896,12 @@ struct drm_connector_state {
>   	 */
>   	unsigned int content_protection;
>   
> +	/**
> +	 * @content_protection_key: DRM blob property for holding the Content
> +	 * Protection Key injected from user space.
> +	 */
> +	struct drm_property_blob *content_protection_key;
> +
>   	/**
>   	 * @colorspace: State variable for Connector property to request
>   	 * colorspace change on Sink. This is most commonly used to switch
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index e5b053001d22e..615d1e5f57562 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -887,6 +887,12 @@ struct drm_mode_config {
>   	 */
>   	struct drm_property *hdcp_content_type_property;
>   
> +	/**
> +	 * @content_protection_key_property: DRM blob property that receives the
> +	 * content protection key from user space to be injected into the kernel.
> +	 */
> +	struct drm_property *content_protection_key_property;
> +
>   	/* dumb ioctl parameters */
>   	uint32_t preferred_depth, prefer_shadow;
>   

-- 
With best wishes
Dmitry