Using the syzkaller kernel fuzzer, Andrey Konovalov generated the
following error in gadgetfs:
> BUG: KASAN: use-after-free in __lock_acquire+0x3069/0x3690
> kernel/locking/lockdep.c:3246
> Read of size 8 at addr ffff88003a2bdaf8 by task kworker/3:1/903
>
> CPU: 3 PID: 903 Comm: kworker/3:1 Not tainted 4.12.0-rc4+ #35
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Workqueue: usb_hub_wq hub_event
> Call Trace:
> __dump_stack lib/dump_stack.c:16 [inline]
> dump_stack+0x292/0x395 lib/dump_stack.c:52
> print_address_description+0x78/0x280 mm/kasan/report.c:252
> kasan_report_error mm/kasan/report.c:351 [inline]
> kasan_report+0x230/0x340 mm/kasan/report.c:408
> __asan_report_load8_noabort+0x19/0x20 mm/kasan/report.c:429
> __lock_acquire+0x3069/0x3690 kernel/locking/lockdep.c:3246
> lock_acquire+0x22d/0x560 kernel/locking/lockdep.c:3855
> __raw_spin_lock include/linux/spinlock_api_smp.h:142 [inline]
> _raw_spin_lock+0x2f/0x40 kernel/locking/spinlock.c:151
> spin_lock include/linux/spinlock.h:299 [inline]
> gadgetfs_suspend+0x89/0x130 drivers/usb/gadget/legacy/inode.c:1682
> set_link_state+0x88e/0xae0 drivers/usb/gadget/udc/dummy_hcd.c:455
> dummy_hub_control+0xd7e/0x1fb0 drivers/usb/gadget/udc/dummy_hcd.c:2074
> rh_call_control drivers/usb/core/hcd.c:689 [inline]
> rh_urb_enqueue drivers/usb/core/hcd.c:846 [inline]
> usb_hcd_submit_urb+0x92f/0x20b0 drivers/usb/core/hcd.c:1650
> usb_submit_urb+0x8b2/0x12c0 drivers/usb/core/urb.c:542
> usb_start_wait_urb+0x148/0x5b0 drivers/usb/core/message.c:56
> usb_internal_control_msg drivers/usb/core/message.c:100 [inline]
> usb_control_msg+0x341/0x4d0 drivers/usb/core/message.c:151
> usb_clear_port_feature+0x74/0xa0 drivers/usb/core/hub.c:412
> hub_port_disable+0x123/0x510 drivers/usb/core/hub.c:4177
> hub_port_init+0x1ed/0x2940 drivers/usb/core/hub.c:4648
> hub_port_connect drivers/usb/core/hub.c:4826 [inline]
> hub_port_connect_change drivers/usb/core/hub.c:4999 [inline]
> port_event drivers/usb/core/hub.c:5105 [inline]
> hub_event+0x1ae1/0x3d40 drivers/usb/core/hub.c:5185
> process_one_work+0xc08/0x1bd0 kernel/workqueue.c:2097
> process_scheduled_works kernel/workqueue.c:2157 [inline]
> worker_thread+0xb2b/0x1860 kernel/workqueue.c:2233
> kthread+0x363/0x440 kernel/kthread.c:231
> ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:424
>
> Allocated by task 9958:
> save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
> save_stack+0x43/0xd0 mm/kasan/kasan.c:513
> set_track mm/kasan/kasan.c:525 [inline]
> kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:617
> kmem_cache_alloc_trace+0x87/0x280 mm/slub.c:2745
> kmalloc include/linux/slab.h:492 [inline]
> kzalloc include/linux/slab.h:665 [inline]
> dev_new drivers/usb/gadget/legacy/inode.c:170 [inline]
> gadgetfs_fill_super+0x24f/0x540 drivers/usb/gadget/legacy/inode.c:1993
> mount_single+0xf6/0x160 fs/super.c:1192
> gadgetfs_mount+0x31/0x40 drivers/usb/gadget/legacy/inode.c:2019
> mount_fs+0x9c/0x2d0 fs/super.c:1223
> vfs_kern_mount.part.25+0xcb/0x490 fs/namespace.c:976
> vfs_kern_mount fs/namespace.c:2509 [inline]
> do_new_mount fs/namespace.c:2512 [inline]
> do_mount+0x41b/0x2d90 fs/namespace.c:2834
> SYSC_mount fs/namespace.c:3050 [inline]
> SyS_mount+0xb0/0x120 fs/namespace.c:3027
> entry_SYSCALL_64_fastpath+0x1f/0xbe
>
> Freed by task 9960:
> save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
> save_stack+0x43/0xd0 mm/kasan/kasan.c:513
> set_track mm/kasan/kasan.c:525 [inline]
> kasan_slab_free+0x72/0xc0 mm/kasan/kasan.c:590
> slab_free_hook mm/slub.c:1357 [inline]
> slab_free_freelist_hook mm/slub.c:1379 [inline]
> slab_free mm/slub.c:2961 [inline]
> kfree+0xed/0x2b0 mm/slub.c:3882
> put_dev+0x124/0x160 drivers/usb/gadget/legacy/inode.c:163
> gadgetfs_kill_sb+0x33/0x60 drivers/usb/gadget/legacy/inode.c:2027
> deactivate_locked_super+0x8d/0xd0 fs/super.c:309
> deactivate_super+0x21e/0x310 fs/super.c:340
> cleanup_mnt+0xb7/0x150 fs/namespace.c:1112
> __cleanup_mnt+0x1b/0x20 fs/namespace.c:1119
> task_work_run+0x1a0/0x280 kernel/task_work.c:116
> exit_task_work include/linux/task_work.h:21 [inline]
> do_exit+0x18a8/0x2820 kernel/exit.c:878
> do_group_exit+0x14e/0x420 kernel/exit.c:982
> get_signal+0x784/0x1780 kernel/signal.c:2318
> do_signal+0xd7/0x2130 arch/x86/kernel/signal.c:808
> exit_to_usermode_loop+0x1ac/0x240 arch/x86/entry/common.c:157
> prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline]
> syscall_return_slowpath+0x3ba/0x410 arch/x86/entry/common.c:263
> entry_SYSCALL_64_fastpath+0xbc/0xbe
>
> The buggy address belongs to the object at ffff88003a2bdae0
> which belongs to the cache kmalloc-1024 of size 1024
> The buggy address is located 24 bytes inside of
> 1024-byte region [ffff88003a2bdae0, ffff88003a2bdee0)
> The buggy address belongs to the page:
> page:ffffea0000e8ae00 count:1 mapcount:0 mapping: (null)
> index:0x0 compound_mapcount: 0
> flags: 0x100000000008100(slab|head)
> raw: 0100000000008100 0000000000000000 0000000000000000 0000000100170017
> raw: ffffea0000ed3020 ffffea0000f5f820 ffff88003e80efc0 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff88003a2bd980: fb fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff88003a2bda00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> >ffff88003a2bda80: fc fc fc fc fc fc fc fc fc fc fc fc fb fb fb fb
> ^
> ffff88003a2bdb00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff88003a2bdb80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ==================================================================
What this means is that the gadgetfs_suspend() routine was trying to
access dev->lock after it had been deallocated. The root cause is a
race in the dummy_hcd driver; the dummy_udc_stop() routine can race
with the rest of the driver because it contains no locking. And even
when proper locking is added, it can still race with the
set_link_state() function because that function incorrectly drops the
private spinlock before invoking any gadget driver callbacks.
The result of this race, as seen above, is that set_link_state() can
invoke a callback in gadgetfs even after gadgetfs has been unbound
from dummy_hcd's UDC and its private data structures have been
deallocated.
include/linux/usb/gadget.h documents that the ->reset, ->disconnect,
->suspend, and ->resume callbacks may be invoked in interrupt context.
In general this is necessary, to prevent races with gadget driver
removal. This patch fixes dummy_hcd to retain the spinlock across
these calls, and it adds a spinlock acquisition to dummy_udc_stop() to
prevent the race.
The net2280 driver makes the same mistake of dropping the private
spinlock for its ->disconnect and ->reset callback invocations. The
patch fixes it too.
Lastly, since gadgetfs_suspend() may be invoked in interrupt context,
it cannot assume that interrupts are enabled when it runs. It must
use spin_lock_irqsave() instead of spin_lock_irq(). The patch fixes
that bug as well.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-and-tested-by: Andrey Konovalov <andreyknvl@google.com>
CC: <stable@vger.kernel.org>
Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The PN_INT_ENA register should be used after usb3_pn_change() is called.
So, this patch moves the access from renesas_usb3_stop_controller() to
usb3_disable_pipe_n().
Fixes: 746bfe63bb ("usb: gadget: renesas_usb3: add support for Renesas USB3.0 peripheral controller")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
This controller disallows to change the PIPE until reading/writing
a packet finishes. However. the previous code is not enough to hold
the lock in some functions. So, this patch fixes it.
Fixes: 746bfe63bb ("usb: gadget: renesas_usb3: add support for Renesas USB3.0 peripheral controller")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
This patch fixes an issue that this driver is possible to cause
deadlock by double-spinclocked in renesas_usb3_stop_controller().
So, this patch removes spinlock API calling in renesas_usb3_stop().
(In other words, the previous code had a redundant lock.)
Fixes: 746bfe63bb ("usb: gadget: renesas_usb3: add support for Renesas USB3.0 peripheral controller")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
This patch fixes an issue that this driver is possible to access
the registers before pm_runtime_get_sync() if a gadget driver is
installed first. After that, oops happens on R-Car Gen3 environment.
To avoid it, this patch changes the pm_runtime call timing from
probe/remove to udc_start/udc_stop.
Fixes: 746bfe63bb ("usb: gadget: renesas_usb3: add support for Renesas USB3.0 peripheral controller")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Check that req->buf is a valid DMA capable address, produce a warning
and return an error if it's either coming from vmalloc space or is an on
stack buffer.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Flag the first and only port as removable while also leaving the
remaining bits (including the reserved bit zero) unset in accordance
with the specifications:
"Within a byte, if no port exists for a given location, the bit
field representing the port characteristics shall be 0."
Also add a comment marking the legacy PortPwrCtrlMask field.
Fixes: 1cd8fd2887 ("usb: gadget: dummy_hcd: add SuperSpeed support")
Fixes: 1da177e4c3 ("Linux-2.6.12-rc2")
Cc: Tatyana Brokhman <tlinder@codeaurora.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
With 51 non-merge commits, this is one of the smallest USB Gadget pull
requests. Apart from your expected set of non-critical fixes, and
other miscellaneous items, we have most of the changes in dwc3 (52.5%)
with all other UDCs following with 34.8%.
As for the actual changes, the most important of them are all the
recent changes to reduce memory footprint of dwc3, bare minimum
dual-role support on dwc3 and reworked endpoint count and
initialization routines.
-----BEGIN PGP SIGNATURE-----
iQJRBAABCAA7FiEElLzh7wn96CXwjh2IzL64meEamQYFAljsjRwdHGZlbGlwZS5i
YWxiaUBsaW51eC5pbnRlbC5jb20ACgkQzL64meEamQaPZA/9H6GgNlvdGWPweJ0r
g86iBmio/Qn334SOCAJDrdw4ULSTSQR8nQHGWuETE2pQ2uYA/dbBWvjkx8Pujs+u
Ye2ig/iYchvHhY7cIWBG6A/iwvBClZF+yTv4ruh3i99BOGeAtGSyFS3HftQZLkGs
360CritNs+fSJsU6jnCE2+808evbkRiMaEfiBpiqXF2wvjMonWUMUlHaTBTHzDnk
k2/NJSzDINr4HaALalJxVpEetC500LDkQ531O5s7V6NIareCIBOh2li9xZJur38h
728BbCIiwPrdfLI2gGJLUOJq5vf5fJPT6DMJDN4hsu7gXcl2GiIc3nqlcxBB8P9R
kurWgjVd8aIGS7qqZzfDBH7I5UgFbJZbtVXy6UhSRILF+hzAb/l95pZUOyTwUnbj
ew3ALD2H7XolJT9jLG/q1CaQQMjudVnhLF2s1Ota8dsBeOShH6yfS6hGGQ8L1j8x
kXe3dl4HEj9s7gT0S+tVN+kt6bLGZ013CjCbuZgG9JLRkswyn780KTgL4pbjHiAE
KNGlCGUPRh3AiDLSbQ4IRU/J/0MVFs51k8aPn6PWpUqU8nc1QNEfczqrlAGPvkqU
nE77OktC1UCTrId7F8kllTEglOrR4CVXuT2N7rYzwgM2jmfHLzb0DaJ+a/ZdsUfI
DjuY8jNbuGnvZRRiVQU5ynva5BU=
=wYOI
-----END PGP SIGNATURE-----
Merge tag 'usb-for-v4.12' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-next
Felipe writes:
usb: changes for v4.12
With 51 non-merge commits, this is one of the smallest USB Gadget pull
requests. Apart from your expected set of non-critical fixes, and
other miscellaneous items, we have most of the changes in dwc3 (52.5%)
with all other UDCs following with 34.8%.
As for the actual changes, the most important of them are all the
recent changes to reduce memory footprint of dwc3, bare minimum
dual-role support on dwc3 and reworked endpoint count and
initialization routines.
Update Kconfig help for fifo_mode = 0 to explain the behavior better.
Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Use dev_err() to display EP configuration error to avoid silent failure.
Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
allow usb_del_gadget_udc() and usb add_gadget_udc() to be called
repeatedly on the same gadget->dev structure.
We need to clear the gadget->dev structure so that kobject_init()
doesn't complain about already initialized object.
Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
There are several inconsistencies in the error handling code.
1. If clk_get() fails, it goes to clk_put().
2. If pdata->phy_init() fails, it does not disable u3d->clk.
3. In case of failure after stopping u3d, it does pdata->phy_deinit()
and clk_disable(u3d->clk) twice.
4. It ignores failures in clk_enable().
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Check fifo configuration values against device tree values for endpoint fifo
in auto configuration mode (fifo_mode=0).
Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Currently the of glue code in fsl-mph-dr-of will create the platform
device fsl-usb2-udc. As this driver should also be probed by this name,
this patch adds it to the devtypes list.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
This patch splits the amd5536udc driver into two -- one that does
pci device registration and the other file that does the rest of
the driver tasks like the gadget/ep ops etc for Synopsys UDC.
This way of splitting helps in exporting core driver symbols which
can be used by any other platform/pci driver that is written for
the same Synopsys USB device controller.
The current patch also includes a change in the Kconfig and Makefile.
A new config option USB_SNP_CORE will be selected automatically when
any one of the platform or pci driver for the same UDC is selected.
Main changes:
- amd5536udc_pci.c: PCI device registration is moved to this file.
- amd5536udc.c:
This file does rest of the core UDC fucntionality.
9 symbols are exported so as to be used by amd5536udc_pci.c.
Module parameter definitions are moved to header file.
- amd5536udc.h:
Function declarations, module parameters definitions and few common
header file includes are added to this file
- Kconfig:
New USB_SNP_CORE option is added which will be auto selected when
any pci or platform driver config option for the UDC is chosen.
- Makefile:
Compiles the core and pci files separately.
Signed-off-by: Raviteja Garimella <raviteja.garimella@broadcom.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
This patch adds support for usb role swap via sysfs "role".
For example:
1) Connect a usb cable using 2 Salvator-X boards.
- For A-Device, the cable is connected to CN11 (USB3.0 ch0).
- For B-Device, the cable is connected to CN9 (USB2.0 ch0).
2) On A-Device, you input the following command:
# echo peripheral > /sys/devices/platform/soc/ee020000.usb/role
3) On B-Device, you input the following command:
# echo host > /sys/devices/platform/soc/ee080200.usb-phy/role
Then, the A-Device acts as a peripheral and the B-Device acts as
a host. Please note that A-Device must input the following command
if you want the board to act as a host again.
# echo host > /sys/devices/platform/soc/ee020000.usb/role
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Minor code cleanup based on feedback received on mailinglist.
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
This patch adds extcon support to see VBUS/ID signal states.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
This usb 3.0 peripheral controller has a register (USB_OTG_STA) to monitor
the USB ID signal. So, this patch adds the ID signal monitoring to change
the mode to host (A-Host) or peripheral (B-Peripheral).
This patch also removes hardcoded setting as B-Peripheral mode.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
This fixes the commit: 1cd8fd2887 ("usb: gadget: dummy_hcd: add
SuperSpeed support").
In the case of ClearPortFeature and USB_PORT_FEAT_POWER, simply clear
the right bit regardless of what the wValue is.
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Yuyang Du <yuyang.du@intel.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Remove unnecessary variable and update function prototype.
Acked-by: Michal Nazarewicz <mina86@mina86.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Rewrite udc_free_dma_chain() function to avoid use of pointer after free.
Addresses-Coverity-ID: 1091172
Acked-by: Michal Nazarewicz <mina86@mina86.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Functions udc_enable() and udc_disable() have a duplicated prototype.
Remove it.
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Signed-off-by: Petr Cvek <petr.cvek@tul.cz>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Rearrange statements in mv_ep_enable function so that it’s obvious
what the switch does and how zlt, ios and mult variables are
initialised. Most notably, this gets rid of an implicit fall-through
so people don’t have to wonder whether it was intenional or not.
Addresses-Coverity-ID: 201385
Reported-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Remove pointer dereference after free.
Addresses-Coverity-ID: 1091173
Acked-by: Michal Nazarewicz <mina86@mina86.com>
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
a lot of embeded system SOC (e.g. freescale T2080) have both
PCI and USB modules. But USB module is controlled by registers directly,
it have no relationship with PCI module.
when say N here it will not build PCI related code in USB driver.
Signed-off-by: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>
Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The PCI pool API is deprecated. This commit replaces the PCI pool old
API by the appropriate function with the DMA pool API.
Signed-off-by: Romain Perier <romain.perier@collabora.com>
Acked-by: Peter Senna Tschudin <peter.senna@collabora.com>
Tested-by: Peter Senna Tschudin <peter.senna@collabora.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The PCI pool API is deprecated. This commit replaces the PCI pool old
API by the appropriate function with the DMA pool API.
Signed-off-by: Romain Perier <romain.perier@collabora.com>
Reviewed-by: Peter Senna Tschudin <peter.senna@collabora.com>
Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The PCI pool API is deprecated. This commit replaces the PCI pool old
API by the appropriate function with the DMA pool API.
Signed-off-by: Romain Perier <romain.perier@collabora.com>
Acked-by: Peter Senna Tschudin <peter.senna@collabora.com>
Tested-by: Peter Senna Tschudin <peter.senna@collabora.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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 <balbi@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Signed-off-by: Raz Manor <Raz.Manor@valens.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
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 <robert.jarzmik@free.fr>
Reported-by: Robert Jarzmik <robert.jarzmik@free.fr>
Signed-off-by: Petr Cvek <petr.cvek@tul.cz>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
The debug output now contains the wrong variable, as seen from the compiler
warning:
drivers/usb/gadget/udc/atmel_usba_udc.c: In function 'usba_ep_enable':
drivers/usb/gadget/udc/atmel_usba_udc.c:632:550: error: 'ept_cfg' may be used uninitialized in this function [-Werror=maybe-uninitialized]
DBG(DBG_ERR, "%s: EPT_CFG = 0x%lx (maxpacket = %lu)\n",
This changes the debug output the same way as the other code.
Fixes: 741d2558bf ("usb: gadget: udc: atmel: Update endpoint allocation scheme")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Fix typos and add the following to the scripts/spelling.txt:
followings||following
While we are here, add a missing colon in the boilerplate in DT binding
documents. The "you SoC" in allwinner,sunxi-pinctrl.txt was fixed as
well.
I reworded "as the followings:" to "as follows:" for
drivers/usb/gadget/udc/renesas_usb3.c.
Link: http://lkml.kernel.org/r/1481573103-11329-32-git-send-email-yamada.masahiro@socionext.com
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Fix typos and add the following to the scripts/spelling.txt:
intialization||initialization
The "inintialization" in drivers/acpi/spcr.c is a different pattern but
I fixed it as well in this commit.
Link: http://lkml.kernel.org/r/1481573103-11329-16-git-send-email-yamada.masahiro@socionext.com
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Here's the big pull request for the Gadget
API. Again the majority of changes sit in dwc2
driver. Most important changes contain a workaround
for GOTGCTL being wrong, a sleep-inside-spinlock fix
and the big series of cleanups on dwc2.
One important thing on dwc3 is that we don't anymore
need gadget drivers to cope with unaligned OUT
transfers for us. We have support for appending one
extra chained TRB to align transfer ourselves.
Apart from these, the usual set of typos,
non-critical fixes, etc.
-----BEGIN PGP SIGNATURE-----
iQJRBAABCAA7FiEElLzh7wn96CXwjh2IzL64meEamQYFAliKBd0dHGZlbGlwZS5i
YWxiaUBsaW51eC5pbnRlbC5jb20ACgkQzL64meEamQb1Yw//XEI0VeUdsRM/JKK5
4BBoFCCUXT7jZMyzm7x9G7/4Tz3Xn9I9elAbZ7A3ctpLQXVO0KUAc5S3MRmq1ghj
8h18ZrOLqWG6gG+0eOtzSQLJ91+ULV+nyV6jS+RJyio7AMKy6XW0hRHQQs8ggcqg
A/hZKVI/GdvnoSSLVNoWFxTiVC7Pu62IsWJV6BJuWubEze7u97xfBIt3qB9h2R8q
ARfy29LYZulgrRYZ2DAtrKNFC8yKdIiKxWcmI47FHcNXtnQ6yAu7VqFcKWlH9Jvb
NY/YAzv30leOXHrwHVXCM/Urh7SK6FJIIdgoDMpCWoy1CK2pHXkJCndt/NuM721h
DYaopfNBJLtjvYDsvtnGXo3DC6+hBd3oYEz+gogGS/5gocr2ChnRK8Qc5T8eXYnC
OBIVKeHcsx9wMxYwwxk2axWjflfbSRjPnpZHibgkvdrwGVY+Hcwf5GyixvTdqQJM
dMrZcIPqtPJAg7OBPoH0LEViT14+lc8f5n6D0T/NezLxeeZvbcWQ5p17LpNal5oi
S+4kmtNWR7nXKSdICvc00Ap+Iqu91eB4c1MfvP6HSLq7wpjs5QhWtDvM7++ygjL1
7m3ExmXUx+j29M64WM9KTNlXZFkQ8XB5PzmcXueQkz9fd2u8JJhBqGHdVWFHOv9y
4YGdaSpeqsui9bFoIdbjn0EO3+g=
=gk8j
-----END PGP SIGNATURE-----
Merge tag 'usb-for-v4.11' of git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-next
Felipe writes:
USB: changes for v4.11
Here's the big pull request for the Gadget
API. Again the majority of changes sit in dwc2
driver. Most important changes contain a workaround
for GOTGCTL being wrong, a sleep-inside-spinlock fix
and the big series of cleanups on dwc2.
One important thing on dwc3 is that we don't anymore
need gadget drivers to cope with unaligned OUT
transfers for us. We have support for appending one
extra chained TRB to align transfer ourselves.
Apart from these, the usual set of typos,
non-critical fixes, etc.
Commit 304f7e5e1d ("usb: gadget: Refactor request completion")
removed check if req->req.complete is non-NULL, resulting in a NULL
pointer derefence and a kernel panic.
This patch adds an empty complete function instead of re-introducing
the req->req.complete check.
Fixes: 304f7e5e1d ("usb: gadget: Refactor request completion")
Signed-off-by: Magnus Lilja <lilja.magnus@gmail.com>
Cc: <stable@vger.kernel.org> # 3.18+
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
This patch updates the usb endpoint allocation scheme for atmel usba
driver to make sure all endpoints are allocated in order. This
requirement comes from the datasheet of the controller.
The allocation scheme is decided by fifo_mode parameter. For fifo_mode =
0 the driver tries to autoconfigure the endpoints fifo size. All other
modes contain fixed configurations optimized for different purposes. The
idea is somehow similar with the approach used on musb driver.
Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Declare usb_gadget_ops structures as const as they are only stored in
the ops field of a usb_gadget structure. This field is of type const, so
usb_gadget_ops structures having this property can be declared const
too.
Done using Coccinelle:
@r1 disable optional_qualifier@
identifier i;
position p;
@@
static struct usb_gadget_ops i@p={...};
@ok1@
identifier r1.i;
position p;
struct fotg210_udc fotg210;
@@
fotg210.gadget.ops=&i@p
@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p
@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct usb_gadget_ops i;
File size before:
text data bss dec hex filename
7559 384 8 7951 1f0f usb/gadget/udc/fotg210-udc.o
File size after:
text data bss dec hex filename
7655 288 8 7951 1f0f usb/gadget/udc/fotg210-udc.o
Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Since:
commit 855ed04a37 ("usb: gadget: udc-core: independent registration
of gadgets and gadget drivers")
if we load gadget module but there is no free udc available
then it will be stored on a pending gadgets list.
$ modprobe g_zero.ko
$ modprobe g_ether.ko
[] udc-core: couldn't find an available UDC - added [g_ether] to list
of pending drivers
We scan this list each time when new UDC appears in system.
But we can get a free UDC each time after gadget unbind.
This commit add scanning of that list directly after unbinding
gadget from udc.
Thanks to this, when we unload first gadget:
$ rmmod g_zero.ko
gadget which is pending is automatically
attached to that UDC (if name matches).
Fixes: 855ed04a37 ("usb: gadget: udc-core: independent registration of gadgets and gadget drivers")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Krzysztof Opasiak <k.opasiak@samsung.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Declare usb_gadget_ops structures as const as they are only stored in
the ops field of a usb_gadget structure. This field is of type const, so
usb_gadget_ops structures having this property can be declared const
too.
Done using Coccinelle:
@r1 disable optional_qualifier@
identifier i;
position p;
@@
static struct usb_gadget_ops i@p={...};
@ok1@
identifier r1.i;
position p;
struct fotg210_udc fotg210;
@@
fotg210.gadget.ops=&i@p
@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p
@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct usb_gadget_ops i;
File size before:
text data bss dec hex filename
7559 384 8 7951 1f0f usb/gadget/udc/fotg210-udc.o
File size after:
text data bss dec hex filename
7655 288 8 7951 1f0f usb/gadget/udc/fotg210-udc.o
Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Commit bbe097f092 ("usb: gadget: udc: atmel: fix endpoint name")
introduced a memory leak when unbinding the driver. The endpoint names
would not be freed. Solve that by including the name as a string in struct
usba_ep so it is freed when the endpoint is.
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
The stop_activity() routine in dummy-hcd is supposed to unlink all
active requests for every endpoint, among other things. But it
doesn't handle ep0. As a result, fuzz testing can generate a WARNING
like the following:
WARNING: CPU: 0 PID: 4410 at drivers/usb/gadget/udc/dummy_hcd.c:672 dummy_free_request+0x153/0x170
Modules linked in:
CPU: 0 PID: 4410 Comm: syz-executor Not tainted 4.9.0-rc7+ #32
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
ffff88006a64ed10 ffffffff81f96b8a ffffffff41b58ab3 1ffff1000d4c9d35
ffffed000d4c9d2d ffff880065f8ac00 0000000041b58ab3 ffffffff8598b510
ffffffff81f968f8 0000000041b58ab3 ffffffff859410e0 ffffffff813f0590
Call Trace:
[< inline >] __dump_stack lib/dump_stack.c:15
[<ffffffff81f96b8a>] dump_stack+0x292/0x398 lib/dump_stack.c:51
[<ffffffff812b808f>] __warn+0x19f/0x1e0 kernel/panic.c:550
[<ffffffff812b831c>] warn_slowpath_null+0x2c/0x40 kernel/panic.c:585
[<ffffffff830fcb13>] dummy_free_request+0x153/0x170 drivers/usb/gadget/udc/dummy_hcd.c:672
[<ffffffff830ed1b0>] usb_ep_free_request+0xc0/0x420 drivers/usb/gadget/udc/core.c:195
[<ffffffff83225031>] gadgetfs_unbind+0x131/0x190 drivers/usb/gadget/legacy/inode.c:1612
[<ffffffff830ebd8f>] usb_gadget_remove_driver+0x10f/0x2b0 drivers/usb/gadget/udc/core.c:1228
[<ffffffff830ec084>] usb_gadget_unregister_driver+0x154/0x240 drivers/usb/gadget/udc/core.c:1357
This patch fixes the problem by iterating over all the endpoints in
the driver's ep array instead of iterating over the gadget's ep_list,
which explicitly leaves out ep0.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: Andrey Konovalov <andreyknvl@google.com>
CC: <stable@vger.kernel.org>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
The patch improves readability of mv_u3d_start_queue()
by rearranging its code with two semantic modifications:
- assignment zero to ep->processing if usb_gadget_map_request() fails;
- propagation of error code from mv_u3d_req_to_trb() instead of
hardcoded -ENOMEM.
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>