linux-kernel - Re: [PATCH] usb: chipidea: Fix missing resume call after suspend

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:   Mon, 24 Apr 2017 11:51:09 +0800
From:   Peter Chen <peter.chen@....com>
To:     Bernhard Walle <bernhard@...lle.de>
CC:     <linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: chipidea: Fix missing resume call after suspend

On Fri, Apr 21, 2017 at 12:10:53PM +0200, Bernhard Walle wrote:
> We have a i.MX53-based hardware (quite similar to the i.MX53 QSB from
> Freescale/NXP). I'm reading the ..../ci_hdrc.0/gadget/suspended sysfs
> file to find out whether a PC is connected to the USB gadget. With old
> kernel versions, this worked. However, with kernel 4.9 this didn't work.
> 
> When the host is suspended once, it never sets back the suspended status
> to 0. The reason is that this seems to be done in the resume handler,
> which should be executed in the interrupt handler:
> 
> udc_irq:
> 
> ...
>      if (USBi_PCI & intr) {
>              ci->gadget.speed = hw_port_is_high_speed(ci) ?
>                      USB_SPEED_HIGH : USB_SPEED_FULL;
>              if (ci->suspended && ci->driver->resume) {
>                      spin_unlock(&ci->lock);
>                      ci->driver->resume(&ci->gadget);
>                      spin_lock(&ci->lock);
>                      ci->suspended = 0;
>              }
>      }
> ...
> 
> However, ci->suspended is already 0 here because _gadget_stop_activity
> is called before. So the resume handler never gets called. The obvious
> solution is to not touch ci->suspended in _gadget_stop_activity and
> to trust the interrupt handler to set it back (and to modify it to set
> ci->suspended to 0 even if ci->driver->resume is NULL).

The current code logic is:
- When the resume is received from host, the ci->dirver->resume is
called, and suspended is cleared.
- When the reset is received from host, the isr_reset_handler is called,
and suspended is cleared by _gadget_stop_activity. Since reset is
called, so ci->driver->resume doesn't need to be called.

There is a patch to fix clear suspended even the ci->driver->resume is
NULL at v4.12-rc1.

      usb: chipidea: udc: update gadget state after bus resume

-- 

Best Regards,
Peter Chen