linux-kernel - Re: [PATCH] xhci: Fix the lack of support for the Handle Port Configure Error

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]

Date:	Wed, 11 Feb 2015 19:30:46 +0200
From:	Mathias Nyman <mathias.nyman@...el.com>
To:	Gregory CLEMENT <gregory.clement@...e-electrons.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
CC:	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	Boris BREZILLON <boris.brezillon@...e-electrons.com>,
	Lior Amsalem <alior@...vell.com>,
	Tawfik Bayouk <tawfik@...vell.com>,
	Nadav Haklai <nadavh@...vell.com>,
	Shimmer Huang <shimmerh@...vell.com>,
	Guang Shen <gshen@...vell.com>, stable@...r.kernel.org
Subject: Re: [PATCH] xhci: Fix the lack of support for the Handle Port Configure
 Error

On 06.02.2015 17:20, Gregory CLEMENT wrote:
> From: Shimmer Huang <shimmerh@...vell.com>
> 
> Linux xHCI driver does not check the CEC bit in register PORTSC when
> handling port status events. If Port Configure Error for root hub port
> occurs, CEC bit in PORTSC would be set by xHC and remains 1. This
> happends when the root port fails to configure its link partner,
> e.g. the port fails to exchange port capabilities information using
> Port Capability LMPs.
> 
> Then the Port Status Change Events will be blocked until all status
> change bits(CEC is one of the change bits) are cleared('0') (refer to
> xHCI spec 4.19.2). Otherwise, the port status change event for this
> root port will not be generated anymore, then root port would look
> like “dead” for user and can’t be recovered until a Host Controller
> Reset(HCRST)
> 
> This patch is to check CEC bit and clear the CEC bit if it's set to 1
> in function handle_port_status().
> 
> [gregory.clement@...e-electrons.com: ported from 3.10 and added more
> explanations(from Shimmer) in the commit log]
> 
> Signed-off-by: Guang Shen <gshen@...vell.com>
> Signed-off-by: Shimmer Huang <shimmerh@...vell.com>
> Signed-off-by: Nadav Haklai <nadavh@...vell.com>
> Reviewed-by: Yehuda Yitschak <yehuday@...vell.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@...e-electrons.com>
> Cc: <stable@...r.kernel.org>
> ---
> Hi,
> 
> usually I tried to add a kernel version for the stable team, but for
> this patch I don't know since when it makes sens to apply it.
> 
> Gregory
> 
> 
>  drivers/usb/host/xhci-ring.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index e692e769c50c..45d8dd7e07f3 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -1541,6 +1541,13 @@ static void handle_port_status(struct xhci_hcd *xhci,
>  			port_id);
>  
>  	temp = readl(port_array[faked_port_index]);
> +
> +	if (temp & PORT_CEC) {
> +		xhci_dbg(xhci, "port failed to configure its link partner.\n");
> +		xhci_test_and_clear_bit(xhci, port_array,
> +					faked_port_index, PORT_CEC);
> +	}
> +
>  	if (hcd->state == HC_STATE_SUSPENDED) {
>  		xhci_dbg(xhci, "resume root hub\n");
>  		usb_hcd_resume_root_hub(hcd);
> 

Nice catch,

Looks like we're not handling the Config Error Change (CEC) at all in xhci.

I think we instead need to set a Config Error in the return status of the GetPortStatus
requests by setting USB_PORT_FEAT_C_PORT_CONFIG_ERROR in xhci_get_port_status().

This should cause a ClearPortFeature request, where we then can clear the bit in
xhci_clear_port_change_bit()

So going the long way round informing the usb core about the error instead of
immediately clearing the bit.

-Mathias



 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/