linux-kernel - Re: [PATCH RESEND] xhci: Use xhci_get_virt_ep() to validate ep

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:   Fri, 29 Apr 2022 13:23:50 +0300
From:   Mathias Nyman <mathias.nyman@...ux.intel.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Mayank Rana <quic_mrana@...cinc.com>, peter.chen@...nel.org,
        balbi@...nel.org, stern@...land.harvard.edu,
        chunfeng.yun@...iatek.com, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org
Subject: Re: [PATCH RESEND] xhci: Use xhci_get_virt_ep() to validate ep_index

On 29.4.2022 13.02, Greg KH wrote:
> On Fri, Apr 29, 2022 at 12:49:59PM +0300, Mathias Nyman wrote:
>> On 28.4.2022 22.04, Mayank Rana wrote:
>>> ring_doorbell_for_active_rings() API is being called from
>>> multiple context. This specific API tries to get virt_dev
>>> based endpoint using passed slot_id and ep_index. Some caller
>>> API is having check against slot_id and ep_index using
>>> xhci_get_virt_ep() API whereas xhci_handle_cmd_config_ep() API
>>> only check ep_index against -1 value but not upper bound i.e.
>>> EP_CTX_PER_DEV. Hence use xhci_get_virt_ep() API to get virt_dev
>>> based endpoint which checks both slot_id and ep_index to get
>>> valid endpoint.
>>
>> ep_index upper bound is known to be in range as EP_CTX_PER_DEV is 31,
>> and ep_index = fls(u32 value)  - 1 - 1; 
>>
>> We can change to use xhci_get_virt_ep(), but this would be more useful
>> earlier in xhci_handle_cmd_config_ep() where we touch the ep before
>> calling ring_doorbell_for_active_rings()
>>
>> Also note that this codepath is only used for some prototype
>> xHC controller that probably never made it to the market about 10 years ago.
> 
> Can we just delete the codepath entirely then?

Probably.
Commit ac9d8fe7c6a8 USB: xhci: Add quirk for Fresco Logic xHCI hardware.
that added this states:

"This patch is for prototype hardware that will be given to other companies
for evaluation purposes only, and should not reach consumer hands.  Fresco
Logic's next chip rev should have this bug fixed."

Should we print some warning instead if this controller is used?
just in case. 
    
Thanks
Mathias