linux-kernel - Re: [Bug Report] drm/edid: drm_edid_override_connector

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] [day] [month] [year] [list]

Message-ID: <87jzpq1go5.fsf@intel.com>
Date:   Thu, 07 Dec 2023 11:54:34 +0200
From:   Jani Nikula <jani.nikula@...ux.intel.com>
To:     bbaa <bbaa@...a.fun>, maarten.lankhorst@...ux.intel.com,
        mripard@...nel.org, tzimmermann@...e.de, airlied@...il.com,
        daniel@...ll.ch
Cc:     linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [Bug Report] drm/edid: drm_edid_override_connector_update
 returns a incorrect value

On Thu, 07 Dec 2023, bbaa <bbaa@...a.fun> wrote:
> Hello everyone,
>
> drm_edid_override_connector_update seem return a incorrect value.
>
> drivers/gpu/drm/drm_edid.c (Linux 6.7-rc4)
>    2294	/**
>    2295	 * drm_edid_override_connector_update - add modes from override/firmware EDID
>    2296	 * @connector: connector we're probing
>    2297	 *
>    2298	 * Add modes from the override/firmware EDID, if available. Only to be used from
>    2299	 * drm_helper_probe_single_connector_modes() as a fallback for when DDC probe
>    2300	 * failed during drm_get_edid() and caused the override/firmware EDID to be
>    2301	 * skipped.
>    2302	 *
>    2303	 * Return: The number of modes added or 0 if we couldn't find any.
>    2304	 */

Thanks for the report. I've sent a patch to hopefully fix this [1].

[1] https://patchwork.freedesktop.org/patch/msgid/20231207093821.2654267-1-jani.nikula@intel.com

However, please read the documentation comment above: "Only to be used
from drm_helper_probe_single_connector_modes() ..."

The function is a fallback for a *very* specific and rare scenario.

> This will break the EDID override behavior on Nvidia graphics cards.
>
> NVIDIA/open-gpu-kernel-modules:
> kernel-open/nvidia-drm/nvidia-drm-connector.c:
>    103  #if defined(NV_DRM_CONNECTOR_HAS_OVERRIDE_EDID)   104      if 
> (connector->override_edid) {   105  #else   106      if 
> (drm_edid_override_connector_update(connector) > 0) {   107  #endif 
>    108          const struct drm_property_blob *edid = 
> connector->edid_blob_ptr;   109 
> drm_edid_override_connector_update(connector)  will return zero here.

That's an out-of-tree driver that doesn't follow the documentation
above. Drivers have no business calling the function.

All of the override/firmware EDID handling should be covered
transparently via the drm_edid_read*() and drm_get_edid() functions, and
the drivers shouldn't have to ever care about overrides, at all. Drivers
shouldn't really use connector->edid_blob_ptr directly either.

Please report and get that fixed downstream.


BR,
Jani.


-- 
Jani Nikula, Intel