linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: Mayank Rana <quic_mrana@quicinc.com>,
	peter.chen@kernel.org, balbi@kernel.org,
	stern@rowland.harvard.edu, chunfeng.yun@mediatek.com,
	gregkh@linuxfoundation.org
Cc: linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
Subject: Re: [PATCH RESEND] xhci: Use xhci_get_virt_ep() to validate ep_index
Date: Fri, 6 May 2022 16:58:40 +0300	[thread overview]
Message-ID: <8e186a1d-23f9-dec8-234e-9599b65a65c2@linux.intel.com> (raw)
In-Reply-To: <d7baf8d8-5643-fb90-8a14-f25a24317516@quicinc.com>

On 29.4.2022 22.01, Mayank Rana wrote:
> On 4/29/2022 3:13 AM, Mathias Nyman wrote:
>> On 29.4.2022 12.49, 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()
>>>
>> After a second look I would appreciate if you could clean up
>> ep_index checking in xhci_handle_cmd_config_ep()
>>
>> It currenty does some horrible typecasting.
>> ep_index is an unsigned int, so the fls() -1 operation might wrap it around.
>> Checking this was solved by typecasting a -1 to an unsigned int.
>>
>> if (ep_index != (unsigned int) -1)
>>
>> Thanks
>> Mathias
> 
> Thanks Mathias for review and suggestion here.
> let me try to clean up xhci_handle_cmd_config_ep() API based ep_index usage.
> 

Please don't spend too much time on this,
I'm going to remove this code as Greg suggested.

Should have replied earlier, sorry about the delay

Thanks
-Mathias 
 

      reply	other threads:[~2022-05-06 13:56 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-28 19:04 [PATCH RESEND] xhci: Use xhci_get_virt_ep() to validate ep_index Mayank Rana
2022-04-29  9:49 ` Mathias Nyman
2022-04-29 10:02   ` Greg KH
2022-04-29 10:23     ` Mathias Nyman
2022-04-29 10:36       ` Greg KH
2022-04-29 10:13   ` Mathias Nyman
2022-04-29 19:01     ` Mayank Rana
2022-05-06 13:58       ` Mathias Nyman [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8e186a1d-23f9-dec8-234e-9599b65a65c2@linux.intel.com \
    --to=mathias.nyman@linux.intel.com \
    --cc=balbi@kernel.org \
    --cc=chunfeng.yun@mediatek.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=peter.chen@kernel.org \
    --cc=quic_mrana@quicinc.com \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).