| 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: <87sf13zpmc.fsf@intel.com>
Date: Wed, 06 Mar 2024 11:16:59 +0200
From: Jani Nikula <jani.nikula@...ux.intel.com>
To: Hsin-Yi Wang <hsinyi@...omium.org>, Dmitry Baryshkov
<dmitry.baryshkov@...aro.org>, Douglas Anderson <dianders@...omium.org>
Cc: Neil Armstrong <neil.armstrong@...aro.org>, Jessica Zhang
<quic_jesszhan@...cinc.com>, Sam Ravnborg <sam@...nborg.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>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/5] drm/edid: Add a function to match EDID with
identity
On Tue, 05 Mar 2024, Hsin-Yi Wang <hsinyi@...omium.org> wrote:
> Create a type drm_edid_ident as the identity of an EDID. Currently it
> contains panel id and monitor name.
>
> Create a function that can match a given EDID and an identity:
> 1. Reject if the panel id doesn't match.
> 2. If name is not null in identity, try to match it in the detailed timing
> blocks. Note that some panel vendors put the monitor name after
> EDID_DETAIL_MONITOR_STRING.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@...omium.org>
> ---
> v3->v4:
> 1. add a type drm_edid_ident
> 2. match name -> match identity. Modify function to use edid iterators.
> ---
> drivers/gpu/drm/drm_edid.c | 76 ++++++++++++++++++++++++++++++++++++++
> include/drm/drm_edid.h | 8 ++++
> 2 files changed, 84 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index f9e09f327f81..5e7e69e0e345 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -102,6 +102,11 @@ struct detailed_mode_closure {
> int modes;
> };
>
> +struct drm_edid_ident_closure {
> + const struct drm_edid_ident *ident;
> + bool matched;
> +};
More like drm_edid_match_closure.
> +
> #define LEVEL_DMT 0
> #define LEVEL_GTF 1
> #define LEVEL_GTF2 2
> @@ -5455,6 +5460,77 @@ drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db)
> connector->audio_latency[0], connector->audio_latency[1]);
> }
>
> +static void
> +match_identity(const struct detailed_timing *timing, void *data)
> +{
> + struct drm_edid_ident_closure *closure = data;
> + unsigned int i, j;
> + const char *str = closure->ident->name;
> + unsigned int buflen = strlen(str);
> + unsigned int size = ARRAY_SIZE(timing->data.other_data.data.str.str);
> +
> + if (buflen > size ||
> + !(is_display_descriptor(timing, EDID_DETAIL_MONITOR_NAME) ||
> + is_display_descriptor(timing, EDID_DETAIL_MONITOR_STRING)))
> + return;
> +
> + for (i = 0; i < buflen; i++) {
> + char c = timing->data.other_data.data.str.str[i];
> +
> + if (c != str[i] || c == '\n')
> + break;
> + }
> +
> + if (i == buflen) {
This will never be true.
> + /* Allow trailing white spaces. */
> + for (j = i; j < size; j++) {
> + char c = timing->data.other_data.data.str.str[j];
> +
> + if (c == '\n') {
> + closure->matched = true;
> + return;
> + } else if (c != ' ') {
> + break;
> + }
> + }
> + if (j == size) {
> + closure->matched = true;
> + return;
> + }
> + }
Please let's use strcmp and friends instead of reinventing our own:
const char *name = closure->ident->name;
int name_len = strlen(name);
const char *desc = timing->data.other_data.data.str.str;
int desc_len = ARRAY_SIZE(timing->data.other_data.data.str.str);
if (name_len > desc_len)
return;
if (strncmp(name, desc, name_en))
return;
for (i = name_len; i < desc_len; i++) {
if (!isspace(desc[i]) && !desc[i])
return;
}
closure->matched = true;
> +}
> +
> +/**
> + * drm_edid_match_identity - match drm_edid with given identity
> + * @drm_edid: EDID
> + * @ident: the EDID identity to match with
> + *
> + * Check if the EDID matches with the given identity.
> + *
> + * Return: True if the given identity matched with EDID, false otherwise.
> + */
> +bool drm_edid_match_identity(const struct drm_edid *drm_edid,
> + const struct drm_edid_ident *ident)
Can we please just call this drm_edid_match()? Is the _identity in the
name somehow helpful?
> +{
> + if (!drm_edid || edid_extract_panel_id(drm_edid->edid) != ident->panel_id)
> + return false;
Side note, edid_extract_panel_id() could now be made to take struct
drm_edid.
> +
> + /* Match with name only if it's not NULL. */
> + if (ident->name) {
> + struct drm_edid_ident_closure closure = {
> + .ident = ident,
> + .matched = false,
> + };
> +
> + drm_for_each_detailed_block(drm_edid, match_identity, &closure);
> +
> + return closure.matched;
> + }
> +
> + return true;
> +}
> +EXPORT_SYMBOL(drm_edid_match_identity);
> +
> static void
> monitor_name(const struct detailed_timing *timing, void *data)
> {
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 9686a7cee6a6..01825a8954b6 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -312,6 +312,12 @@ struct edid {
> u8 checksum;
> } __packed;
>
> +/* EDID matching */
> +struct drm_edid_ident {
> + u32 panel_id;
> + const char *name;
> +};
> +
> #define EDID_PRODUCT_ID(e) ((e)->prod_code[0] | ((e)->prod_code[1] << 8))
>
> /* Short Audio Descriptor */
> @@ -412,6 +418,8 @@ struct edid *drm_get_edid(struct drm_connector *connector,
> struct i2c_adapter *adapter);
> const struct drm_edid *drm_edid_read_base_block(struct i2c_adapter *adapter);
> u32 drm_edid_get_panel_id(const struct drm_edid *drm_edid);
> +bool drm_edid_match_identity(const struct drm_edid *drm_edid,
> + const struct drm_edid_ident *ident);
> struct edid *drm_get_edid_switcheroo(struct drm_connector *connector,
> struct i2c_adapter *adapter);
> struct edid *drm_edid_duplicate(const struct edid *edid);
--
Jani Nikula, Intel
Powered by blists - more mailing lists