linux-kernel - Re: [PATCH 2/3 v2] usb: dwc2: Error out of dwc2_hsotg_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:   Tue, 24 Oct 2017 08:33:58 +0000
From:   Minas Harutyunyan <Minas.Harutyunyan@...opsys.com>
To:     John Stultz <john.stultz@...aro.org>,
        lkml <linux-kernel@...r.kernel.org>
CC:     Wei Xu <xuwei5@...ilicon.com>, Guodong Xu <guodong.xu@...aro.org>,
        "Amit Pundir" <amit.pundir@...aro.org>,
        YongQin Liu <yongqin.liu@...aro.org>,
        John Youn <John.Youn@...opsys.com>,
        Minas Harutyunyan <Minas.Harutyunyan@...opsys.com>,
        Douglas Anderson <dianders@...omium.org>,
        Chen Yu <chenyu56@...wei.com>,
        Felipe Balbi <felipe.balbi@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
Subject: Re: [PATCH 2/3 v2] usb: dwc2: Error out of dwc2_hsotg_ep_disable()
 if we're in host mode

On 10/24/2017 1:33 AM, John Stultz wrote:
> We've found that while in host mode, using Android, if one runs
> the command:
>    stop adbd
> 
> The existing usb devices being utilized in host mode are disconnected.
> This is most visible with usb networking devices.
> 
> This seems to be due to adbd closing the file:
>    /dev/usb-ffs/adb/ep0
> Which calls ffs_ep0_release() and the following backtrace:
> 
> [<ffffff800875a430>] dwc2_hsotg_ep_disable+0x148/0x150
> [<ffffff800875a498>] dwc2_hsotg_udc_stop+0x60/0x110
> [<ffffff8008787950>] usb_gadget_remove_driver+0x58/0x78
> [<ffffff80087879e4>] usb_gadget_unregister_driver+0x74/0xe8
> [<ffffff80087850c0>] unregister_gadget+0x28/0x58
> [<ffffff800878511c>] unregister_gadget_item+0x2c/0x40
> [<ffffff8008790ea8>] ffs_data_clear+0xe8/0xf8
> [<ffffff8008790ed8>] ffs_data_reset+0x20/0x58
> [<ffffff8008793218>] ffs_data_closed+0x98/0xe8
> [<ffffff80087932d8>] ffs_ep0_release+0x20/0x30
> 
> Then when dwc2_hsotg_ep_disable() is called, we call
> kill_all_requests() which causes a bunch of the following
> messages:
> 
> dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
> dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
> init: Service 'adbd' (pid 1915) killed by signal 9
> init: Sending signal 9 to service 'adbd' (pid 1915) process group...
> init: Successfully killed process cgroup uid 0 pid 1915 in 0ms
> init: processing action (init.svc.adbd=stopped) from (/init.usb.configfs.rc:15)
> dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 8 - ChHltd set, but reason is unknown
> dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
> dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 12 - ChHltd set, but reason is unknown
> dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
> dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 15 - ChHltd set, but reason is unknown
> dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
> dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 3 - ChHltd set, but reason is unknown
> dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
> dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 4 - ChHltd set, but reason is unknown
> dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
> dwc2 f72c0000.usb: dwc2_update_urb_state_abn(): trimming xfer length
> 
> And the usb devices connected are basically hung at this point.
> 
> It seems like if we're in host mode, we probably shouldn't run
> the dwc2_hostg_ep_disable logic, so this patch returns an error
> in that case.
> 
> With this patch (along with the previous patch in this set), we avoid
> the mismatched interrupts and connected usb devices continue to function.
> 
> I'm not sure if some other solution would be better here, but this seems
> to work, so I wanted to send it out for input on what the right approach
> should be.
> 
> Cc: Wei Xu <xuwei5@...ilicon.com>
> Cc: Guodong Xu <guodong.xu@...aro.org>
> Cc: Amit Pundir <amit.pundir@...aro.org>
> Cc: YongQin Liu <yongqin.liu@...aro.org>
> Cc: John Youn <johnyoun@...opsys.com>
> Cc: Minas Harutyunyan <Minas.Harutyunyan@...opsys.com>
> Cc: Douglas Anderson <dianders@...omium.org>
> Cc: Chen Yu <chenyu56@...wei.com>
> Cc: Felipe Balbi <felipe.balbi@...ux.intel.com>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: linux-usb@...r.kernel.org
> Tested by: Minas Harutyunyan <hminas@...opsys.com>
> Reported-by: YongQin Liu <yongqin.liu@...aro.org>
> Signed-off-by: John Stultz <john.stultz@...aro.org>
> ---
>   drivers/usb/dwc2/gadget.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 0d8e09c..7fd0e38 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -4004,6 +4004,11 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
>   		return -EINVAL;
>   	}
>   
> +	if (hsotg->op_state != OTG_STATE_B_PERIPHERAL) {
> +		dev_err(hsotg->dev, "%s: called in host mode?\n", __func__);
> +		return -EINVAL;
> +	}
> +
>   	epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index);
>   
>   	spin_lock_irqsave(&hsotg->lock, flags);
> 
Acked-by: Minas Harutyunyan <hminas@...opsys.com>
Tested-by: Minas Harutyunyan <hminas@...opsys.com>