linux-kernel - Re: [PATCH v3] drm/display: Select DP helper for DRM_DP_AUX_CHARDEV and DRM_DP

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: <03b38035c582d3c2616010a87a365e1eb62a81de.camel@redhat.com>
Date:   Thu, 28 Apr 2022 14:07:39 -0400
From:   Lyude Paul <lyude@...hat.com>
To:     Javier Martinez Canillas <javierm@...hat.com>,
        linux-kernel@...r.kernel.org
Cc:     Jani Nikula <jani.nikula@...ux.intel.com>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>,
        dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v3] drm/display: Select DP helper for DRM_DP_AUX_CHARDEV
 and DRM_DP_CEC

Reviewed-by: Lyude Paul <lyude@...hat.com>

On Thu, 2022-04-28 at 10:22 +0200, Javier Martinez Canillas wrote:
> The DRM_DP_AUX_CHARDEV and DRM_DP_CEC Kconfig symbols enable code that use
> DP helper functions, that are only present if CONFIG_DRM_DISPLAY_DP_HELPER
> is also enabled.
> 
> But these don't select the DRM_DISPLAY_DP_HELPER symbol, meaning that it
> is possible to enable any of them without CONFIG_DRM_DISPLAY_DP_HELPER.
> 
> That will lead to the following linking errors with the mentioned config:
> 
>   LD      vmlinux.o
>   MODPOST vmlinux.symvers
>   MODINFO modules.builtin.modinfo
>   GEN     modules.builtin
>   LD      .tmp_vmlinux.kallsyms1
>   KSYMS   .tmp_vmlinux.kallsyms1.S
>   AS      .tmp_vmlinux.kallsyms1.S
>   LD      .tmp_vmlinux.kallsyms2
>   KSYMS   .tmp_vmlinux.kallsyms2.S
>   AS      .tmp_vmlinux.kallsyms2.S
>   LD      vmlinux
>   SYSMAP  System.map
>   SORTTAB vmlinux
>   OBJCOPY arch/arm64/boot/Image
>   MODPOST modules-only.symvers
> ERROR: modpost: "drm_dp_dpcd_write"
> [drivers/gpu/drm/display/drm_display_helper.ko] undefined!
> ERROR: modpost: "drm_dp_read_desc"
> [drivers/gpu/drm/display/drm_display_helper.ko] undefined!
> ERROR: modpost: "drm_dp_dpcd_read"
> [drivers/gpu/drm/display/drm_display_helper.ko] undefined!
> make[1]: *** [scripts/Makefile.modpost:134: modules-only.symvers] Error 1
> make[1]: *** Deleting file 'modules-only.symvers'
> make: *** [Makefile:1749: modules] Error 2
> 
> Besides making these symbols to select CONFIG_DRM_DISPLAY_DP_HELPER, make
> them to depend on DRM_DISPLAY_HELPER, since can't be enabled without it.
> 
> Note: It seems this has been an issue for a long time but was made easier
> to reproduce after the commit 1e0f66420b13 ("drm/display: Introduce a DRM
> display-helper module"). Adding a Fixes: tag just to make sure that this
> fix will be picked for stable once the mentioned change also lands there.
> 
> Fixes: 1e0f66420b13 ("drm/display: Introduce a DRM display-helper module")
> Signed-off-by: Javier Martinez Canillas <javierm@...hat.com>
> ---
> 
> Changes in v3:
> - Also make DRM_DP_AUX_CHARDEV and DRM_DP_CEC depend on DRM_DISPLAY_HELPER
>   (Thomas Zimmermann).
> 
> Changes in v2:
> - Explain better the issue in the change description.
> - Only select DRM_DISPLAY_DP_HELPER and not DRM_DISPLAY_HELPER.
> 
>  drivers/gpu/drm/display/Kconfig | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/Kconfig
> b/drivers/gpu/drm/display/Kconfig
> index f84f1b0cd23f..1b6e6af37546 100644
> --- a/drivers/gpu/drm/display/Kconfig
> +++ b/drivers/gpu/drm/display/Kconfig
> @@ -31,7 +31,8 @@ config DRM_DISPLAY_HDMI_HELPER
>  
>  config DRM_DP_AUX_CHARDEV
>         bool "DRM DP AUX Interface"
> -       depends on DRM
> +       depends on DRM && DRM_DISPLAY_HELPER
> +       select DRM_DISPLAY_DP_HELPER
>         help
>           Choose this option to enable a /dev/drm_dp_auxN node that allows
> to
>           read and write values to arbitrary DPCD registers on the DP aux
> @@ -39,7 +40,8 @@ config DRM_DP_AUX_CHARDEV
>  
>  config DRM_DP_CEC
>         bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
> -       depends on DRM
> +       depends on DRM && DRM_DISPLAY_HELPER
> +       select DRM_DISPLAY_DP_HELPER
>         select CEC_CORE
>         help
>           Choose this option if you want to enable HDMI CEC support for

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat