linux-kernel - Re: [PATCH] drm/bridge: dw-hdmi: fix SCDC configuration for ddc-i2c-bus

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: <ab34c440-c5ad-3c21-9fc0-d7c24aad4c59@baylibre.com>
Date:   Tue, 23 Apr 2019 14:47:46 +0200
From:   Neil Armstrong <narmstrong@...libre.com>
To:     Jonas Karlman <jonas@...boo.se>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>
Cc:     Archit Taneja <architt@...eaurora.org>,
        Andrzej Hajda <a.hajda@...sung.com>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Sean Paul <seanpaul@...omium.org>,
        Jernej Skrabec <jernej.skrabec@...l.net>,
        Heiko Stuebner <heiko@...ech.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/bridge: dw-hdmi: fix SCDC configuration for
 ddc-i2c-bus

On 21/04/2019 10:25, Jonas Karlman wrote:
> When ddc-i2c-bus property is used, a NULL pointer dereference is reported:
> 
> [   31.041669] Unable to handle kernel NULL pointer dereference at virtual address 00000008
> [   31.041671] pgd = 4d3c16f6
> [   31.041673] [00000008] *pgd=00000000
> [   31.041678] Internal error: Oops: 5 [#1] SMP ARM
> 
> [   31.041711] Hardware name: Rockchip (Device Tree)
> [   31.041718] PC is at i2c_transfer+0x8/0xe4
> [   31.041721] LR is at drm_scdc_read+0x54/0x84
> [   31.041723] pc : [<c073273c>]    lr : [<c05926c4>]    psr: 280f0013
> [   31.041725] sp : edffdad0  ip : 5ccb5511  fp : 00000058
> [   31.041727] r10: 00000780  r9 : edf91608  r8 : c11b0f48
> [   31.041728] r7 : 00000438  r6 : 00000000  r5 : 00000000  r4 : 00000000
> [   31.041730] r3 : edffdae7  r2 : 00000002  r1 : edffdaec  r0 : 00000000
> 
> [   31.041908] [<c073273c>] (i2c_transfer) from [<c05926c4>] (drm_scdc_read+0x54/0x84)
> [   31.041913] [<c05926c4>] (drm_scdc_read) from [<c0592858>] (drm_scdc_set_scrambling+0x30/0xbc)
> [   31.041919] [<c0592858>] (drm_scdc_set_scrambling) from [<c05cc0f4>] (dw_hdmi_update_power+0x1440/0x1610)
> [   31.041926] [<c05cc0f4>] (dw_hdmi_update_power) from [<c05cc574>] (dw_hdmi_bridge_enable+0x2c/0x70)
> [   31.041932] [<c05cc574>] (dw_hdmi_bridge_enable) from [<c05aed48>] (drm_bridge_enable+0x24/0x34)
> [   31.041938] [<c05aed48>] (drm_bridge_enable) from [<c0591060>] (drm_atomic_helper_commit_modeset_enables+0x114/0x220)
> [   31.041943] [<c0591060>] (drm_atomic_helper_commit_modeset_enables) from [<c05c3fe0>] (rockchip_atomic_helper_commit_tail_rpm+0x28/0x64)
> 
> hdmi->i2c may not be set when ddc-i2c-bus property is used in device tree.
> Fix this by using hdmi->ddc as the i2c adapter when calling drm_scdc_*().
> Also report that SCDC is not supported when there is no DDC bus.
> 
> Fixes: 264fce6cc2c1 ("drm/bridge: dw-hdmi: Add SCDC and TMDS Scrambling support")
> Signed-off-by: Jonas Karlman <jonas@...boo.se>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 7a47c65d8ecb..fd1c319a4ee0 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1046,6 +1046,10 @@ static bool dw_hdmi_support_scdc(struct dw_hdmi *hdmi)
>  	if (hdmi->version < 0x200a)
>  		return false;
>  
> +	/* Disable if no DDC bus */
> +	if (!hdmi->ddc)
> +		return false;
> +
>  	/* Disable if SCDC is not supported, or if an HF-VSDB block is absent */
>  	if (!display->hdmi.scdc.supported ||
>  	    !display->hdmi.scdc.scrambling.supported)
> @@ -1683,13 +1687,13 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
>  			 * Source Devices compliant shall set the
>  			 * Source Version = 1.
>  			 */
> -			drm_scdc_readb(&hdmi->i2c->adap, SCDC_SINK_VERSION,
> +			drm_scdc_readb(hdmi->ddc, SCDC_SINK_VERSION,
>  				       &bytes);
> -			drm_scdc_writeb(&hdmi->i2c->adap, SCDC_SOURCE_VERSION,
> +			drm_scdc_writeb(hdmi->ddc, SCDC_SOURCE_VERSION,
>  				min_t(u8, bytes, SCDC_MIN_SOURCE_VERSION));
>  
>  			/* Enabled Scrambling in the Sink */
> -			drm_scdc_set_scrambling(&hdmi->i2c->adap, 1);
> +			drm_scdc_set_scrambling(hdmi->ddc, 1);
>  
>  			/*
>  			 * To activate the scrambler feature, you must ensure
> @@ -1705,7 +1709,7 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
>  			hdmi_writeb(hdmi, 0, HDMI_FC_SCRAMBLER_CTRL);
>  			hdmi_writeb(hdmi, (u8)~HDMI_MC_SWRSTZ_TMDSSWRST_REQ,
>  				    HDMI_MC_SWRSTZ);
> -			drm_scdc_set_scrambling(&hdmi->i2c->adap, 0);
> +			drm_scdc_set_scrambling(hdmi->ddc, 0);
>  		}
>  	}
>  
> 

Nice catch !

Reviewed-by: Neil Armstrong <narmstrong@...libre.com>

Thanks,
Neil