Hi,
Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx> writes:
>> Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx> writes:
>>> On 01/31/2017 02:08 PM, Felipe Balbi wrote:
>>>> If we're dealing with SuperSpeed endpoints, we need
>>>> to make sure to pass along the companion descriptor
>>>> and initialize fields needed by the Gadget
>>>> API. Eventually, f_fs.c should be converted to use
>>>> config_ep_by_speed() like all other functions,
>>>> though.
>>>>
>>>> Cc: <stable@xxxxxxxxxxxxxxx>
>>>> Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
>>>> ---
>>>>
>>>> Will be sent in a pull request during v4.11-rc
>>>>
>>>> drivers/usb/gadget/function/f_fs.c | 15 +++++++++++++--
>>>> 1 file changed, 13 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
>>>> index 87fccf611b69..86aba2ebb3ef 100644
>>>> --- a/drivers/usb/gadget/function/f_fs.c
>>>> +++ b/drivers/usb/gadget/function/f_fs.c
>>>> @@ -1833,11 +1833,14 @@ static int ffs_func_eps_enable(struct ffs_function *func)
>>>> spin_lock_irqsave(&func->ffs->eps_lock, flags);
>>>> while(count--) {
>>>> struct usb_endpoint_descriptor *ds;
>>>> + struct usb_ss_ep_comp_descriptor *comp_desc = NULL;
>>>> + int needs_comp_desc = false;
>>>> int desc_idx;
>>>>
>>>> - if (ffs->gadget->speed == USB_SPEED_SUPER)
>>>> + if (ffs->gadget->speed == USB_SPEED_SUPER) {
>>>> desc_idx = 2;
>>>> - else if (ffs->gadget->speed == USB_SPEED_HIGH)
>>>> + needs_comp_desc = true;
>>>> + } else if (ffs->gadget->speed == USB_SPEED_HIGH)
>>>> desc_idx = 1;
>>>> else
>>>> desc_idx = 0;
>>>> @@ -1854,6 +1857,14 @@ static int ffs_func_eps_enable(struct ffs_function *func)
>>>>
>>>> ep->ep->driver_data = ep;
>>>> ep->ep->desc = ds;
>>>> +
>>>> + comp_desc = (struct usb_ss_ep_comp_descriptor *)(ds +
>>>> + USB_DT_ENDPOINT_SIZE);
>>>> + ep->ep->maxburst = comp_desc->bMaxBurst + 1;
>>>> +
>>>> + if (needs_comp_desc)
>>>> + ep->ep->comp_desc = comp_desc;
>>>> +
>>>
>>> Please correct me if I'm wrong but wouldn't we read rubbish here if user
>>> provided us SS ep descriptor without companion descriptor?
>>
>> companion desc is required for SS endpoints, it's also required that
>> they follow EP desc. If user doesn't write it, don't they deserve the
>> errors they'll have?
>>
>
> But do we deserve to access potentially unallocated memory inside kernel
> each time when some malicious application requests this?;)
>
> In my humble opinion user should get -EINVAL or sth like this from
> write(desc, sizeof(desc)) instead of some random data in companion
> descriptor.
We are *already* getting random data in companion descriptor :-) But I
agree that -EINVAL would be really nice. Can you cook up a patch for
that?
--
balbi
Attachment:
signature.asc
Description: PGP signature