linux.git/drivers/usb/core/hcd.c, branch v5.17

USB: core: Fix hang in usb_kill_urb by adding memory barriers

2022-01-25T17:43:19+00:00

The syzbot fuzzer has identified a bug in which processes hang waiting
for usb_kill_urb() to return.  It turns out the issue is not unlinking
the URB; that works just fine.  Rather, the problem arises when the
wakeup notification that the URB has completed is not received.

The reason is memory-access ordering on SMP systems.  In outline form,
usb_kill_urb() and __usb_hcd_giveback_urb() operating concurrently on
different CPUs perform the following actions:

CPU 0					CPU 1
----------------------------		---------------------------------
usb_kill_urb():				__usb_hcd_giveback_urb():
  ...					  ...
  atomic_inc(&urb->reject);		  atomic_dec(&urb->use_count);
  ...					  ...
  wait_event(usb_kill_urb_queue,
	atomic_read(&urb->use_count) == 0);
					  if (atomic_read(&urb->reject))
						wake_up(&usb_kill_urb_queue);

Confining your attention to urb->reject and urb->use_count, you can
see that the overall pattern of accesses on CPU 0 is:

	write urb->reject, then read urb->use_count;

whereas the overall pattern of accesses on CPU 1 is:

	write urb->use_count, then read urb->reject.

This pattern is referred to in memory-model circles as SB (for "Store
Buffering"), and it is well known that without suitable enforcement of
the desired order of accesses -- in the form of memory barriers -- it
is entirely possible for one or both CPUs to execute their reads ahead
of their writes.  The end result will be that sometimes CPU 0 sees the
old un-decremented value of urb->use_count while CPU 1 sees the old
un-incremented value of urb->reject.  Consequently CPU 0 ends up on
the wait queue and never gets woken up, leading to the observed hang
in usb_kill_urb().

The same pattern of accesses occurs in usb_poison_urb() and the
failure pathway of usb_hcd_submit_urb().

The problem is fixed by adding suitable memory barriers.  To provide
proper memory-access ordering in the SB pattern, a full barrier is
required on both CPUs.  The atomic_inc() and atomic_dec() accesses
themselves don't provide any memory ordering, but since they are
present, we can use the optimized smp_mb__after_atomic() memory
barrier in the various routines to obtain the desired effect.

This patch adds the necessary memory barriers.

CC: 
Reported-and-tested-by: syzbot+76629376e06e2c2ad626@syzkaller.appspotmail.com
Signed-off-by: Alan Stern 
Link: https://lore.kernel.org/r/Ye8K0QYee0Q0Nna2@rowland.harvard.edu
Signed-off-by: Greg Kroah-Hartman

USB: Fix "slab-out-of-bounds Write" bug in usb_hcd_poll_rh_status

2022-01-03T13:38:34+00:00

When the USB core code for getting root-hub status reports was
originally written, it was assumed that the hub driver would be its
only caller.  But this isn't true now; user programs can use usbfs to
communicate with root hubs and get status reports.  When they do this,
they may use a transfer_buffer that is smaller than the data returned
by the HCD, which will lead to a buffer overflow error when
usb_hcd_poll_rh_status() tries to store the status data.  This was
discovered by syzbot:

BUG: KASAN: slab-out-of-bounds in memcpy include/linux/fortify-string.h:225 [inline]
BUG: KASAN: slab-out-of-bounds in usb_hcd_poll_rh_status+0x5f4/0x780 drivers/usb/core/hcd.c:776
Write of size 2 at addr ffff88801da403c0 by task syz-executor133/4062

This patch fixes the bug by reducing the amount of status data if it
won't fit in the transfer_buffer.  If some data gets discarded then
the URB's completion status is set to -EOVERFLOW rather than 0, to let
the user know what happened.

Reported-and-tested-by: syzbot+3ae6a2b06f131ab9849f@syzkaller.appspotmail.com
Signed-off-by: Alan Stern 
Cc: 
Link: https://lore.kernel.org/r/Yc+3UIQJ2STbxNua@rowland.harvard.edu
Signed-off-by: Greg Kroah-Hartman

usb: core: hcd: change sizeof(vaddr) to sizeof(unsigned long)

2021-12-13T14:03:18+00:00

`vaddr` is a pointer to unsigned char. sizeof(vaddr) here intends
to get the size of a pointer. But readers may get confused. Change
sizeof(vaddr) to sizeof(unsigned long) makes more sense.

Suggested-by: Alan Stern 
Signed-off-by: Guo Zhengkui 
Link: https://lore.kernel.org/r/20211209062441.9856-1-guozhengkui@vivo.com
Signed-off-by: Greg Kroah-Hartman

Revert "usb: core: hcd: Add support for deferring roothub registration"

2021-11-05T07:41:02+00:00

This reverts commit 58877b0824da15698bd85a0a9dbfa8c354e6ecb7.

It has been reported to be causing problems in Arch and Fedora bug
reports.

Reported-by: Hans de Goede 
Link: https://bbs.archlinux.org/viewtopic.php?pid=2000956#p2000956
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2019542
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2019576
Link: https://lore.kernel.org/r/42bcbea6-5eb8-16c7-336a-2cb72e71bc36@redhat.com
Cc: Mathias Nyman 
Cc: Chris Chiu 
Cc: Alan Stern 
Cc: Kishon Vijay Abraham I 
Signed-off-by: Greg Kroah-Hartman

Merge 5.15-rc3 into usb-next

2021-09-27T14:34:40+00:00

We need the USB fixes in here as well.

Signed-off-by: Greg Kroah-Hartman

usb: core: hcd: fix messages in usb_hcd_request_irqs()

2021-09-21T14:34:14+00:00

Two dev_info() calls in usb_hcd_request_irqs() mistreat the I/O port base
address, calling it just "io base" instead of "io port".

While fixing this, make indenataion of the argument lists more sane...

Signed-off-by: Sergey Shtylyov 
Link: https://lore.kernel.org/r/5d10014d-e58b-d081-ed7c-7424f649ce0b@omp.ru
Signed-off-by: Greg Kroah-Hartman

usb: core: hcd: Modularize HCD stop configuration in usb_stop_hcd()

2021-09-14T08:41:30+00:00

No functional change. Since configuration to stop HCD is invoked from
multiple places, group all of them in usb_stop_hcd().

Tested-by: Chris Chiu 
Acked-by: Alan Stern 
Signed-off-by: Kishon Vijay Abraham I 
Link: https://lore.kernel.org/r/20210909064200.16216-4-kishon@ti.com
Signed-off-by: Greg Kroah-Hartman

usb: core: hcd: Add support for deferring roothub registration

2021-09-14T08:41:30+00:00

It has been observed with certain PCIe USB cards (like Inateck connected
to AM64 EVM or J7200 EVM) that as soon as the primary roothub is
registered, port status change is handled even before xHC is running
leading to cold plug USB devices not detected. For such cases, registering
both the root hubs along with the second HCD is required. Add support for
deferring roothub registration in usb_add_hcd(), so that both primary and
secondary roothubs are registered along with the second HCD.

CC: stable@vger.kernel.org # 5.4+
Suggested-by: Mathias Nyman 
Tested-by: Chris Chiu 
Acked-by: Alan Stern 
Signed-off-by: Kishon Vijay Abraham I 
Link: https://lore.kernel.org/r/20210909064200.16216-2-kishon@ti.com
Signed-off-by: Greg Kroah-Hartman

usb: core: hcd: use map_urb_for_dma for single step set feature urb

2021-05-13T14:02:30+00:00

Use map_urb_for_dma() to improve the dma map code for single step
set feature request urb in test mode.

Signed-off-by: Li Jun 
Acked-by: Alan Stern 
Link: https://lore.kernel.org/r/1620452039-11694-3-git-send-email-jun.li@nxp.com
Signed-off-by: Greg Kroah-Hartman

usb: host: move EH SINGLE_STEP_SET_FEATURE implementation to core

2021-05-13T14:01:59+00:00

It is needed at USB Certification test for Embedded Host 2.0, and
the detail is at CH6.4.1.1 of On-The-Go and Embedded Host Supplement
to the USB Revision 2.0 Specification. Since other USB 2.0 capable
host like XHCI also need it, so move it to HCD core.

Acked-by: Alan Stern 
Signed-off-by: Peter Chen 
Signed-off-by: Li Jun 
Link: https://lore.kernel.org/r/1620452039-11694-1-git-send-email-jun.li@nxp.com
Signed-off-by: Greg Kroah-Hartman