Re: USB vulnerability

2016-07-29 Thread roswest 


On Thu Jul 28 2016 15:56:17 GMT-0400 (EDT), Alan Stern wrote:
> On Thu, 28 Jul 2016, Alan Stern wrote:
> 
>> Only bits 10..0 of the wMaxPacketSize field contain the maximum packet
>> size; bits 12..11 contain something else (valid only for high-speed
>> periodic endpoints) and bits 15..13 are reserved (see Table 9-13 in the
>> USB-2.0 spec).
>>
>> Furthermore, the value in bits 10..0 is never supposed to be larger
>> than 1024 (or less depending on the speed and the endpoint type).  We
>> should check these things in config.c/usb_parse_endpoint().
>>
>> I will whip up a patch for this shortly.
> 
> And here it is.  Rosie, can you or your intern check that this fixes 
> the problem?
> 
> Alan Stern
> 
> 
> 
> Index: usb-4.x/drivers/usb/core/config.c
> ===
> --- usb-4.x.orig/drivers/usb/core/config.c
> +++ usb-4.x/drivers/usb/core/config.c
> @@ -171,6 +171,31 @@ static void usb_parse_ss_endpoint_compan
>   ep, buffer, size);
>  }
>  
> +static const unsigned short low_speed_maxpacket_maxes[4] = {
> + [USB_ENDPOINT_XFER_CONTROL] = 8,
> + [USB_ENDPOINT_XFER_ISOC] = 0,
> + [USB_ENDPOINT_XFER_BULK] = 0,
> + [USB_ENDPOINT_XFER_INT] = 8,
> +};
> +static const unsigned short full_speed_maxpacket_maxes[4] = {
> + [USB_ENDPOINT_XFER_CONTROL] = 64,
> + [USB_ENDPOINT_XFER_ISOC] = 1023,
> + [USB_ENDPOINT_XFER_BULK] = 64,
> + [USB_ENDPOINT_XFER_INT] = 64,
> +};
> +static const unsigned short high_speed_maxpacket_maxes[4] = {
> + [USB_ENDPOINT_XFER_CONTROL] = 64,
> + [USB_ENDPOINT_XFER_ISOC] = 1024,
> + [USB_ENDPOINT_XFER_BULK] = 512,
> + [USB_ENDPOINT_XFER_INT] = 1023,
> +};
> +static const unsigned short super_speed_maxpacket_maxes[4] = {
> + [USB_ENDPOINT_XFER_CONTROL] = 512,
> + [USB_ENDPOINT_XFER_ISOC] = 1024,
> + [USB_ENDPOINT_XFER_BULK] = 1024,
> + [USB_ENDPOINT_XFER_INT] = 1024,
> +};
> +
>  static int usb_parse_endpoint(struct device *ddev, int cfgno, int inum,
>  int asnum, struct usb_host_interface *ifp, int num_ep,
>  unsigned char *buffer, int size)
> @@ -179,6 +204,8 @@ static int usb_parse_endpoint(struct dev
>   struct usb_endpoint_descriptor *d;
>   struct usb_host_endpoint *endpoint;
>   int n, i, j, retval;
> + unsigned int maxp;
> + const unsigned short *maxpacket_maxes;
>  
>   d = (struct usb_endpoint_descriptor *) buffer;
>   buffer += d->bLength;
> @@ -290,6 +317,42 @@ static int usb_parse_endpoint(struct dev
>   endpoint->desc.wMaxPacketSize = cpu_to_le16(8);
>   }
>  
> + /* Validate the wMaxPacketSize field */
> + maxp = usb_endpoint_maxp(>desc);
> +
> + /* Find the highest legal maxpacket size for this endpoint */
> + i = 0;  /* additional transactions per microframe */
> + switch (to_usb_device(ddev)->speed) {
> + case USB_SPEED_LOW:
> + maxpacket_maxes = low_speed_maxpacket_maxes;
> + break;
> + case USB_SPEED_FULL:
> + maxpacket_maxes = full_speed_maxpacket_maxes;
> + break;
> + case USB_SPEED_HIGH:
> + /* Bits 12..11 are allowed only for HS periodic endpoints */
> + if (usb_endpoint_xfer_int(d) || usb_endpoint_xfer_isoc(d)) {
> + i = maxp & (BIT(12) | BIT(11));
> + maxp &= ~i;
> + }
> + /* fallthrough */
> + default:
> + maxpacket_maxes = high_speed_maxpacket_maxes;
> + break;
> + case USB_SPEED_SUPER:
> + case USB_SPEED_SUPER_PLUS:
> + maxpacket_maxes = super_speed_maxpacket_maxes;
> + break;
> + }
> + j = maxpacket_maxes[usb_endpoint_type(>desc)];
> +
> + if (maxp > j) {
> + dev_warn(ddev, "config %d interface %d altsetting %d endpoint 
> 0x%X has invalid maxpacket %d, setting to %d\n",
> + cfgno, inum, asnum, d->bEndpointAddress, maxp, j);
> + maxp = j;
> + endpoint->desc.wMaxPacketSize = cpu_to_le16(i | maxp);
> + }
> +
>   /*
>* Some buggy high speed devices have bulk endpoints using
>* maxpacket sizes other than 512.  High speed HCDs may not
> @@ -297,9 +360,6 @@ static int usb_parse_endpoint(struct dev
>*/
>   if (to_usb_device(ddev)->speed == USB_SPEED_HIGH
>   && usb_endpoint_xfer_bulk(d)) {
> - unsigned maxp;
> -
> - maxp = usb_endpoint_maxp(>desc) & 0x07ff;
>   if (maxp != 512)
>   dev_warn(ddev, "config %d interface %d altsetting %d "
>   "bulk endpoint 0x%X has invalid maxpacket %d\n",
> 

Alan,

Yes, Jake confirmed that both this patch and commit c66f59ee5050
referenced in your other email prevent the crash.  However, with this
patch the keyboard is still able to connect.

Thanks!
Rosie Hall

Re: [PATCH V2 RFC 5/6] DT: bindings: bcm: Add Raspberry Pi Zero

2016-07-29 Thread Rob Herring 
On Tue, Jul 26, 2016 at 06:53:32PM +, Stefan Wahren wrote:
> Signed-off-by: Stefan Wahren 
> ---
>  .../devicetree/bindings/arm/bcm/brcm,bcm2835.txt   |4 
>  1 file changed, 4 insertions(+)

Acked-by: Rob Herring 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 RFC 2/6] usb: dwc2: Add DT properties to specify fifo size in host/otg mode

2016-07-29 Thread Rob Herring 
On Tue, Jul 26, 2016 at 06:53:29PM +, Stefan Wahren wrote:
> Currently the fifo sizes for host/otg mode are defined per platform
> and doesn't take the mode into account. So we will get errors
> like this:
> 
> dwc2 2098.usb: 256 invalid for host_nperio_tx_fifo_size.
> dwc2 2098.usb: Setting host_nperio_tx_fifo_size to 32
> 
> So add DT properties for these mode specific fifo sizes in order
> to define them in the devicetree and avoid these errors.
> 
> Signed-off-by: Stefan Wahren 
> ---
>  Documentation/devicetree/bindings/usb/dwc2.txt |3 +++

Acked-by: Rob Herring 

>  drivers/usb/dwc2/core.c|   25 
> ++--
>  2 files changed, 22 insertions(+), 6 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/6] binding-doc: usb: usb-device: add optional properties for power sequence

2016-07-29 Thread Rob Herring 
On Wed, Jul 20, 2016 at 05:40:26PM +0800, Peter Chen wrote:
> Add optional properties for power sequence.
> 
> Signed-off-by: Peter Chen 
> ---
>  Documentation/devicetree/bindings/usb/usb-device.txt | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)

This probably should be put into a binding doc for the specific hub 
chip, but this is fine for now.

Acked-by: Rob Herring 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/6] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library

2016-07-29 Thread Rob Herring 
On Wed, Jul 20, 2016 at 05:40:24PM +0800, Peter Chen wrote:
> Add binding doc for generic power sequence library.
> 
> Signed-off-by: Peter Chen 
> Acked-by: Philipp Zabel 
> ---
>  .../bindings/power/pwrseq/pwrseq-generic.txt   | 48 
> ++
>  1 file changed, 48 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt

Acked-by: Rob Herring 

> 
> diff --git 
> a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt 
> b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> new file mode 100644
> index 000..48bb3e1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
> @@ -0,0 +1,48 @@
> +The generic power sequence library
> +
> +Some hard-wired devices (eg USB/MMC) need to do power sequence before
> +the device can be enumerated on the bus, the typical power sequence
> +like: enable USB PHY clock, toggle reset pin, etc. But current
> +Linux device driver lacks of such code to do it, it may cause some
> +hard-wired devices works abnormal or can't be recognized by
> +controller at all. The power sequence will be done before this device
> +can be found at the bus.
> +
> +The power sequence properties is under the device node.
> +
> +Optional properties:
> +- clocks: the input clock for device.
> +- reset-gpios: Should specify the GPIO for reset.
> +- reset-duration-us: the duration in microsecond for assert reset signal.
> +
> +Below is the example of USB power sequence properties on USB device
> +nodes which have two level USB hubs.
> +
> + {
> + vbus-supply = <_usb_otg1_vbus>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_usb_otg1_id>;
> + status = "okay";
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> + genesys: hub@1 {
> + compatible = "usb5e3,608";
> + reg = <1>;
> +
> + clocks = < IMX6SX_CLK_CKO>;
> + reset-gpios = < 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
> + reset-duration-us = <10>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> + asix: ethernet@1 {
> + compatible = "usbb95,1708";
> + reg = <1>;
> +
> + clocks = < IMX6SX_CLK_IPG>;
> + reset-gpios = < 6 GPIO_ACTIVE_LOW>; /* 
> ethernet_rst */
> + reset-duration-us = <15>;
> + };
> + };
> +};
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [v3,2/6] power: add power sequence library

2016-07-29 Thread Matthias Kaehlcke 

Hi Peter,

Thanks for your work on this, a few comments inline


On 07/20/2016 02:40 AM, Peter Chen wrote:


...

+static int pwrseq_generic_on(struct device_node *np, struct pwrseq *pwrseq)
+{

...

+   if (gpiod_reset) {
+   u32 duration_us = 50;
+
+   of_property_read_u32(np, "reset-duration-us",
+   _us);
+   usleep_range(duration_us, duration_us + 10);
The end of the range could allow for more margin. Also consider busy 
looping for very short delays as in 
http://lxr.free-electrons.com/source/drivers/regulator/core.c#L2062

...

+static int pwrseq_generic_get(struct device_node *np, struct pwrseq *pwrseq)
+{
+   struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+   enum of_gpio_flags flags;
+   int reset_gpio, ret = 0;
+
+   pwrseq_gen->clk = of_clk_get_by_name(np, NULL);

This only gets the first of potentially multiple clocks, is that intended?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PACTH v3 0/2] usb: xhci: plat: Enable PM, async resume/suspend

2016-07-29 Thread robert . foss 
From: Robert Foss 

This series enables runtime PM and asynchronous resume/suspend support for
xhci-plat devices.

Changes since v1:
- Added Signed-off-by: Robert Foss 
- Added proper metadata tags to series.

Changes since v2:
- Added missing changelog to cover-letter.
- Added error checking to pm_runtime_get_sync().

Andrew Bresticker (2):
  usb: xhci: plat: Enable runtime PM
  usb: xhci: plat: Enable async suspend/resume

 drivers/usb/host/xhci-plat.c | 25 +++--
 1 file changed, 23 insertions(+), 2 deletions(-)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PACTH v3 1/2] usb: xhci: plat: Enable runtime PM

2016-07-29 Thread robert . foss 
From: Andrew Bresticker 

Enable runtime PM for the xhci-plat device so that the parent device
may implement runtime PM.

Signed-off-by: Andrew Bresticker 
Tested-by: Robert Foss 
Signed-off-by: Robert Foss 
---
 drivers/usb/host/xhci-plat.c | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index ed56bf9..4f39d4e 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -246,6 +246,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (ret)
goto dealloc_usb2_hcd;
 
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+
return 0;
 
 
@@ -274,6 +277,8 @@ static int xhci_plat_remove(struct platform_device *dev)
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct clk *clk = xhci->clk;
 
+   pm_runtime_disable(>dev);
+
usb_remove_hcd(xhci->shared_hcd);
usb_phy_shutdown(hcd->usb_phy);
 
@@ -292,7 +297,11 @@ static int xhci_plat_suspend(struct device *dev)
 {
struct usb_hcd  *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+   int ret;
 
+   ret = pm_runtime_get_sync(dev);
+   if (ret < 0)
+   return ret;
/*
 * xhci_suspend() needs `do_wakeup` to know whether host is allowed
 * to do wakeup during suspend. Since xhci_plat_suspend is currently
@@ -301,15 +310,26 @@ static int xhci_plat_suspend(struct device *dev)
 * reconsider this when xhci_plat_suspend enlarges its scope, e.g.,
 * also applies to runtime suspend.
 */
-   return xhci_suspend(xhci, device_may_wakeup(dev));
+   ret = xhci_suspend(xhci, device_may_wakeup(dev));
+   pm_runtime_put(dev);
+
+   return ret;
 }
 
 static int xhci_plat_resume(struct device *dev)
 {
struct usb_hcd  *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+   int ret;
+
+   ret = pm_runtime_get_sync(dev);
+   if (ret < 0)
+   return ret;
 
-   return xhci_resume(xhci, 0);
+   ret = xhci_resume(xhci, 0);
+   pm_runtime_put(dev);
+
+   return ret;
 }
 
 static const struct dev_pm_ops xhci_plat_pm_ops = {
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PACTH v3 2/2] usb: xhci: plat: Enable async suspend/resume

2016-07-29 Thread robert . foss 
From: Andrew Bresticker 

USB host controllers can take a significant amount of time to suspend
and resume, adding several hundred miliseconds to the kernel resume
time. Since the XHCI controller has no outside dependencies (other than
clocks, which are suspended late/resumed early), allow it to suspend and
resume asynchronously.

Signed-off-by: Andrew Bresticker 
Tested-by: Andrew Bresticker 
Tested-by: Robert Foss 
Signed-off-by: Robert Foss 
---
 drivers/usb/host/xhci-plat.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 4f39d4e..89aef0e 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -248,6 +248,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
 
pm_runtime_set_active(>dev);
pm_runtime_enable(>dev);
+   device_enable_async_suspend(>dev);
 
return 0;
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] musb: omap2430: do not assume balanced enable()/disable()

2016-07-29 Thread Andreas Kemnade 
The code assumes that omap2430_musb_enable() and
omap2430_musb_disable() is called in a balanced way. The
That fact is broken by the fact that musb_init_controller() calls
musb_platform_disable() to switch from unknown state to off state.

That means that phy_power_off() is called first so that
phy->power_count gets -1 and the phy is not enabled on phy_power_on().
In the probably common case of using the phy_twl4030, that
prevents also charging the battery and so makes further
kernel debugging hard.

The patch prevents phy_power_off() from being called when
it is already off.

Signed-off-by: Andreas Kemnade 
---
 drivers/usb/musb/omap2430.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index aecb934..c7ae117 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -415,9 +415,10 @@ static void omap2430_musb_disable(struct musb *musb)
struct device *dev = musb->controller;
struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
 
-   if (!WARN_ON(!musb->phy))
-   phy_power_off(musb->phy);
-
+   if (glue->enabled) {
+   if (!WARN_ON(!musb->phy))
+   phy_power_off(musb->phy);
+   }
if (glue->status != MUSB_UNKNOWN)
omap_control_usb_set_mode(glue->control_otghs,
USB_MODE_DISCONNECT);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: pwc over musb: 100% frame drop (lost) on high resolution stream

2016-07-29 Thread Matwey V. Kornilov 
Hello,

I've found that the following commit fixes the issue:

commit 7694ca6e1d6f01122f05039b81f70f64b1ec4063
Author: Viresh Kumar 
Date:   Fri Apr 22 16:58:42 2016 +0530

cpufreq: omap: Use generic platdev driver

The cpufreq-dt-platdev driver supports creation of cpufreq-dt platform
device now, reuse that and remove similar code from platform code.


2016-07-28 19:16 GMT+03:00 Matwey V. Kornilov :
> Hello,
>
> I've just bisected commit, which fixed the issue in v4.7
>
> commit 9fa64d6424adabf0e3a546ae24d01a62a927b342
> Merge: f55532a febce40
> Author: Rafael J. Wysocki 
> Date:   Sat Apr 2 01:07:17 2016 +0200
>
> Merge back intel_pstate fixes for v4.6.
>
> * pm-cpufreq:
>   intel_pstate: Avoid extra invocation of intel_pstate_sample()
>   intel_pstate: Do not set utilization update hook too early
>
> Unfortunately, intel_pstate branch doesn't have
> f551e13529833e052f75ec628a8af7b034af20f9 applied.
> I'll try to bisect this branch with the patch manually applied.
>
> 2016-07-27 20:34 GMT+03:00 Matwey V. Kornilov :
>> Hello,
>>
>> I've just biseced commit, which introduced this issue
>>
>> commit f551e13529833e052f75ec628a8af7b034af20f9
>> Author: Bin Liu 
>> Date:   Mon Apr 25 15:53:30 2016 -0500
>>
>> Revert "usb: musb: musb_host: Enable HCD_BH flag to handle urb
>> return in bottom half"
>>
>> I have not checked yet, if it was intentionnaly fixed.
>>
>> 2016-07-23 22:24 GMT+03:00 Matwey V. Kornilov :
>>> 2016-07-20 21:56 GMT+03:00 Matwey V. Kornilov :
 2016-07-20 18:06 GMT+03:00 Bin Liu :
> Hi,
>
> On Wed, Jul 20, 2016 at 05:44:56PM +0300, Matwey V. Kornilov wrote:
>> 2016-07-20 17:13 GMT+03:00 Bin Liu :
>> > Hi,
>> >
>> > On Wed, Jul 20, 2016 at 09:09:42AM +0300, Matwey V. Kornilov wrote:
>> >> 2016-07-20 0:34 GMT+03:00 Bin Liu :
>> >> > Hi,
>> >> >
>> >> > On Wed, Jul 20, 2016 at 12:25:44AM +0300, Matwey V. Kornilov wrote:
>> >> >> 2016-07-19 23:56 GMT+03:00 Bin Liu :
>> >> >> > Hi,
>> >> >> >
>> >> >> > On Tue, Jul 19, 2016 at 11:21:17PM +0300, mat...@sai.msu.ru 
>> >> >> > wrote:
>> >> >> >> Hello,
>> >> >> >>
>> >> >> >> I have Philips SPC 900 camera (0471:0329) connected to my 
>> >> >> >> AM335x based BeagleBoneBlack SBC.
>> >> >> >> I am sure that both of them are fine and work properly.
>> >> >> >> I am running Linux 4.6.4 (my kernel config is available at 
>> >> >> >> https://clck.ru/A2kQs ) and I've just discovered, that there is 
>> >> >> >> an issue with frame transfer when high resolution formats are 
>> >> >> >> used.
>> >> >> >>
>> >> >> >> The issue is the following. I use simple v4l2 example tool 
>> >> >> >> (taken from API docs), which source code is available at 
>> >> >> >> http://pastebin.com/grcNXxfe
>> >> >> >>
>> >> >> >> When I use (see line 488) 640x480 frames
>> >> >> >>
>> >> >> >> fmt.fmt.pix.width   = 640;
>> >> >> >> fmt.fmt.pix.height  = 480;
>> >> >> >>
>> >> >> >> then I get "select timeout" and don't get any frames.
>> >> >> >>
>> >> >> >> When I use 320x240 frames
>> >> >> >>
>> >> >> >> fmt.fmt.pix.width   = 320;
>> >> >> >> fmt.fmt.pix.height  = 240;
>> >> >> >>
>> >> >> >> then about 60% frames are missed. An example outpout of ./a.out 
>> >> >> >> -f is available at https://yadi.sk/d/aRka8xWPtSc4y
>> >> >> >> It looks like there are pauses between bulks of frames (frame 
>> >> >> >> counter and timestamp as returned from v4l2 API):
>> >> >> >>
>> >> >> >> 3 3705.142553
>> >> >> >> 8 3705.342533
>> >> >> >> 13 3705.542517
>> >> >> >> 110 3708.776208
>> >> >> >> 115 3708.976190
>> >> >> >> 120 3709.176169
>> >> >> >> 125 3709.376152
>> >> >> >> 130 3709.576144
>> >> >> >> 226 3712.807848
>> >> >> >>
>> >> >> >> When I use tiny 160x120 frames
>> >> >> >>
>> >> >> >> fmt.fmt.pix.width   = 160;
>> >> >> >> fmt.fmt.pix.height  = 120;
>> >> >> >>
>> >> >> >> then more frames are received. See output example at 
>> >> >> >> https://yadi.sk/d/DedBmH6ftSc9t
>> >> >> >> That is why I thought that everything was fine in May when used 
>> >> >> >> tiny xawtv window to check kernel OOPS presence (see 
>> >> >> >> http://www.spinics.net/lists/linux-usb/msg141188.html for 
>> >> >> >> reference)
>> >> >> >>
>> >> >> >> Even more. When I introduce USB hub between the host and the 
>> >> >> >> webcam, I can not receive even any 320x240 frames.
>> >> >> >>
>> >> >> >> I've managed to use ftrace to see what is going on when no 
>> >> >> >>

Re: [RFC] usb: phy: generic: get rid of VBUS handling

2016-07-29 Thread Robert Jarzmik 
Felipe Balbi  writes:

> Hi Robert,
>
> Robert Jarzmik  writes:
>> Felipe Balbi  writes:
>>
>>> Hi,
>>>
>>> Robert Jarzmik  writes:
 I don't know if there are other users than pxa, I'll make the assumption 
 there
 aren't.
>>>
>>> yeah, we can keep patches in linux-next for as long as possible. If
>>> nobody complains, we're good.
>>
>> Hi Felipe,
>>
>> Just a small update. I made the change, but I'm not submitting it yet
>
> thanks for the update :-)
Euh I think you have even more backlog in your mails than I do :)

The latest update is here :
https://patchwork.kernel.org/patch/9221755

This one is fully functional AFAIK.

Cheers.

--
Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: USB vulnerability

2016-07-29 Thread Alan Stern 
On Fri, 29 Jul 2016, Felipe Balbi wrote:

> Hi,
> 
> Alan Stern  writes:
> > On Fri, 29 Jul 2016, Felipe Balbi wrote:
> >
> >> slightly unrelated, but...
> >
> >> > -maxp = usb_endpoint_maxp(>desc) & 0x07ff;
> >> 
> >> usb_endpoint_maxp() should probably be updated to return only maximum
> >> packet size. Then we would need to introduce usb_endpoint_mult() or
> >> something along those lines to take care of the other valid bits.
> >
> > Yes, I thought of that too.  I didn't want to do it right now because
> > it would mean auditing all the host controller drivers (and perhaps
> > checking the device drivers as well).  Still, it is a good idea.
> 
> I have had that in my TODO queue for a few weeks, actually. I need to
> look at it because of some requirement I found on DWC3 which we're not
> always guaranteeing.
> 
> I could do the same thing XHCI did and have its own wrappers, but it's
> best to fix this once and for all.

I'll be happy to review your work when it's ready.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: xhci_hcd crash on linux 4.7.0

2016-07-29 Thread Alex Damian 
On Fri, Jul 29, 2016 at 2:53 PM, Greg KH  wrote:
> On Fri, Jul 29, 2016 at 10:58:03AM +0100, Alex Damian wrote:
>> Hi Greg,
>>
>> I managed to reproduce with a untainted kernel, see dmesg paste below.
>> The stack seemed corrupted as well ?
>>
>> I refered to it as a crash since after a couple of these issues, the
>> machine hard freezes - I set up a serial console via a USB cable, but
>> I don't get the kernel oops out of the machine. The network is also
>> dead before getting any data. I could not think of any other way to
>> get a console out of a Macbook - any ideas ?
>>
>> There is a progressive level of deterioration going on below, this is
>> why I'm adding multiple pastes. See the obviously invalid pointer
>> 0001 in 3rd paste below. Also, see the protection fault in
>> the last paste. To me, something is trampling all over memory, and it
>> is usb-related.
>
> Not good, thanks for reproducing it without the closed kernel drivers.
>
> If you disable the list debug kernel option, do you have any problems
> with the machine?  We aren't having any other reports of issues like
> this at the moment, which makes me worry that it's something unique to
> your situation/hardware.

I strongly suspect it's related to the macbook 12,1 hardware. I
haven't been able
to reproduce this with other machines, including other macbook
versions with the same peripherals.

This machine has never been stable in this particular peripheral configuration.
I had Apple run all HW diagnostics on the machine, I ran the memcheck
to verify that
the RAM is ok - all results are clean. The machine is very stable under Mac OSX.

> And you don't know that it's a USB problem, only that USB is the one
> that is showing the issue.  Anyone could be writing over memory.

True. However it seems particularly related to the USB mouse - that's
how I manage
to reproduce the error.

>
> Also, any chance you can use 'git bisect' to track down an offending
> commit?  I'm assuming that this used to work properly and something
> recently caused the issue, correct?

The earliest kernels I've tested are in the 3.3 range. All kernels
before 4.7 just lock up.
4.7 is the first kernel where I have meaningful dmesg errors before
locking up. As such,
there is very little that I can do to bisect :(.

>
> thanks,
>
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: USB vulnerability

2016-07-29 Thread Felipe Balbi 

Hi,

Alan Stern  writes:
> On Fri, 29 Jul 2016, Felipe Balbi wrote:
>
>> slightly unrelated, but...
>
>> > -  maxp = usb_endpoint_maxp(>desc) & 0x07ff;
>> 
>> usb_endpoint_maxp() should probably be updated to return only maximum
>> packet size. Then we would need to introduce usb_endpoint_mult() or
>> something along those lines to take care of the other valid bits.
>
> Yes, I thought of that too.  I didn't want to do it right now because
> it would mean auditing all the host controller drivers (and perhaps
> checking the device drivers as well).  Still, it is a good idea.

I have had that in my TODO queue for a few weeks, actually. I need to
look at it because of some requirement I found on DWC3 which we're not
always guaranteeing.

I could do the same thing XHCI did and have its own wrappers, but it's
best to fix this once and for all.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: gadget: Add per-lun inquiry string

2016-07-29 Thread Krzysztof Opasiak 


On 07/29/2016 01:45 PM, Philipp Gesang wrote:
> Introduce an attribute "inquiry_string" to the lun.
> 
> In some environments, e. g. BIOS boot menus, the inquiry string
> is the only information about devices presented to the user. The
> default string depends on the "cdrom" bit of the first lun as
> well as the kernel version and allows no further customization.
> So without access to the client it is not obvious which gadget is
> active at a given point and what any of the available luns might
> contain.
> 
> If "inquiry_string" is ignored or set to the empty string, the
> old behavior is preserved.
> 
> Signed-off-by: Philipp Gesang 
> ---
>  drivers/usb/gadget/function/f_mass_storage.c | 22 +-
>  drivers/usb/gadget/function/f_mass_storage.h |  1 +
>  drivers/usb/gadget/function/storage_common.c | 24 
>  drivers/usb/gadget/function/storage_common.h |  4 
>  4 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c 
> b/drivers/usb/gadget/function/f_mass_storage.c
> index 2505117..47d8248 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -1107,7 +1107,12 @@ static int do_inquiry(struct fsg_common *common, 
> struct fsg_buffhd *bh)
>   buf[5] = 0; /* No special options */
>   buf[6] = 0;
>   buf[7] = 0;
> - memcpy(buf + 8, common->inquiry_string, sizeof common->inquiry_string);
> + if (curlun->inquiry_string[0])
> + memcpy(buf + 8, curlun->inquiry_string,
> +sizeof(curlun->inquiry_string));
> + else
> + memcpy(buf + 8, common->inquiry_string,
> +sizeof(common->inquiry_string));
>   return 36;
>  }
>  
> @@ -3209,12 +3214,27 @@ static ssize_t fsg_lun_opts_nofua_store(struct 
> config_item *item,
>  
>  CONFIGFS_ATTR(fsg_lun_opts_, nofua);
>  
> +static ssize_t fsg_lun_opts_inquiry_string_show(struct config_item *item,
> + char *page)
> +{
> + return fsg_show_inquiry_string(to_fsg_lun_opts(item)->lun, page);
> +}
> +
> +static ssize_t fsg_lun_opts_inquiry_string_store(struct config_item *item,
> +  const char *page, size_t len)
> +{
> + return fsg_store_inquiry_string(to_fsg_lun_opts(item)->lun, page, len);
> +}
> +
> +CONFIGFS_ATTR(fsg_lun_opts_, inquiry_string);
> +
>  static struct configfs_attribute *fsg_lun_attrs[] = {
>   _lun_opts_attr_file,
>   _lun_opts_attr_ro,
>   _lun_opts_attr_removable,
>   _lun_opts_attr_cdrom,
>   _lun_opts_attr_nofua,
> + _lun_opts_attr_inquiry_string,
>   NULL,
>  };
>  
> diff --git a/drivers/usb/gadget/function/f_mass_storage.h 
> b/drivers/usb/gadget/function/f_mass_storage.h
> index b6a9918..99ac7ad 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.h
> +++ b/drivers/usb/gadget/function/f_mass_storage.h
> @@ -100,6 +100,7 @@ struct fsg_lun_config {
>   char removable;
>   char cdrom;
>   char nofua;
> + char inquiry_string[8 + 16 + 4 + 1];

Could you please make those calculations as some define (as they exist
in at least two places) and add some comments about meaning of those
magic numbers?

>  };
>  
>  struct fsg_config {
> diff --git a/drivers/usb/gadget/function/storage_common.c 
> b/drivers/usb/gadget/function/storage_common.c
> index 990df22..9b56859 100644
> --- a/drivers/usb/gadget/function/storage_common.c
> +++ b/drivers/usb/gadget/function/storage_common.c
> @@ -369,6 +369,12 @@ ssize_t fsg_show_removable(struct fsg_lun *curlun, char 
> *buf)
>  }
>  EXPORT_SYMBOL_GPL(fsg_show_removable);
>  
> +ssize_t fsg_show_inquiry_string(struct fsg_lun *curlun, char *buf)
> +{
> + return sprintf(buf, "%s\n", curlun->inquiry_string);
> +}
> +EXPORT_SYMBOL_GPL(fsg_show_inquiry_string);
> +
>  /*
>   * The caller must hold fsg->filesem for reading when calling this function.
>   */
> @@ -499,4 +505,22 @@ ssize_t fsg_store_removable(struct fsg_lun *curlun, 
> const char *buf,
>  }
>  EXPORT_SYMBOL_GPL(fsg_store_removable);
>  
> +ssize_t fsg_store_inquiry_string(struct fsg_lun *curlun, const char *buf,
> +  size_t count)
> +{
> + const size_t len = min(count, sizeof(curlun->inquiry_string));
> +
> + if (len == 0 || buf[0] == '\n')
> + curlun->inquiry_string[0] = 0;
> + else {
> + snprintf(curlun->inquiry_string,
> +  sizeof(curlun->inquiry_string), "%-28s", buf);
> + if (curlun->inquiry_string[len-1] == '\n')
> + curlun->inquiry_string[len-1] = ' ';
> + }

According to codding style you should probably do:

if (...) {
instruction;
} else {
instruction;
instruction;
/* ... */
}

in this case.

> +
> + return count;
> +}
> +EXPORT_SYMBOL_GPL(fsg_store_inquiry_string);
>

Re: USB vulnerability

2016-07-29 Thread Alan Stern 
On Fri, 29 Jul 2016, Felipe Balbi wrote:

> slightly unrelated, but...

> > -   maxp = usb_endpoint_maxp(>desc) & 0x07ff;
> 
> usb_endpoint_maxp() should probably be updated to return only maximum
> packet size. Then we would need to introduce usb_endpoint_mult() or
> something along those lines to take care of the other valid bits.

Yes, I thought of that too.  I didn't want to do it right now because
it would mean auditing all the host controller drivers (and perhaps
checking the device drivers as well).  Still, it is a good idea.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: xhci_hcd crash on linux 4.7.0

2016-07-29 Thread Greg KH 
On Fri, Jul 29, 2016 at 10:58:03AM +0100, Alex Damian wrote:
> Hi Greg,
> 
> I managed to reproduce with a untainted kernel, see dmesg paste below.
> The stack seemed corrupted as well ?
> 
> I refered to it as a crash since after a couple of these issues, the
> machine hard freezes - I set up a serial console via a USB cable, but
> I don't get the kernel oops out of the machine. The network is also
> dead before getting any data. I could not think of any other way to
> get a console out of a Macbook - any ideas ?
> 
> There is a progressive level of deterioration going on below, this is
> why I'm adding multiple pastes. See the obviously invalid pointer
> 0001 in 3rd paste below. Also, see the protection fault in
> the last paste. To me, something is trampling all over memory, and it
> is usb-related.

Not good, thanks for reproducing it without the closed kernel drivers.

If you disable the list debug kernel option, do you have any problems
with the machine?  We aren't having any other reports of issues like
this at the moment, which makes me worry that it's something unique to
your situation/hardware.

And you don't know that it's a USB problem, only that USB is the one
that is showing the issue.  Anyone could be writing over memory.

Also, any chance you can use 'git bisect' to track down an offending
commit?  I'm assuming that this used to work properly and something
recently caused the issue, correct?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PACTH v1 1/2] usb: xhci: plat: Enable runtime PM

2016-07-29 Thread Robert Foss 


On 2016-07-28 08:24 PM, Felipe Balbi wrote:


Hi,

robert.f...@collabora.com writes:

From: Andrew Bresticker 

Enable runtime PM for the xhci-plat device so that the parent device
may implement runtime PM.

Signed-off-by: Andrew Bresticker 
Tested-by: Robert Foss 
Signed-off-by: Robert Foss 
---
 drivers/usb/host/xhci-plat.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index ed56bf9..fbd45c2 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -246,6 +246,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (ret)
goto dealloc_usb2_hcd;

+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+
return 0;


@@ -274,6 +277,8 @@ static int xhci_plat_remove(struct platform_device *dev)
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct clk *clk = xhci->clk;

+   pm_runtime_disable(>dev);
+
usb_remove_hcd(xhci->shared_hcd);
usb_phy_shutdown(hcd->usb_phy);

@@ -292,7 +297,9 @@ static int xhci_plat_suspend(struct device *dev)
 {
struct usb_hcd  *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+   int ret;

+   pm_runtime_get_sync(dev);


you need to check for possible errors here. Ditto to the other occurence
below.



Thanks for the feedback (on this and the 0/2), I'll fix the issues and 
release a v3.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: lvstest: Remove deprecated create_singlethread_workqueue

2016-07-29 Thread Tejun Heo 
On Thu, Jul 28, 2016 at 02:15:11PM +0530, Bhaktipriya Shridhar wrote:
> The workqueue has a single work item(>rh_work) and hence
> doesn't require ordering. Also, it is not being used on a memory
> reclaim path. Hence, the singlethreaded workqueue has been replaced
> with the use of system_wq.
> 
> System workqueues have been able to handle high level of concurrency
> for a long time now and hence it's not required to have a singlethreaded
> workqueue just to gain concurrency. Unlike a dedicated per-cpu workqueue
> created with create_singlethread_workqueue(), system_wq allows multiple
> work items to overlap executions even on the same CPU; however, a
> per-cpu workqueue doesn't have any CPU locality or global ordering
> guarantee unless the target CPU is explicitly specified and thus the
> increase of local concurrency shouldn't make any difference.
> 
> The work item has been flushed in lvs_rh_disconnect() to ensure that
> there are no pending tasks while disconnecting the driver.
> 
> Signed-off-by: Bhaktipriya Shridhar 

Acked-by: Tejun Heo 

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: dwc2: Remove deprecated create_singlethread_workqueue

2016-07-29 Thread Tejun Heo 
On Thu, Jul 28, 2016 at 01:57:29PM +0530, Bhaktipriya Shridhar wrote:
> alloc_ordered_workqueue replaces the deprecated
> create_singlethread_workqueue.
> 
> There are multiple work items on the work queue, which require
> ordering. Hence, an ordered workqueue has been used.
> 
> The workqueue "wq_otg" is not being used on a memory reclaim path.
> Hence, WQ_MEM_RECLAIM has not been set.
> 
> Signed-off-by: Bhaktipriya Shridhar 

Acked-by: Tejun Heo 

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] usb: host: u132-hcd: Remove deprecated create_singlethread_workqueue

2016-07-29 Thread Tejun Heo 
Hello, Alan.

On Wed, Jul 27, 2016 at 04:45:19PM -0400, Alan Stern wrote:
> > Hmm... That doesn't really make them dependable during memory reclaim.
> 
> True.  But it does mean that they can't cause a deadlock by waiting
> indefinitely for some other memory to be paged out to the very device
> they are on the access pathway for.
> 
> > What happens when those allocations fail?
> 
> The same thing that happens when any allocation fails -- the original
> I/O request fails with -ENOMEM or the equivalent.  In the case of 
> usb-storage, this is likely to trigger error recovery, which will need 
> to allocate memory of its own...  A bad situation to get into.

All that would do is deferring the deadlock, right?  I'm not sure it
makes a lot of sense to protect an IO path against memory pressure
half-way.  It either can be depended during memory reclaim or it
can't.  The use of GFP_NOIO / ATOMIC is probably increases the chance
of IO errors under moderate memory pressure too when there are
dependable memory backing devices (and there better be) which can push
things forward if called upon.

Can MM people please chime in?  The question is about USB stoage
devices and memory reclaim.  USB doesn't guarantee forward progress
under memory pressure but tries a best-effort attempt with GFP_NOIO
and ATOMIC.  Is this the right thing to do?

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb:serial: Add Fintek F81532/534 driver

2016-07-29 Thread One Thousand Gnomes 
O
> +static int f81534_set_normal_register(struct usb_device *dev, u16 reg, u8 
> data)
> +{
> + size_t count = F81534_USB_MAX_RETRY;
> + int status;
> + u8 *tmp;
> +
> + tmp = kmalloc(sizeof(u8), GFP_KERNEL);
> + if (!tmp)
> + return -ENOMEM;

You end up doing huge numbers of tiny allocation and frees in some of the
code paths. I think it would be better to allocate them at a higher level
as they are not that cheap on CPU time.

> +static int f81534_read_data(struct usb_serial *usbserial, u32 address,
> + size_t size, unsigned char *buf)
> +{

Is a particularly good example - you do 4 mallocs plus two per byte of
data.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: gadget: Add per-lun inquiry string

2016-07-29 Thread Philipp Gesang 
Introduce an attribute "inquiry_string" to the lun.

In some environments, e. g. BIOS boot menus, the inquiry string
is the only information about devices presented to the user. The
default string depends on the "cdrom" bit of the first lun as
well as the kernel version and allows no further customization.
So without access to the client it is not obvious which gadget is
active at a given point and what any of the available luns might
contain.

If "inquiry_string" is ignored or set to the empty string, the
old behavior is preserved.

Signed-off-by: Philipp Gesang 
---
 drivers/usb/gadget/function/f_mass_storage.c | 22 +-
 drivers/usb/gadget/function/f_mass_storage.h |  1 +
 drivers/usb/gadget/function/storage_common.c | 24 
 drivers/usb/gadget/function/storage_common.h |  4 
 4 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_mass_storage.c 
b/drivers/usb/gadget/function/f_mass_storage.c
index 2505117..47d8248 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -1107,7 +1107,12 @@ static int do_inquiry(struct fsg_common *common, struct 
fsg_buffhd *bh)
buf[5] = 0; /* No special options */
buf[6] = 0;
buf[7] = 0;
-   memcpy(buf + 8, common->inquiry_string, sizeof common->inquiry_string);
+   if (curlun->inquiry_string[0])
+   memcpy(buf + 8, curlun->inquiry_string,
+  sizeof(curlun->inquiry_string));
+   else
+   memcpy(buf + 8, common->inquiry_string,
+  sizeof(common->inquiry_string));
return 36;
 }
 
@@ -3209,12 +3214,27 @@ static ssize_t fsg_lun_opts_nofua_store(struct 
config_item *item,
 
 CONFIGFS_ATTR(fsg_lun_opts_, nofua);
 
+static ssize_t fsg_lun_opts_inquiry_string_show(struct config_item *item,
+   char *page)
+{
+   return fsg_show_inquiry_string(to_fsg_lun_opts(item)->lun, page);
+}
+
+static ssize_t fsg_lun_opts_inquiry_string_store(struct config_item *item,
+const char *page, size_t len)
+{
+   return fsg_store_inquiry_string(to_fsg_lun_opts(item)->lun, page, len);
+}
+
+CONFIGFS_ATTR(fsg_lun_opts_, inquiry_string);
+
 static struct configfs_attribute *fsg_lun_attrs[] = {
_lun_opts_attr_file,
_lun_opts_attr_ro,
_lun_opts_attr_removable,
_lun_opts_attr_cdrom,
_lun_opts_attr_nofua,
+   _lun_opts_attr_inquiry_string,
NULL,
 };
 
diff --git a/drivers/usb/gadget/function/f_mass_storage.h 
b/drivers/usb/gadget/function/f_mass_storage.h
index b6a9918..99ac7ad 100644
--- a/drivers/usb/gadget/function/f_mass_storage.h
+++ b/drivers/usb/gadget/function/f_mass_storage.h
@@ -100,6 +100,7 @@ struct fsg_lun_config {
char removable;
char cdrom;
char nofua;
+   char inquiry_string[8 + 16 + 4 + 1];
 };
 
 struct fsg_config {
diff --git a/drivers/usb/gadget/function/storage_common.c 
b/drivers/usb/gadget/function/storage_common.c
index 990df22..9b56859 100644
--- a/drivers/usb/gadget/function/storage_common.c
+++ b/drivers/usb/gadget/function/storage_common.c
@@ -369,6 +369,12 @@ ssize_t fsg_show_removable(struct fsg_lun *curlun, char 
*buf)
 }
 EXPORT_SYMBOL_GPL(fsg_show_removable);
 
+ssize_t fsg_show_inquiry_string(struct fsg_lun *curlun, char *buf)
+{
+   return sprintf(buf, "%s\n", curlun->inquiry_string);
+}
+EXPORT_SYMBOL_GPL(fsg_show_inquiry_string);
+
 /*
  * The caller must hold fsg->filesem for reading when calling this function.
  */
@@ -499,4 +505,22 @@ ssize_t fsg_store_removable(struct fsg_lun *curlun, const 
char *buf,
 }
 EXPORT_SYMBOL_GPL(fsg_store_removable);
 
+ssize_t fsg_store_inquiry_string(struct fsg_lun *curlun, const char *buf,
+size_t count)
+{
+   const size_t len = min(count, sizeof(curlun->inquiry_string));
+
+   if (len == 0 || buf[0] == '\n')
+   curlun->inquiry_string[0] = 0;
+   else {
+   snprintf(curlun->inquiry_string,
+sizeof(curlun->inquiry_string), "%-28s", buf);
+   if (curlun->inquiry_string[len-1] == '\n')
+   curlun->inquiry_string[len-1] = ' ';
+   }
+
+   return count;
+}
+EXPORT_SYMBOL_GPL(fsg_store_inquiry_string);
+
 MODULE_LICENSE("GPL");
diff --git a/drivers/usb/gadget/function/storage_common.h 
b/drivers/usb/gadget/function/storage_common.h
index c3544e6..af74044 100644
--- a/drivers/usb/gadget/function/storage_common.h
+++ b/drivers/usb/gadget/function/storage_common.h
@@ -112,6 +112,7 @@ struct fsg_lun {
struct device   dev;
const char  *name;  /* "lun.name" */
const char  **name_pfx; /* "function.name" */
+   charinquiry_string[8 + 16 + 4 + 1];
 };
 
 static inline bool

Re: [RFC] usb: phy: generic: get rid of VBUS handling

2016-07-29 Thread Felipe Balbi 

Hi Robert,

Robert Jarzmik  writes:
> Felipe Balbi  writes:
>
>> Hi,
>>
>> Robert Jarzmik  writes:
>>> I don't know if there are other users than pxa, I'll make the assumption 
>>> there
>>> aren't.
>>
>> yeah, we can keep patches in linux-next for as long as possible. If
>> nobody complains, we're good.
>
> Hi Felipe,
>
> Just a small update. I made the change, but I'm not submitting it yet

thanks for the update :-)

> because my testings show me that even if the VBus is correctly
> detected, the enumeration fails, which means I have an error in my
> flow somewhere.

oh okay. S**t happens :-)

> The current patch is in [1], I'm still working on it.
>
> Cheers.
>
> -- 
> Robert
>
> [1] Current patch
> ---8>---
> From c801e9eba7cfe177a93b2c1675c8a19b408c8ac5 Mon Sep 17 00:00:00 2001
> From: Robert Jarzmik 
> Date: Thu, 7 Jul 2016 19:53:43 +0200
> Subject: [PATCH] usb: gadget: pxa27x: add phy notifier event handler
>
> In the legacy behavior, and USB phy, upon detection a VBus signal, was
> calling usb_gadget_vbus_(dis)connect().
>
> This model doesn't work if the phy is generic and doesn't have an
> adherence to the gadget API.
>
> Instead of relying on the phy to call the gadget API, hook up the phy
> notifier to report the VBus event, and upon it call the usb gadget API
> ourselves.
>
> This brings a new ordering problem, as before even if the usb_get_phy()
> was failing because the UDC was probed before the phy, the phy would
> call the gadget anyway, making the VBus connection event forwarded to
> the gadget. Now we rely on the notifier, we have to ensure the
> xxx_get_phy() does indeed work.
>
> In order to cope with this, it is assumed that :
>  - for legacy platform_data machine, as the ordering cannot be ensured,
>the phy must call usb_gadget_vbus_(dis)connect, such as
>phy-gpio-vbus-usb.c
>  - for new devicetree platforms, we'll rely on the probe deferral, and
>the phy can be gadget API agnostic.
>
> Signed-off-by: Robert Jarzmik 
> ---
>  drivers/usb/gadget/udc/pxa27x_udc.c | 45 
> -
>  1 file changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c 
> b/drivers/usb/gadget/udc/pxa27x_udc.c
> index 001a3b74a993..f79a74304d4d 100644
> --- a/drivers/usb/gadget/udc/pxa27x_udc.c
> +++ b/drivers/usb/gadget/udc/pxa27x_udc.c
> @@ -33,6 +33,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "pxa27x_udc.h"
>  
> @@ -1655,6 +1656,37 @@ static int pxa_udc_vbus_draw(struct usb_gadget 
> *_gadget, unsigned mA)
>   return -EOPNOTSUPP;
>  }
>  
> +/**
> + * pxa_udc_phy_event - Called by phy upon VBus event
> + * @nb: notifier block
> + * @action: phy action, is vbus connect or disconnect
> + * @data: the usb_gadget structure in pxa_udc
> + *
> + * Called by the USB Phy when a cable connect or disconnect is sensed.
> + *
> + * Returns 0
> + */
> +static int pxa_udc_phy_event(struct notifier_block *nb, unsigned long action,
> +  void *data)
> +{
> + struct usb_gadget *gadget = data;
> +

add a printk() here printing the action. Then make sure this is being
called at the right time.

-- 
balbi


signature.asc
Description: PGP signature

VBUS GPIO for drivers/phy (was: Re: [RFC] usb: phy: generic: get rid of VBUS handling)

2016-07-29 Thread Felipe Balbi 

Hi,

(next time, please create a new thread :-)

Zhangfei Gao  writes:
> Hi, Balbi & Robert
>
> Have one question about commit 7acc9973e3c4 ("usb: phy: generic: add
> vbus support").
> Sorry asking here.
>
> Commit 7acc9973e3c4 ("usb: phy: generic: add vbus support") is adding
>  GPIO-based VBUS handling for phy-generic.c
>
> And now we are uploading usb phy to drivers/phy/, as Balbi mentioned
> drivers/usb/phy/ will not accept new phy driver.
>
> Any suggestion about handling vbus (gpio) for the phy under drivers/phy/

yeah, just define a fixed 5V regulator with a gpio enable. Then just
call regulator_enable/disable() at the right time.

-- 
balbi


signature.asc
Description: PGP signature

Re: xhci_hcd crash on linux 4.7.0

2016-07-29 Thread Alex Damian 
Hi Greg,

I managed to reproduce with a untainted kernel, see dmesg paste below.
The stack seemed corrupted as well ?

I refered to it as a crash since after a couple of these issues, the
machine hard freezes - I set up a serial console via a USB cable, but
I don't get the kernel oops out of the machine. The network is also
dead before getting any data. I could not think of any other way to
get a console out of a Macbook - any ideas ?

There is a progressive level of deterioration going on below, this is
why I'm adding multiple pastes. See the obviously invalid pointer
0001 in 3rd paste below. Also, see the protection fault in
the last paste. To me, something is trampling all over memory, and it
is usb-related.

Any suggestions are welcome,
Alex

[ 6462.324835] [ cut here ]
[ 6462.324844] WARNING: CPU: 2 PID: 6296 at
/home/kernel/COD/linux/lib/list_debug.c:59
handle_cmd_completion+0x4db/0xc60 [xhci_hcd]
[ 6462.324846] list_del corruption. prev->next should be
880260d5b780, but was 8801f6377000
[ 6462.324847] Modules linked in: tun msr cpufreq_stats mcs7830 usbnet
mii uas usb_storage fuse zram zsmalloc rfcomm lz4_compress bnep joydev
intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp nls_utf8
nls_cp437 vfat btusb btrtl btbcm btintel bluetooth kvm_intel kvm fat
applesmc input_polldev bcm5974 snd_hda_codec_hdmi irqbypass
crct10dif_pclmul crc32_pclmul ghash_clmulni_intel hmac drbg ansi_cprng
snd_hda_codec_cirrus snd_hda_codec_generic efi_pstore snd_hda_intel
snd_hda_codec snd_hda_core snd_hwdep snd_pcm aesni_intel snd_seq_midi
aes_x86_64 snd_seq_midi_event lrw gf128mul glue_helper ablk_helper
cryptd intel_cstate snd_rawmidi intel_rapl_perf brcmfmac snd_seq
brcmutil snd_seq_device snd_timer cfg80211 lpc_ich sg
intel_pch_thermal mfd_core efivars mmc_core snd thunderbolt mei_me
rfkill mei
[ 6462.324876]  soundcore shpchp sbs acpi_als kfifo_buf sbshc
industrialio evdev ac battery apple_bl tpm_tis tpm binfmt_misc nfsd
auth_rpcgss nfs_acl lockd grace parport_pc ppdev lp parport sunrpc
efivarfs autofs4 ext4 crc16 jbd2 mbcache sd_mod hid_apple hid_generic
usbhid hid crc32c_intel ahci libahci libata i915 scsi_mod xhci_pci
xhci_hcd usbcore usb_common i2c_algo_bit drm_kms_helper drm fjes video
button
[ 6462.324895] CPU: 2 PID: 6296 Comm: chrome Not tainted
4.7.0-040700-generic #201607241632
[ 6462.324896] Hardware name: Apple Inc.
MacBookPro12,1/Mac-E43C1C25D4880AD6, BIOS
MBP121.88Z.0167.B17.1606231721 06/23/2016
[ 6462.324898]  0086 a491e776 81321405
88026ec83d90
[ 6462.324899]   81078a3e 880260d5b790
88026ec83de8
[ 6462.324901]  880260238258  880260d5b780
8802603dcb40
[ 6462.324902] Call Trace:
[ 6462.324903][] ? dump_stack+0x5c/0x77
[ 6462.324909]  [] ? __warn+0xbe/0xe0
[ 6462.324910]  [] ? warn_slowpath_fmt+0x5f/0x80
[ 6462.324914]  [] ?
handle_cmd_completion+0x4db/0xc60 [xhci_hcd]
[ 6462.324918]  [] ? xhci_irq+0x326/0xb00 [xhci_hcd]
[ 6462.324920]  [] ? try_to_wake_up+0x53/0x390
[ 6462.324922]  [] ? handle_irq_event_percpu+0x78/0x1b0
[ 6462.324924]  [] ? handle_irq_event+0x39/0x60
[ 6462.324926]  [] ? handle_edge_irq+0x7b/0x140
[ 6462.324928]  [] ? handle_irq+0x19/0x30
[ 6462.324930]  [] ? do_IRQ+0x46/0xd0
[ 6462.324931]  [] ? common_interrupt+0x82/0x82
[ 6462.324932][] ? cap_settime+0x20/0x20
[ 6462.324935]  [] ? security_capable+0x41/0x60
[ 6462.324937]  [] ? ns_capable+0x25/0x50
[ 6462.324955]  [] ?
i915_get_reset_stats_ioctl+0x83/0xe0 [i915]
[ 6462.324965]  [] ? drm_ioctl+0x131/0x4c0 [drm]
[ 6462.324979]  [] ? i915_reg_read_ioctl+0x100/0x100 [i915]
[ 6462.324981]  [] ? bpf_prog_select_runtime+0xc0/0xc0
[ 6462.324983]  [] ? seccomp_phase1+0xa5/0x2d0
[ 6462.324985]  [] ? do_vfs_ioctl+0x9d/0x5c0
[ 6462.324987]  [] ? syscall_trace_enter_phase1+0x11f/0x150
[ 6462.324989]  [] ? SyS_ioctl+0x74/0x80
[ 6462.324990]  [] ? do_syscall_64+0x59/0xb0
[ 6462.324992]  [] ? entry_SYSCALL64_slow_path+0x25/0x25
[ 6462.324993] ---[ end trace affc0ce74aea10c3 ]---

[ 6882.389120] WARNING: CPU: 2 PID: 12607 at
/home/kernel/COD/linux/lib/list_debug.c:59
handle_cmd_completion+0x4db/0xc60 [xhci_hcd]
[ 6882.389121] list_del corruption. prev->next should be
8800891a9500, but was 88008116b510
[ 6882.389122] Modules linked in: tun msr cpufreq_stats mcs7830 usbnet
mii uas usb_storage fuse zram zsmalloc rfcomm lz4_compress bnep joydev
intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp nls_utf8
nls_cp437 vfat btusb btrtl btbcm btintel bluetooth kvm_intel kvm fat
applesmc input_polldev bcm5974 snd_hda_codec_hdmi irqbypass
crct10dif_pclmul crc32_pclmul ghash_clmulni_intel hmac drbg ansi_cprng
snd_hda_codec_cirrus snd_hda_codec_generic efi_pstore snd_hda_intel
snd_hda_codec snd_hda_core snd_hwdep snd_pcm aesni_intel snd_seq_midi
aes_x86_64 snd_seq_midi_event lrw gf128mul glue_helper ablk_helper
cryptd intel_cstate snd_rawmidi intel_rapl_perf brcmfmac snd_seq
brcmutil snd_seq_device

Re: USB vulnerability

2016-07-29 Thread Felipe Balbi 

Hi,

Alan Stern  writes:
> On Thu, 28 Jul 2016, Alan Stern wrote:
>
>> Only bits 10..0 of the wMaxPacketSize field contain the maximum packet
>> size; bits 12..11 contain something else (valid only for high-speed
>> periodic endpoints) and bits 15..13 are reserved (see Table 9-13 in the
>> USB-2.0 spec).
>> 
>> Furthermore, the value in bits 10..0 is never supposed to be larger
>> than 1024 (or less depending on the speed and the endpoint type).  We
>> should check these things in config.c/usb_parse_endpoint().
>> 
>> I will whip up a patch for this shortly.
>
> And here it is.  Rosie, can you or your intern check that this fixes 
> the problem?

slightly unrelated, but...

acket %d, setting to %d\n",
> + cfgno, inum, asnum, d->bEndpointAddress, maxp, j);
> + maxp = j;
> + endpoint->desc.wMaxPacketSize = cpu_to_le16(i | maxp);
> + }
> +
>   /*
>* Some buggy high speed devices have bulk endpoints using
>* maxpacket sizes other than 512.  High speed HCDs may not
> @@ -297,9 +360,6 @@ static int usb_parse_endpoint(struct dev
>*/
>   if (to_usb_device(ddev)->speed == USB_SPEED_HIGH
>   && usb_endpoint_xfer_bulk(d)) {
> - unsigned maxp;
> -
> - maxp = usb_endpoint_maxp(>desc) & 0x07ff;

usb_endpoint_maxp() should probably be updated to return only maximum
packet size. Then we would need to introduce usb_endpoint_mult() or
something along those lines to take care of the other valid bits.

-- 
balbi


signature.asc
Description: PGP signature

Re: USB vulnerability

2016-07-29 Thread Felipe Balbi 

Hi,

roswest  writes:
> [ Unknown signature status ]
>
> Alan,
>
> Hi, I am an engineer at Cisco Systems, and this summer we tasked some
> interns with performing USB fuzzing. One of the interns, Jake Lamberson,
> was able to cause a kernel panic when emulating an HID keyboard because
> the OHCI driver fails to reserve bandwidth for the device.  Please see
> the attachment for details.
>
> Thank you,
> Rosie Hall
>
> Headline: Linux Kernel Panic Over USB with HID Keyboard wMaxPacketSize
> Platforms:Ubuntu
> Versions: Linux Kernel 4.4.0-22-generic

you need to try with a more recent (v4.7 or today's Linus' HEAD) or you
need to ask for support on ubuntu forums. 

-- 
balbi


signature.asc
Description: PGP signature

Re: [PACTH v1 0/2] usb: xhci: plat: Enable PM, async resume/suspend

2016-07-29 Thread Felipe Balbi 

Hi,

Robert Foss  writes:
> This series should be labelled v2 instead of v1.

right, and you should also list your changes since v1.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PACTH v1 1/2] usb: xhci: plat: Enable runtime PM

2016-07-29 Thread Felipe Balbi 

Hi,

robert.f...@collabora.com writes:
> From: Andrew Bresticker 
>
> Enable runtime PM for the xhci-plat device so that the parent device
> may implement runtime PM.
>
> Signed-off-by: Andrew Bresticker 
> Tested-by: Robert Foss 
> Signed-off-by: Robert Foss 
> ---
>  drivers/usb/host/xhci-plat.c | 19 +--
>  1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index ed56bf9..fbd45c2 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -246,6 +246,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
>   if (ret)
>   goto dealloc_usb2_hcd;
>  
> + pm_runtime_set_active(>dev);
> + pm_runtime_enable(>dev);
> +
>   return 0;
>  
>  
> @@ -274,6 +277,8 @@ static int xhci_plat_remove(struct platform_device *dev)
>   struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>   struct clk *clk = xhci->clk;
>  
> + pm_runtime_disable(>dev);
> +
>   usb_remove_hcd(xhci->shared_hcd);
>   usb_phy_shutdown(hcd->usb_phy);
>  
> @@ -292,7 +297,9 @@ static int xhci_plat_suspend(struct device *dev)
>  {
>   struct usb_hcd  *hcd = dev_get_drvdata(dev);
>   struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> + int ret;
>  
> + pm_runtime_get_sync(dev);

you need to check for possible errors here. Ditto to the other occurence
below.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH V2] leds: trigger: Introduce an USB port trigger

2016-07-29 Thread Rafał Miłecki 
HI Rob,

I got problems following your objections, so it took me some time to
go back to this.

On 21 July 2016 at 22:42, Rob Herring  wrote:
> On Wed, Jul 20, 2016 at 10:06:23AM +0200, Rafał Miłecki wrote:
>> On 20 July 2016 at 03:02, Rob Herring  wrote:
>> > On Fri, Jul 15, 2016 at 11:10:45PM +0200, Rafał Miłecki wrote:
>> >> This commit adds a new trigger that can turn on LED when USB device gets
>> >> connected to the USB port. This can be useful for various home routers
>> >> that have USB port and a proper LED telling user a device is connected.
>> >>
>> >> Right now this trigger is usable with a proper DT only, there isn't a
>> >> way to specify USB ports from user space. This may change in a future.
>> >>
>> >> Signed-off-by: Rafał Miłecki 
>> >> ---
>> >> V2: The first version got support for specifying list of USB ports from
>> >> user space only. There was a (big try &) discussion on adding DT
>> >> support. It led to a pretty simple solution of comparing of_node of
>> >> usb_device to of_node specified in usb-ports property.
>> >> Since it appeared DT support may be simpler and non-DT a bit more
>> >> complex, this version drops previous support for "ports" and
>> >> "new_port" and focuses on DT only. The plan is to see if this
>> >> solution with DT is OK, get it accepted and then work on non-DT.
>> >>
>> >> Felipe: if there won't be any objections I'd like to ask for your Ack.
>> >> ---
>> >>  Documentation/devicetree/bindings/leds/common.txt |  11 ++
>> >>  Documentation/leds/ledtrig-usbport.txt|  19 ++
>> >>  drivers/leds/trigger/Kconfig  |   8 +
>> >>  drivers/leds/trigger/Makefile |   1 +
>> >>  drivers/leds/trigger/ledtrig-usbport.c| 206 
>> >> ++
>> >>  5 files changed, 245 insertions(+)
>> >>  create mode 100644 Documentation/leds/ledtrig-usbport.txt
>> >>  create mode 100644 drivers/leds/trigger/ledtrig-usbport.c
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/leds/common.txt 
>> >> b/Documentation/devicetree/bindings/leds/common.txt
>> >> index af10678..75536f7 100644
>> >> --- a/Documentation/devicetree/bindings/leds/common.txt
>> >> +++ b/Documentation/devicetree/bindings/leds/common.txt
>> >> @@ -50,6 +50,12 @@ property can be omitted.
>> >>  For controllers that have no configurable timeout the 
>> >> flash-max-timeout-us
>> >>  property can be omitted.
>> >>
>> >> +Trigger specific properties for child nodes:
>> >> +
>> >> +usbport trigger:
>> >> +- usb-ports : List of USB ports that usbport should observed for turning 
>> >> on a
>> >> +   given LED.
>> >
>> > I think this should be more generic such that it could work for disk or
>> > network LEDs too. Perhaps 'led-triggers = '? trigger is a bit of
>> > a Linux name, but I haven't thought of anything better. Really, I'd
>> > prefer the link in the other direction (e.g. port node have a 'leds" or
>> > '*-leds' property), but that's maybe harder to parse.
>>
>> I was already thinking on this as I plan to add "netdev" trigger at
>> some point in the future. I'd like to use "net-devices" for it (as a
>> equivalent or "usb-ports").
>>
>> However there is a problem with your idea of sharing "led-triggers"
>> between triggers.. Consider device with generic purpose LED that
>> should be controllable by a user. I believe we should let user switch
>> it's state, e.g. with:
>> echo usbport > trigger
>> echo netdev > trigger
>> with a shared property I don't see how we couldn't handle it properly.
>
> Well, the userspace interface and DT bindings are independent things.
> You can't argue for what the DT binding should look like based on the
> userspace interface.

I don't understand that. It sounds like you don't want user to have
control over a LED that was specified in DT. If I got it right, it
sounds like a bad idea to me. It's a known case (in marketing, usage
model) to allow user disable all LEDs (e.g. to don't disturb him
during the night). That sounds like a valid usage for me, so I want
user to be able to switch between triggers. And this is of course what
is already supported by triggers and user space interface. With
*current* triggers implementation you can do:
cd /sys/class/leds/foo/
echo none > trigger
echo timer > trigger

So I'm trying to have trigger specific entry in order to support more
than a single trigger.


> Perhaps we need a "device trigger" where you echo the device to
> be the trigger (similar to how bind works). If you have something to id
> the device, then you can lookup its struct device and then its of_node
> ptr.

Are you describing mixing user space interface with DT interface at
this point? Because echoing device sounds like doing some "echo foo >
bar" in user space. If so, I didn't design setting trigger details
from user space yet. I'm aware it'll require more discussion, so I
left it fo later.


> The other problem with