linux-stable.git/drivers/usb, branch v4.9.69

usb: gadget: udc: net2280: Fix tmp reusage in net2280 driver

2017-12-14T08:28:18+00:00

[ Upstream commit ef5e2fa9f65befa12f1113c734602d2c1964d2a5 ]

In the function scan_dma_completions() there is a reusage of tmp
variable. That coused a wrong value being used in some case when
reading a short packet terminated transaction from an endpoint,
in 2 concecutive reads.

This was my logic for the patch:

The req->td->dmadesc equals to 0 iff:
-- There was a transaction ending with a short packet, and
-- The read() to read it was shorter than the transaction length, and
-- The read() to complete it is longer than the residue.
I believe this is true from the printouts of various cases,
but I can't be positive it is correct.

Entering this if, there should be no more data in the endpoint
(a short packet terminated the transaction).
If there is, the transaction wasn't really done and we should exit and
wait for it to finish entirely. That is the inner if.
That inner if should never happen, but it is there to be on the safe
side. That is why it is marked with the comment /* paranoia */.
The size of the data available in the endpoint is ep->dma->dmacount
and it is read to tmp.
This entire clause is based on my own educated guesses.

If we passed that inner if without breaking in the original code,
than tmp & DMA_BYTE_MASK_COUNT== 0.
That means we will always pass dma bytes count of 0 to dma_done(),
meaning all the requested bytes were read.

dma_done() reports back to the upper layer that the request (read())
was done and how many bytes were read.
In the original code that would always be the request size,
regardless of the actual size of the data.
That did not make sense to me at all.

However, the original value of tmp is req->td->dmacount,
which is the dmacount value when the request's dma transaction was
finished. And that is a much more reasonable value to report back to
the caller.

To recreate the problem:
Read from a bulk out endpoint in a loop, 1024 * n bytes in each
iteration.
Connect the PLX to a host you can control.
Send to that endpoint 1024 * n + x bytes,
such that 0 < x < 1024 * n and (x % 1024) != 0
You would expect the first read() to return 1024 * n
and the second read() to return x.
But you will get the first read to return 1024 * n
and the second one to return 1024 * n.
That is true for every positive integer n.

Cc: Felipe Balbi 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Signed-off-by: Raz Manor 
Signed-off-by: Felipe Balbi 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman

usb: gadget: pxa27x: Test for a valid argument pointer

2017-12-14T08:28:18+00:00

[ Upstream commit df7545719a14fa7b481896fb8689e23d0a00f682 ]

A call usb_put_phy(udc->transceiver) must be tested for a valid pointer.
Use an already existing test for usb_unregister_notifier call.

Acked-by: Robert Jarzmik 
Reported-by: Robert Jarzmik 
Signed-off-by: Petr Cvek 
Signed-off-by: Felipe Balbi 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman

usb: dwc3: gadget: Fix system suspend/resume on TI platforms

2017-12-14T08:28:18+00:00

[ Upstream commit 1551e35ea4189c1f7199fe278395fc94196715f2 ]

On TI platforms (dra7, am437x), the DWC3_DSTS_DEVCTRLHLT bit is not set
after the device controller is stopped via DWC3_DCTL_RUN_STOP.

If we don't disconnect and stop the gadget, it stops working after a
system resume with the trace below.

There is no point in preventing gadget disconnect and gadget stop during
system suspend/resume as we're going to suspend in any case, whether
DEVCTRLHLT timed out or not.

[  141.727480] ------------[ cut here ]------------
[  141.732349] WARNING: CPU: 1 PID: 2135 at drivers/usb/dwc3/gadget.c:2384 dwc3_stop_active_transfer.constprop.4+0xc4/0xe4 [dwc3]
[  141.744299] Modules linked in: usb_f_ss_lb g_zero libcomposite xhci_plat_hcd xhci_hcd usbcore dwc3 evdev udc_core m25p80 usb_common spi_nor snd_soc_davinci_mcasp snd_soc_simple_card snd_soc_edma snd_soc_tlv3e
[  141.792163] CPU: 1 PID: 2135 Comm: irq/456-dwc3 Not tainted 4.10.0-rc8 #1138
[  141.799547] Hardware name: Generic DRA74X (Flattened Device Tree)
[  141.805940] [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[  141.814066] [] (show_stack) from [] (dump_stack+0xac/0xe0)
[  141.821648] [] (dump_stack) from [] (__warn+0xd8/0x104)
[  141.828955] [] (__warn) from [] (warn_slowpath_null+0x20/0x28)
[  141.836902] [] (warn_slowpath_null) from [] (dwc3_stop_active_transfer.constprop.4+0xc4/0xe4 [dwc3])
[  141.848329] [] (dwc3_stop_active_transfer.constprop.4 [dwc3]) from [] (__dwc3_gadget_ep_disable+0x64/0x528 [dwc3])
[  141.861034] [] (__dwc3_gadget_ep_disable [dwc3]) from [] (dwc3_gadget_ep_disable+0x3c/0xc8 [dwc3])
[  141.872280] [] (dwc3_gadget_ep_disable [dwc3]) from [] (usb_ep_disable+0x11c/0x18c [udc_core])
[  141.883160] [] (usb_ep_disable [udc_core]) from [] (disable_ep+0x18/0x54 [usb_f_ss_lb])
[  141.893408] [] (disable_ep [usb_f_ss_lb]) from [] (disable_endpoints+0x18/0x50 [usb_f_ss_lb])
[  141.904168] [] (disable_endpoints [usb_f_ss_lb]) from [] (disable_source_sink+0x2c/0x34 [usb_f_ss_lb])
[  141.915771] [] (disable_source_sink [usb_f_ss_lb]) from [] (reset_config+0x48/0x7c [libcomposite])
[  141.927012] [] (reset_config [libcomposite]) from [] (composite_disconnect+0x2c/0x54 [libcomposite])
[  141.938444] [] (composite_disconnect [libcomposite]) from [] (usb_gadget_udc_reset+0x10/0x34 [udc_core])
[  141.950237] [] (usb_gadget_udc_reset [udc_core]) from [] (dwc3_gadget_reset_interrupt+0x64/0x698 [dwc3])
[  141.962022] [] (dwc3_gadget_reset_interrupt [dwc3]) from [] (dwc3_thread_interrupt+0x618/0x1a3c [dwc3])
[  141.973723] [] (dwc3_thread_interrupt [dwc3]) from [] (irq_thread_fn+0x1c/0x54)
[  141.983215] [] (irq_thread_fn) from [] (irq_thread+0x120/0x1f0)
[  141.991247] [] (irq_thread) from [] (kthread+0xf8/0x138)
[  141.998641] [] (kthread) from [] (ret_from_fork+0x14/0x24)
[  142.006213] ---[ end trace b4ecfe9f175b9a9c ]---

Signed-off-by: Roger Quadros 
Signed-off-by: Felipe Balbi 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman

USB: gadgetfs: Fix a potential memory leak in 'dev_config()'

2017-12-14T08:28:18+00:00

[ Upstream commit b6e7aeeaf235901c42ec35de4633c7c69501d303 ]

'kbuf' is allocated just a few lines above using 'memdup_user()'.
If the 'if (dev->buf)' test fails, this memory is never released.

Signed-off-by: Christophe JAILLET 
Signed-off-by: Felipe Balbi 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman

usb: gadget: configs: plug memory leak

2017-12-14T08:28:18+00:00

[ Upstream commit 38355b2a44776c25b0f2ad466e8c51bb805b3032 ]

When binding a gadget to a device, "name" is stored in gi->udc_name, but
this does not happen when unregistering and the string is leaked.

Signed-off-by: John Keeping 
Signed-off-by: Felipe Balbi 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman

usb: f_fs: Force Reserved1=1 in OS_DESC_EXT_COMPAT

2017-12-14T08:28:15+00:00

commit a3acc696085e112733d191a77b106e67a4fa110b upstream.

The specification says that the Reserved1 field in OS_DESC_EXT_COMPAT
must have the value "1", but when this feature was first implemented we
rejected any non-zero values.

This was adjusted to accept all non-zero values (while now rejecting
zero) in commit 53642399aa71 ("usb: gadget: f_fs: Fix wrong check on
reserved1 of OS_DESC_EXT_COMPAT"), but that breaks any userspace
programs that worked previously by returning EINVAL when Reserved1 == 0
which was previously the only value that succeeded!

If we just set the field to "1" ourselves, both old and new userspace
programs continue to work correctly and, as a bonus, old programs are
now compliant with the specification without having to fix anything
themselves.

Fixes: 53642399aa71 ("usb: gadget: f_fs: Fix wrong check on reserved1 of OS_DESC_EXT_COMPAT")
Signed-off-by: John Keeping 
Signed-off-by: Felipe Balbi 
Signed-off-by: Greg Kroah-Hartman

usb: gadget: udc: renesas_usb3: fix number of the pipes

2017-12-14T08:28:09+00:00

commit a58204ab91ad8cae4d8474aa0ba5d1fc504860c9 upstream.

This controller on R-Car Gen3 has 6 pipes that included PIPE 0 for
control actually. But, the datasheet has error in writing as it has
31 pipes. (However, the previous code defined 30 pipes wrongly...)

Anyway, this patch fixes it.

Fixes: 746bfe63bba3 ("usb: gadget: renesas_usb3: add support for Renesas USB3.0 peripheral controller")
Signed-off-by: Yoshihiro Shimoda 
Signed-off-by: Felipe Balbi 
Signed-off-by: Greg Kroah-Hartman

usb: host: fix incorrect updating of offset

2017-12-09T21:01:57+00:00

commit 1d5a31582ef046d3b233f0da1a68ae26519b2f0a upstream.

The variable temp is incorrectly being updated, instead it should
be offset otherwise the loop just reads the same capability value
and loops forever.  Thanks to Alan Stern for pointing out the
correct fix to my original fix.  Fix also cleans up clang warning:

drivers/usb/host/ehci-dbg.c:840:4: warning: Value stored to 'temp'
is never read

Fixes: d49d43174400 ("USB: misc ehci updates")
Signed-off-by: Colin Ian King 
Acked-by: Alan Stern 
Signed-off-by: Greg Kroah-Hartman

USB: usbfs: Filter flags passed in from user space

2017-12-09T21:01:57+00:00

commit 446f666da9f019ce2ffd03800995487e79a91462 upstream.

USBDEVFS_URB_ISO_ASAP must be accepted only for ISO endpoints.
Improve sanity checking.

Reported-by: Andrey Konovalov 
Signed-off-by: Oliver Neukum 
Acked-by: Alan Stern 
Signed-off-by: Greg Kroah-Hartman

USB: devio: Prevent integer overflow in proc_do_submiturb()

2017-12-09T21:01:57+00:00

commit 57999d1107c1e60c2ca7088f2ac0f819e2f554b3 upstream.

There used to be an integer overflow check in proc_do_submiturb() but
we removed it.  It turns out that it's still required.  The
uurb->buffer_length variable is a signed integer and it's controlled by
the user.  It can lead to an integer overflow when we do:

	num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE);

If we strip away the macro then that line looks like this:

	num_sgs = (uurb->buffer_length + USB_SG_SIZE - 1) / USB_SG_SIZE;
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
It's the first addition which can overflow.

Fixes: 1129d270cbfb ("USB: Increase usbfs transfer limit")
Signed-off-by: Dan Carpenter 
Acked-by: Alan Stern 
Signed-off-by: Greg Kroah-Hartman