linux-kernel - Re: [PATCH 1/3] thunderbolt: Read DP IN adapter first two dwords in one go

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: <abad9eb5-ef39-5936-7d9d-bdf53a81a1c4@fnarfbargle.com>
Date:   Fri, 4 Oct 2019 10:43:59 +0800
From:   Brad Campbell <lists2009@...rfbargle.com>
To:     Mika Westerberg <mika.westerberg@...ux.intel.com>,
        linux-kernel@...r.kernel.org
Cc:     Andreas Noever <andreas.noever@...il.com>,
        Michael Jamet <michael.jamet@...el.com>,
        Yehezkel Bernat <YehezkelShB@...il.com>,
        Lukas Wunner <lukas@...ner.de>,
        Dominik Brodowski <linux@...inikbrodowski.net>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Nicholas Johnson <nicholas.johnson-opensource@...look.com.au>,
        linux-pci@...r.kernel.org
Subject: Re: [PATCH 1/3] thunderbolt: Read DP IN adapter first two dwords in
 one go

On 1/10/19 6:29 pm, Mika Westerberg wrote:
> When we discover existing DP tunnels the code checks whether DP IN
> adapter port is enabled by calling tb_dp_port_is_enabled() before it
> continues the discovery process. On Light Ridge (gen 1) controller
> reading only the first dword of the DP IN config space causes subsequent
> access to the same DP IN port path config space to fail or return
> invalid data as can be seen in the below splat:
> 
>    thunderbolt 0000:07:00.0: CFG_ERROR(0:d): Invalid config space or offset
>    Call Trace:
>     tb_cfg_read+0xb9/0xd0
>     __tb_path_deactivate_hop+0x98/0x210
>     tb_path_activate+0x228/0x7d0
>     tb_tunnel_restart+0x95/0x200
>     tb_handle_hotplug+0x30e/0x630
>     process_one_work+0x1b4/0x340
>     worker_thread+0x44/0x3d0
>     kthread+0xeb/0x120
>     ? process_one_work+0x340/0x340
>     ? kthread_park+0xa0/0xa0
>     ret_from_fork+0x1f/0x30
> 
> If both DP In adapter config dwords are read in one go the issue does
> not reproduce. This is likely firmware bug but we can work it around by
> always reading the two dwords in one go. There should be no harm for
> other controllers either so can do it unconditionally.
> 
> Link: https://lkml.org/lkml/2019/8/28/160
> Reported-by: Brad Campbell <lists2009@...rfbargle.com>

Tested-by: Brad Campbell <lists2009@...rfbargle.com>

> Signed-off-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
> ---
>   drivers/thunderbolt/switch.c | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
> index 410bf1bceeee..8e712fbf8233 100644
> --- a/drivers/thunderbolt/switch.c
> +++ b/drivers/thunderbolt/switch.c
> @@ -896,12 +896,13 @@ int tb_dp_port_set_hops(struct tb_port *port, unsigned int video,
>    */
>   bool tb_dp_port_is_enabled(struct tb_port *port)
>   {
> -	u32 data;
> +	u32 data[2];
>   
> -	if (tb_port_read(port, &data, TB_CFG_PORT, port->cap_adap, 1))
> +	if (tb_port_read(port, data, TB_CFG_PORT, port->cap_adap,
> +			 ARRAY_SIZE(data)))
>   		return false;
>   
> -	return !!(data & (TB_DP_VIDEO_EN | TB_DP_AUX_EN));
> +	return !!(data[0] & (TB_DP_VIDEO_EN | TB_DP_AUX_EN));
>   }
>   
>   /**
> @@ -914,19 +915,21 @@ bool tb_dp_port_is_enabled(struct tb_port *port)
>    */
>   int tb_dp_port_enable(struct tb_port *port, bool enable)
>   {
> -	u32 data;
> +	u32 data[2];
>   	int ret;
>   
> -	ret = tb_port_read(port, &data, TB_CFG_PORT, port->cap_adap, 1);
> +	ret = tb_port_read(port, data, TB_CFG_PORT, port->cap_adap,
> +			   ARRAY_SIZE(data));
>   	if (ret)
>   		return ret;
>   
>   	if (enable)
> -		data |= TB_DP_VIDEO_EN | TB_DP_AUX_EN;
> +		data[0] |= TB_DP_VIDEO_EN | TB_DP_AUX_EN;
>   	else
> -		data &= ~(TB_DP_VIDEO_EN | TB_DP_AUX_EN);
> +		data[0] &= ~(TB_DP_VIDEO_EN | TB_DP_AUX_EN);
>   
> -	return tb_port_write(port, &data, TB_CFG_PORT, port->cap_adap, 1);
> +	return tb_port_write(port, data, TB_CFG_PORT, port->cap_adap,
> +			     ARRAY_SIZE(data));
>   }
>   
>   /* switch utility functions */
>