linux-kernel - Re: [PATCH 3/5] usb: xhci: Correct last context entry calculation for Configure Endpoint

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]

Date:	Tue, 24 Jun 2014 17:51:16 +0300
From:	Mathias Nyman <mathias.nyman@...ux.intel.com>
To:	Greg KH <gregkh@...uxfoundation.org>
CC:	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	dan.j.williams@...el.com, Julius Werner <jwerner@...omium.org>
Subject: Re: [PATCH 3/5] usb: xhci: Correct last context entry calculation
 for Configure Endpoint

On 06/24/2014 05:10 PM, Greg KH wrote:
> On Tue, Jun 24, 2014 at 05:14:42PM +0300, Mathias Nyman wrote:
>> From: Julius Werner <jwerner@...omium.org>
>>
>> The current XHCI driver recalculates the Context Entries field in the
>> Slot Context on every add_endpoint() and drop_endpoint() call. In the
>> case of drop_endpoint(), it seems to assume that the add_flags will
>> always contain every endpoint for the new configuration, which is not
>> necessarily correct if you don't make assumptions about how the USB core
>> uses the add_endpoint/drop_endpoint interface (add_flags only contains
>> endpoints that are new additions in the new configuration).
>>
>> Furthermore, EP0_FLAG is not consistently set in add_flags throughout
>> the lifetime of a device. This means that when all endpoints are
>> dropped, the Context Entries field can be set to 0 (which is invalid and
>> may cause a Parameter Error) or -1 (which is interpreted as 31 and
>> causes the driver to keep using the old, incorrect value).
>>
>> The only surefire way to set this field right is to also take all
>> existing endpoints into account, and to force the value to 1 (meaning
>> only EP0 is active) if no other endpoint is found. This patch implements
>> that as a single step in the final check_bandwidth() call and removes
>> the intermediary calculations from add_endpoint() and drop_endpoint().
>>
>> Signed-off-by: Julius Werner <jwerner@...omium.org>
>> Signed-off-by: Mathias Nyman <mathias.nyman@...ux.intel.com>
>> ---
>>  drivers/usb/host/xhci.c | 51 +++++++++++++++++--------------------------------
>>  1 file changed, 18 insertions(+), 33 deletions(-)
> 
> Is this something for older (i.e. stable) kernels?
> 

This was intentionally left out of stable, reasoning in a previous mail was:

"We discussed with Sarah that we should try this out and queue it for 
usb-linus. This clearly fixes the generation of a invalid slot context 
if all endpoints are dropped.

But because there hasn't been any reported issue about the other case 
this changes (always setting context entries to last valid ep in target 
configuration), and spec still has this tiny contradiction in this case, 
we should keep this out of stable. At least for now, until it gets some 
real world testing coverage."

The mail:
http://marc.info/?l=linux-usb&m=139879120000807&w=2

Or if you prefer this patch could go to usb-next instead.

-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/