linux-kernel - Re: [PATCH] usb: dwc2: gadget: initialize max

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]

Message-ID: <a2ce248f-79d9-f9a3-f184-e033409fe2b8@synopsys.com>
Date:   Fri, 3 Dec 2021 05:49:36 +0000
From:   Minas Harutyunyan <Minas.Harutyunyan@...opsys.com>
To:     John Keeping <john@...anate.com>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
CC:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Argishti Aleksanyan <argishti.aleksanyan@...opsys.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: dwc2: gadget: initialize max_speed from params

Hi John,

On 12/2/2021 9:17 PM, John Keeping wrote:
> DWC2 may be paired with a full-speed PHY which is not capable of
> high-speed operation.  Report this correctly to the gadget core by
> setting max_speed from the core parameters.
> 
> Prior to commit 5324bad66f09f ("usb: dwc2: gadget: implement
> udc_set_speed()") this didn't cause the hardware to be configured
> incorrectly, although the speed may have been reported incorrectly.  But
> after that commit params.speed is updated based on a value passed in by
> the gadget core which may set it to a faster speed than is supported by
> the hardware.  Initialising the max_speed parameter ensures the speed
> passed to dwc2_gadget_set_speed() will be one supported by the hardware.
> 
> Fixes: 5324bad66f09f ("usb: dwc2: gadget: implement udc_set_speed()")
> Signed-off-by: John Keeping <john@...anate.com>
> ---
>   drivers/usb/dwc2/gadget.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index b884a83b26a6e..2bc03f41c70ad 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -4974,7 +4974,18 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
>   		hsotg->params.g_np_tx_fifo_size);
>   	dev_dbg(dev, "RXFIFO size: %d\n", hsotg->params.g_rx_fifo_size);
>   
> -	hsotg->gadget.max_speed = USB_SPEED_HIGH;
> +	switch (hsotg->params.speed) {
> +	case DWC2_SPEED_PARAM_LOW:
> +		hsotg->gadget.max_speed = USB_SPEED_LOW;
> +		break;
> +	case DWC2_SPEED_PARAM_FULL:
> +		hsotg->gadget.max_speed = USB_SPEED_FULL;
> +		break;
> +	default:
> +		hsotg->gadget.max_speed = USB_SPEED_HIGH;
> +		break;
> +	}
> +
>   	hsotg->gadget.ops = &dwc2_hsotg_gadget_ops;
>   	hsotg->gadget.name = dev_name(dev);
>   	hsotg->gadget.otg_caps = &hsotg->params.otg_caps;
> 

Just setting gadget's max_speed doesn't resolve described issue. After 
setting gadget's max_speed in your patch, it doesn't used in driver at 
all. Per me, you need also fix dwc2_gadget_set_speed() function by 
checking requested speed with max_speed: if requested speed higher than 
max_speed then set speed to max_speed.

Thanks,
Minas