[PATCH] usbnet: ipheth: prevent TX queue timeouts when device not ready

2017-11-13 Thread Alexander Kappner
iOS devices require the host to be "trusted" before servicing network
packets. Establishing trust requires the user to confirm a dialog on the
iOS device.Until trust is established, the iOS device will silently discard
network packets from the host. Currently, the ipheth driver does not detect
whether an iOS device has established trust with the host, and immediately
sets up the transmit queues.

This causes the following problems:

- Kernel taint due to WARN() in netdev watchdog.
- Dmesg spam ("TX timeout").
- Disruption of user space networking activity (dhcpd, etc...) when new
interface comes up but cannot be used.
- Unnecessary host and device wakeups and USB traffic

Example dmesg output:

[ 1101.319778] NETDEV WATCHDOG: eth1 (ipheth): transmit queue 0 timed out
[ 1101.319817] [ cut here ]
[ 1101.319828] WARNING: CPU: 0 PID: 0 at net/sched/sch_generic.c:316 
dev_watchdog+0x20f/0x220
[ 1101.319831] Modules linked in: ipheth usbmon nvidia_drm(PO) 
nvidia_modeset(PO) nvidia(PO) iwlmvm mac80211 iwlwifi btusb btrtl btbcm btintel 
qmi_wwan bluetooth cfg80211 ecdh_generic thinkpad_acpi rfkill [last unloaded: 
ipheth]
[ 1101.319861] CPU: 0 PID: 0 Comm: swapper/0 Tainted: P   O
4.13.12.1 #1
[ 1101.319864] Hardware name: LENOVO 20ENCTO1WW/20ENCTO1WW, BIOS N1EET62W (1.35 
) 11/10/2016
[ 1101.319867] task: 81e11500 task.stack: 81e0
[ 1101.319873] RIP: 0010:dev_watchdog+0x20f/0x220
[ 1101.319876] RSP: 0018:8810a3c03e98 EFLAGS: 00010292
[ 1101.319880] RAX: 003a RBX:  RCX: 
[ 1101.319883] RDX: 8810a3c15c48 RSI: 81ccbfc2 RDI: 
[ 1101.319886] RBP: 880c04ebc41c R08:  R09: 0379
[ 1101.319889] R10: 0100696589d0 R11: 0378 R12: 880c04ebc000
[ 1101.319892] R13:  R14: 0001 R15: 880c2865fc80
[ 1101.319896] FS:  () GS:8810a3c0() 
knlGS:
[ 1101.319899] CS:  0010 DS:  ES:  CR0: 80050033
[ 1101.319902] CR2: 7f3ff24ac000 CR3: 01e0a000 CR4: 003406f0
[ 1101.319905] DR0:  DR1:  DR2: 
[ 1101.319908] DR3:  DR6: fffe0ff0 DR7: 0400
[ 1101.319910] Call Trace:
[ 1101.319914]  
[ 1101.319921]  ? dev_graft_qdisc+0x70/0x70
[ 1101.319928]  ? dev_graft_qdisc+0x70/0x70
[ 1101.319934]  ? call_timer_fn+0x2e/0x170
[ 1101.319939]  ? dev_graft_qdisc+0x70/0x70
[ 1101.319944]  ? run_timer_softirq+0x1ea/0x440
[ 1101.319951]  ? timerqueue_add+0x54/0x80
[ 1101.319956]  ? enqueue_hrtimer+0x38/0xa0
[ 1101.319963]  ? __do_softirq+0xed/0x2e7
[ 1101.319970]  ? irq_exit+0xb4/0xc0
[ 1101.319976]  ? smp_apic_timer_interrupt+0x39/0x50
[ 1101.319981]  ? apic_timer_interrupt+0x8c/0xa0
[ 1101.319983]  
[ 1101.319992]  ? cpuidle_enter_state+0xfa/0x2a0
[ 1101.31]  ? do_idle+0x1a3/0x1f0
[ 1101.320004]  ? cpu_startup_entry+0x5f/0x70
[ 1101.320011]  ? start_kernel+0x444/0x44c
[ 1101.320017]  ? early_idt_handler_array+0x120/0x120
[ 1101.320023]  ? x86_64_start_kernel+0x145/0x154
[ 1101.320028]  ? secondary_startup_64+0x9f/0x9f
[ 1101.320033] Code: 20 04 00 00 eb 9f 4c 89 e7 c6 05 59 44 71 00 01 e8 a7 df 
fd ff 89 d9 4c 89 e6 48 c7 c7 70 b7 cd 81 48 89 c2 31 c0 e8 97 64 90 ff <0f> ff 
eb bf 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00
[ 1101.320103] ---[ end trace 0cc4d251e2b57080 ]---
[ 1101.320110] ipheth 1-5:4.2: ipheth_tx_timeout: TX timeout

The last message "TX timeout" is repeated every 5 seconds until trust is
established or the device is disconnected, filling up dmesg.

The proposed patch eliminates the problem by, upon connection, keeping the
TX queue and carrier disabled until a packet is first received from the iOS
device. This is reflected by the confirmed_pairing variable in the device
structure. Only after at least one packet has been received from the iOS
device, the transmit queue and carrier are brought up during the periodic
device poll in ipheth_carrier_set. Because the iOS device will always send
a packet immediately upon trust being established, this should not delay
the interface becoming useable. To prevent failed UBRs in
ipheth_rcvbulk_callback from perpetually re-enabling the queue if it was
disabled, a new check is added so only successful transfers re-enable the
queue, whereas failed transfers only trigger an immediate poll.

This has the added benefit of removing the periodic control requests to the
iOS device until trust has been established and thus should reduce wakeup
events on both the host and the iOS device.

Signed-off-by: Alexander Kappner <a...@godking.net>
---
 drivers/net/usb/ipheth.c | 30 +-
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/net/usb/ipheth.c b/drivers/net/usb/ipheth.c
index d49c710..ca71f6c 100644
--- a/drivers/net/usb/ipheth.c
+++ b

Re: [PATCH] usb-core: Fix potential null pointer dereference in xhci-debugfs.c

2017-12-08 Thread Alexander Kappner

Hi,

> I think we need to dig a bit deeper. It's good to check if spriv is 
valid

but there are probably other reasons than kzalloc failing.

I agree -- this small allocation is  unlikely to fail in practice. Also, 
while my patch prevents the kernel oops, it also prevents the debugfs 
entries from being created.


I've been debugging this more trying to come up with a better solution, 
but I might need some guidance as I'm not too familiar with the USB 
subsystem. The immediate cause of the crash was usbmuxd sending a 
USBDEVFS_SETCONFIGURATION ioctl to a device, which _only if it fails_ 
calls usb_hcd_alloc_bandwidth to try and reset the device, which in turn 
calls xhci_debugfs_create_endpoint. The ioctl handler acquires a 
device-specific lock via usb_lock_device.


When the system resumed from hibernate, xhci_resume was called. This in 
turn called xhci_mem_cleanup to deallocate the device structures, which 
include setting the debugfs_private pointer to NULL  (via 
xhci_free_virt_devices_depth_first). It thus seems likely that the ioctl 
is somehow racing with the hibernate. The call to xhci_resume is 
protected by a host-controller specific lock (xhci->lock) but it doesn't 
attempt to take the usb_lock_device device-specific lock.


Now my suspicion is that xhci_resume freed and zeroed the device 
structures while racing with the ioctl handler. I can't seem to find any 
exclusion mechanism that would prevent xhci_resume from racing with the 
USBDEVFS_SETCONFIGURATION ioctl (or any other ioctl, for that matter). 
Am I missing something? If not, is there any reason why an ioctl might 
need to execute in parallel with the xhci_resume? If not, can we just do 
a busy wait in xhci_resume until all pending ioctls have returned?


> (Added this as reply subject, please remember to send mail with 
proper subject)

My apologies, thanks for fixing.


On 12/07/2017 04:47 AM, Mathias Nyman wrote:

On 07.12.2017 11:26, Alexander Kappner wrote:

Date: Wed, 6 Dec 2017 15:28:37 -0800
Subject: [PATCH] usb-core: Fix potential null pointer dereference in 
xhci-debugfs.c


(Added this as reply subject, please remember to send mail with proper 
subject)




My kernel crashed just after resuming from hibernate and starting 
usbmuxd

(a user-space daemon for iOS device pairing) with several USB devices
connected (dmesg attached).

Backtrace leads to:

0x8170465d is in xhci_debugfs_create_endpoint
(drivers/usb/host/xhci-debugfs.c:381).
376   int ep_index)
377 {
378 struct xhci_ep_priv *epriv;
379 struct xhci_slot_priv   *spriv = dev->debugfs_private;
380
381 if (spriv->eps[ep_index])
382 return;
383
384 epriv = kzalloc(sizeof(*epriv), GFP_KERNEL);
385 if (!epriv)

The read violation happens at address 0x40 and sizeof(struct
xhci_ep_priv)=0x40, so it seems ep_index is 1 and spriv is NULL here.



Thanks for reporting and debugging this, much appreciated.


spriv gets allocated in xhci_debugfs_create_slot:

...
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
 return;
...

There's no separate error path if this allocation fails, so we might be
left with NULL in priv. Subsequent users of priv thus need to check for
this NULL - so this is what the patch does.



I think we need to dig a bit deeper. It's good to check if spriv is valid
but there are probably other reasons than kzalloc failing.

Resume from hibernation will reset the xhci controller, reinitializing 
a lot.

It could be related.

-Mathias

(keeping rest of message as reference)


There might be other ways of triggering this null pointer dereference,
including when xhci_resume frees the device structures (e.g. after
returning from a hibernate), but I wasn't able to find or reproduce it.

[63953.758083] BUG: unable to handle kernel NULL pointer dereference at
0040
[63953.758090] IP: xhci_debugfs_create_endpoint+0x1d/0xa0
[63953.758091] PGD bb911d067 P4D bb911d067 PUD 10500ff067 PMD 0
[63953.758093] Oops:  [#1] PREEMPT SMP
[63953.758094] Modules linked in: ipheth tun nvidia_modeset(PO) iwlmvm
mac80211 iwlwifi nvidia(PO) btusb btrtl btbcm btintel bluetooth cfg80211
qmi_wwan ecdh_generic thinkpad_acpi rfkill
[63953.758103] CPU: 4 PID: 27091 Comm: usbmuxd Tainted: P   O
4.14.0.1-12769-g1deab8c #1
[63953.758104] Hardware name: LENOVO 20ENCTO1WW/20ENCTO1WW, BIOS 
N1EET62W

(1.35 ) 11/10/2016
[63953.758105] task: 8810527ba0c0 task.stack: c9000a8ec000
[63953.758107] RIP: 0010:xhci_debugfs_create_endpoint+0x1d/0xa0
[63953.758108] RSP: 0018:c9000a8efc80 EFLAGS: 00010206
[63953.758109] RAX:  RBX: 88105a71c000 RCX:
0003
[63953.758110] RDX: 0003 RSI: 880c0b57e000 RDI:
88105a71c238
[63953.758110] RBP: 0003 R08: 881063800600 R09:
0003
[63953.758111] R10: 88105a71c238 R11: 0001 R12:
0

[no subject]

2017-12-07 Thread Alexander Kappner
Date: Wed, 6 Dec 2017 15:28:37 -0800
Subject: [PATCH] usb-core: Fix potential null pointer dereference in 
xhci-debugfs.c

My kernel crashed just after resuming from hibernate and starting usbmuxd
(a user-space daemon for iOS device pairing) with several USB devices
connected (dmesg attached).

Backtrace leads to:

0x8170465d is in xhci_debugfs_create_endpoint
(drivers/usb/host/xhci-debugfs.c:381).
376   int ep_index)
377 {
378 struct xhci_ep_priv *epriv;
379 struct xhci_slot_priv   *spriv = dev->debugfs_private;
380
381 if (spriv->eps[ep_index])
382 return;
383
384 epriv = kzalloc(sizeof(*epriv), GFP_KERNEL);
385 if (!epriv)

The read violation happens at address 0x40 and sizeof(struct
xhci_ep_priv)=0x40, so it seems ep_index is 1 and spriv is NULL here.

spriv gets allocated in xhci_debugfs_create_slot:

...
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
return;
...

There's no separate error path if this allocation fails, so we might be
left with NULL in priv. Subsequent users of priv thus need to check for
this NULL - so this is what the patch does.

There might be other ways of triggering this null pointer dereference,
including when xhci_resume frees the device structures (e.g. after
returning from a hibernate), but I wasn't able to find or reproduce it. 

[63953.758083] BUG: unable to handle kernel NULL pointer dereference at
0040
[63953.758090] IP: xhci_debugfs_create_endpoint+0x1d/0xa0
[63953.758091] PGD bb911d067 P4D bb911d067 PUD 10500ff067 PMD 0
[63953.758093] Oops:  [#1] PREEMPT SMP
[63953.758094] Modules linked in: ipheth tun nvidia_modeset(PO) iwlmvm
mac80211 iwlwifi nvidia(PO) btusb btrtl btbcm btintel bluetooth cfg80211
qmi_wwan ecdh_generic thinkpad_acpi rfkill
[63953.758103] CPU: 4 PID: 27091 Comm: usbmuxd Tainted: P   O
4.14.0.1-12769-g1deab8c #1
[63953.758104] Hardware name: LENOVO 20ENCTO1WW/20ENCTO1WW, BIOS N1EET62W
(1.35 ) 11/10/2016
[63953.758105] task: 8810527ba0c0 task.stack: c9000a8ec000
[63953.758107] RIP: 0010:xhci_debugfs_create_endpoint+0x1d/0xa0
[63953.758108] RSP: 0018:c9000a8efc80 EFLAGS: 00010206
[63953.758109] RAX:  RBX: 88105a71c000 RCX:
0003
[63953.758110] RDX: 0003 RSI: 880c0b57e000 RDI:
88105a71c238
[63953.758110] RBP: 0003 R08: 881063800600 R09:
0003
[63953.758111] R10: 88105a71c238 R11: 0001 R12:
0011
[63953.758112] R13: 0018 R14:  R15:

[63953.758113] FS:  7f0a77715700() GS:8810a3d0()
knlGS:
[63953.758114] CS:  0010 DS:  ES:  CR0: 80050033
[63953.758115] CR2: 0040 CR3: 0003f91a8006 CR4:
003606e0
[63953.758115] DR0:  DR1:  DR2:

[63953.758116] DR3:  DR6: fffe0ff0 DR7:
0400
[63953.758117] Call Trace:
[63953.758120]  xhci_add_endpoint+0x127/0x2b0
[63953.758123]  usb_hcd_alloc_bandwidth+0x1ad/0x300
[63953.758125]  usb_set_configuration+0x1c8/0x880
[63953.758128]  usbdev_do_ioctl+0xc41/0x1120
[63953.758130]  usbdev_ioctl+0xa/0x10
[63953.758151]  do_vfs_ioctl+0x8b/0x5c0
[63953.758153]  ? __fget+0x6c/0xb0
[63953.758155]  SyS_ioctl+0x76/0x90
[63953.758157]  do_syscall_64+0x6b/0x290
[63953.758173]  entry_SYSCALL64_slow_path+0x25/0x25
[63953.758175] RIP: 0033:0x7f0a76a151c7
[63953.758175] RSP: 002b:7ffd1431b0c8 EFLAGS: 0202 ORIG_RAX:
0010
[63953.758177] RAX: ffda RBX: 023239a0 RCX:
7f0a76a151c7
[63953.758178] RDX: 7ffd1431b0dc RSI: 80045505 RDI:
000e
[63953.758178] RBP: 023240c0 R08: 7ffd1431b008 R09:
0004
[63953.758179] R10: 7ffd1431aec0 R11: 0202 R12:
023240c0
[63953.758180] R13: 0001 R14: 0056 R15:
0038
[63953.758182] Code: e9 39 ff ff ff 66 0f 1f 84 00 00 00 00 00 0f 1f 44 00
00 41 57 41 56 41 55 41 54 55 48 63 ea 53 4c 8b b6 88 15 00 00 4d 8d 2c ee
<49> 83 7d 28 00 74 0b 5b 5d 41 5c 41 5d 41 5e 41 5f c3 48 8b 3d
[63953.758202] RIP: xhci_debugfs_create_endpoint+0x1d/0xa0 RSP:
c9000a8efc80
[63953.758203] CR2: 0040
[63953.758204] ---[ end trace 1f7ea9a959f02054 ]---

Signed-off-by: Alexander Kappner <a...@godking.net>
---
 drivers/usb/host/xhci-debugfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index 4f7895d..1cea59c 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -378,6 +378,9 @@ void xhci_debugfs_create_endpoint(struct xhci_hcd *xhci,
struct xhci_ep_priv *epriv;
struct xhci_slot_priv   *spriv = dev->debugfs_private;
 
+   if (!spriv)
+   return;
+
 

[PATCH] xhci: Fix use-after-free in xhci debugfs

2017-12-10 Thread Alexander Kappner
Trying to read from debugfs after the system has resumed from
 hibernate causes a use-after-free and thus a protection fault.

Steps to reproduce:
Hibernate system, resume from hibernate, then run
$ cat /sys/kernel/debug/usb/xhci/*/command-ring/enqueue

dmesg below:

[ 3902.765086] general protection fault:  [#1] PREEMPT SMP
[ 3902.765095] Modules linked in: ipheth nvidia_modeset(PO) iwlmvm mac80211 
nvidia(PO) iwlwifi qmi_wwan cfg80211 thinkpad_acpi rfkill
[ 3902.765118] CPU: 4 PID: 3591 Comm: cat Tainted: P   O 
4.14.0.1-12769-g1deab8c #1
[ 3902.765121] Hardware name: LENOVO 20ENCTO1WW/20ENCTO1WW, BIOS N1EET62W (1.35 
) 11/10/2016
[ 3902.765125] task: 88100263d040 task.stack: c90006e5
[ 3902.765136] RIP: 0010:xhci_trb_virt_to_dma.part.50+0x5/0x30
[ 3902.765140] RSP: 0018:c90006e53da8 EFLAGS: 00010286
[ 3902.765144] RAX: 88100a57e680 RBX: 8810032f1900 RCX: 881013800900
[ 3902.765147] RDX:  RSI: 881000abf488 RDI: 0d0c0a0bb1637008
[ 3902.765151] RBP: c90006e53f00 R08:  R09: 
[ 3902.765154] R10: 7ffcd73165e0 R11: 88100263d040 R12: 880bd5afcc00
[ 3902.765157] R13: 0002 R14: 0001 R15: 8810032f1900
[ 3902.765161] FS:  7f6aa6cec700() GS:881053d0() 
knlGS:
[ 3902.765165] CS:  0010 DS:  ES:  CR0: 80050033
[ 3902.765168] CR2: 7f6aa6d08008 CR3: 0010033f9003 CR4: 003606e0
[ 3902.765172] DR0:  DR1:  DR2: 
[ 3902.765175] DR3:  DR6: fffe0ff0 DR7: 0400
[ 3902.765178] Call Trace:
[ 3902.765188]  xhci_ring_enqueue_show+0x1e/0x40
[ 3902.765197]  seq_read+0xdb/0x3a0
[ 3902.765204]  ? __handle_mm_fault+0x5fb/0x1210
[ 3902.765211]  full_proxy_read+0x4a/0x70
[ 3902.765219]  __vfs_read+0x23/0x120
[ 3902.765228]  vfs_read+0x8e/0x130
[ 3902.765235]  SyS_read+0x42/0x90
[ 3902.765242]  do_syscall_64+0x6b/0x290
[ 3902.765251]  entry_SYSCALL64_slow_path+0x25/0x25
[ 3902.765256] RIP: 0033:0x7f6aa683cba0
[ 3902.765260] RSP: 002b:7ffcd7316818 EFLAGS: 0246 ORIG_RAX: 

[ 3902.765264] RAX: ffda RBX: 0002 RCX: 7f6aa683cba0
[ 3902.765267] RDX: 0002 RSI: 7f6aa6d09000 RDI: 0003
[ 3902.765270] RBP: 0002 R08:  R09: 
[ 3902.765274] R10: 7ffcd73165e0 R11: 0246 R12: 7f6aa6d09000
[ 3902.765277] R13: 0003 R14:  R15: 0002
[ 3902.765282] Code: 0f 44 c0 49 8b 04 24 52 48 c7 c2 30 cd cb 81 48 8d b0 98 
00 00 00 31 c0 e8 39 e6 dc ff 58 e9 75 ff ff ff 0f 1f 00 0f 1f 44 00 00 <48> 8b 
17 31 c0 48 39 f2 77 13 48 29 d6 48 81 fe ff 0f 00 00 77
[ 3902.765371] RIP: xhci_trb_virt_to_dma.part.50+0x5/0x30 RSP: c90006e53da8
[ 3902.765376] ---[ end trace 9ee53de1dccf7d8e ]---

The issue is caused by the xhci ring structures being reallocated
when the system is resumed, but pointers to the old structures
being retained in the debugfs files "private" field:

(gdb) list *(xhci_ring_enqueue_show+0x1e)
0x8170403e is in xhci_ring_enqueue_show 
(drivers/usb/host/xhci-debugfs.c:168).
163 {
164 dma_addr_t  dma;
165 struct xhci_ring*ring = s->private;
166
167 dma = xhci_trb_virt_to_dma(ring->enq_seg, ring->enqueue);
168 seq_printf(s, "%pad\n", );
169
170 return 0;
171 }
172

The proposed patch fixes this issue by storing a pointer to the xhci_ring
field in the xhci device structure in debugfs rather than directly
storing a pointer to the xhci_ring.

Signed-off-by: Alexander Kappner <a...@godking.net>
---
 drivers/usb/host/xhci-debugfs.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index 4f7895d..60aa30a 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -162,7 +162,7 @@ static void xhci_debugfs_extcap_regset(struct xhci_hcd 
*xhci, int cap_id,
 static int xhci_ring_enqueue_show(struct seq_file *s, void *unused)
 {
dma_addr_t  dma;
-   struct xhci_ring*ring = s->private;
+   struct xhci_ring*ring = *(struct xhci_ring **)s->private;
 
dma = xhci_trb_virt_to_dma(ring->enq_seg, ring->enqueue);
seq_printf(s, "%pad\n", );
@@ -173,7 +173,7 @@ static int xhci_ring_enqueue_show(struct seq_file *s, void 
*unused)
 static int xhci_ring_dequeue_show(struct seq_file *s, void *unused)
 {
dma_addr_t  dma;
-   struct xhci_ring*ring = s->private;
+   struct xhci_ring*ring = *(struct xhci_ring **)s->private;
 
dma = xhci_trb_virt_to_dma(ring->deq_seg, ring->dequeue);
seq_printf(s, "%pad\n&quo

Re: [PATCH] usb-core: Fix potential null pointer dereference in xhci-debugfs.c

2017-12-09 Thread Alexander Kappner

Hi Mathias,

thanks for the patch! The system now resumes cleanly from hibernate even 
with usbmuxd doing its thing.


Tested-by: Alexander Kappner <a...@godking.net>

While testing this I hit some other issues with xhci-debugfs.c but I'll 
write these up in a separate bug.


On 12/08/2017 09:01 AM, Mathias Nyman wrote:

On 08.12.2017 13:06, Alexander Kappner wrote:

Hi,


I think we need to dig a bit deeper. It's good to check if spriv is
valid
but there are probably other reasons than kzalloc failing.


I agree -- this small allocation is  unlikely to fail in practice.
Also, while my patch prevents the kernel oops, it also prevents the
debugfs entries from being created.

I've been debugging this more trying to come up with a better
solution, but I might need some guidance as I'm not too familiar with
the USB subsystem. The immediate cause of the crash was usbmuxd
sending a USBDEVFS_SETCONFIGURATION ioctl to a device, which _only if
it fails_ calls usb_hcd_alloc_bandwidth to try and reset the device,
which in turn calls xhci_debugfs_create_endpoint. The ioctl handler
acquires a device-specific lock via usb_lock_device.

When the system resumed from hibernate, xhci_resume was called. This
in turn called xhci_mem_cleanup to deallocate the device structures,
which include setting the debugfs_private pointer to NULL  (via
xhci_free_virt_devices_depth_first). It thus seems likely that the
ioctl is somehow racing with the hibernate. The call to xhci_resume
is protected by a host-controller specific lock (xhci->lock) but it
doesn't attempt to take the usb_lock_device device-specific lock.

Now my suspicion is that xhci_resume freed and zeroed the device
structures while racing with the ioctl handler. I can't seem to find
any exclusion mechanism that would prevent xhci_resume from racing
with the USBDEVFS_SETCONFIGURATION ioctl (or any other ioctl, for
that matter). Am I missing something? If not, is there any reason why
an ioctl might need to execute in parallel with the xhci_resume? If
not, can we just do a busy wait in xhci_resume until all pending
ioctls have returned?


I'm not sure, but if I recall correctly then power management is supposed
to make sure a driver doesn't access usb devices while the host 
controller

is still resuming.

The odd thing here is that
xhci_debugfs_remove_slot(xhci, slot_id), and
xhci_free_virt_device(xhci, slot_id) are called together when
xhci_mem_cleanup() calls xhci_free_virt_devices_depth_first()

That means both the xhci_virt_device *dev and dev->debugfs_private
should both be freed and xhci->devs[slot_id] set NULL for that 
virt_device.


so xhci_add_endpoint() should fail a lot earlier because the 
xhci->devs[slot_id]

should be a null pointer as well.

Allocation is also done together in xhci_alloc_dev()

Looking at it more closely there is actually the .free_dev callback that
first frees the dev->debugs_private but the virt_dev is only freed
conditionally later

Attached a patch that frees them together, can you try it out?

If it doesn't help we need to add some elaborate tracing

Thanks
-Mathias




--
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-storage: Add quirks to make G-Technology "G-Drive" work

2018-05-17 Thread Alexander Kappner
Hi Alan,

thanks for reviewing. (This is my first contribution that touches 
usb-storage, so please bear with me.)

> That's kind of weird.  Does the drive work under Windows in UAS mode?  

On the Windows 10 VM that I just spun up for testing this, access to the 
drive uses "usbstor.inf" (rather than "uaspstor.sys"). So I believe the 
answer is no. 

> If so, why are the WRITE(16) commands failing under Linux?

I don't know exactly why they're failing, but another entry in the 
unusual_uas.h shows another SimpleTech device (only with a slightly 
different product ID) needing the same UAS quirk (see 
https://bugzilla.redhat.com/show_bug.cgi?id=1124119). So my guess is those 
drives are just buggy. 

> That doesn't quite make sense.  Since you prevent the uas driver from 
> binding to this device, it will end up using usb-storage no matter how 
> the kernel was built.  Therefore the second quirk flag has to go into 
> unusual_devs.h no matter what.

Yes that's what I was trying to get at. So even though the UAS flag would 
conceptually belong into unusual_uas, I'm putting both into unusual_devs to 
avoid having to create two separate entries for the same device.

> You don't describe the second quirk flag at all.  Are you sure it is 
> needed?

Yes. Without this flag, the device keeps throwing similar errors on 
usb-storage. That's the same result I get on a host that doesn't have UAS 
compiled in. Here's a dmesg:

[2.183472] usb 3-1: New USB device found, idVendor=4971, idProduct=8024, 
bcdDevice=24.03
[2.184285] usb 3-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[2.184980] usb 3-1: Product: G-DRIVE
[2.185447] usb 3-1: Manufacturer: HGST
[2.185829] usb 3-1: SerialNumber: AA015010004C
[2.195509] usb-storage 3-1:1.0: USB Mass Storage device detected
[2.198668] Floppy drive(s): fd0 is 2.88M AMI BIOS
[2.202981] scsi host2: usb-storage 3-1:1.0
...
[3.233085] scsi 2:0:0:0: Direct-Access G-DRIVE   2403 
PQ: 0 ANSI: 6
[3.234514] sd 2:0:0:0: Attached scsi generic sg1 type 0
[3.235465] sd 2:0:0:0: [sda] Very big device. Trying to use READ 
CAPACITY(16).
[3.236847] sd 2:0:0:0: [sda] 7814037168 512-byte logical blocks: (4.00 
TB/3.64 TiB)
[3.237794] sd 2:0:0:0: [sda] 4096-byte physical blocks
[3.241255] sd 2:0:0:0: [sda] Write Protect is off
[3.242096] sd 2:0:0:0: [sda] Mode Sense: 47 00 10 08
[3.243595] sd 2:0:0:0: [sda] Write cache: enabled, read cache: enabled, 
supports DPO and FUA
[3.257893]  sda: sda1 sda9
[3.261402] sd 2:0:0:0: [sda] Attached SCSI disk
...
[   92.433428] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE
[   92.434759] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] 
[   92.435637] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb
[   92.436401] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 
00 00 00 00 00 08 00 00
[   92.437493] print_req_error: critical target error, dev sda, sector 0
[   92.438211] Buffer I/O error on dev sda, logical block 0, lost sync page 
write
[   92.516692] EXT4-fs (sda): mounted filesystem with ordered data mode. Opts: 
(null)
[  101.449311] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE
[  101.450598] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] 
[  101.451401] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb
[  101.452041] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 
00 18 00 00 00 08 00 00
[  101.452906] print_req_error: critical target error, dev sda, sector 
3905159192
[  101.453593] print_req_error: critical target error, dev sda, sector 
3905159192
[  101.454889] Aborting journal on device sda-8.
[  101.457103] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE
[  101.457988] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] 
[  101.458637] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb
[  101.459250] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 
00 00 00 00 00 08 00 00

Source code comments describe this as a known problem (scsiglue.c:182):

/*
 * Some devices don't like MODE SENSE with page=0x3f,
 * which is the command used for checking if a device
 * is write-protected.  Now that we tell the sd driver
 * to do a 192-byte transfer with this command the
 * majority of devices work fine, but a few still can't
 * handle it.  The sd driver will simply assume those
 * devices are write-enabled.
 */
if (us->fflags & US_FL_NO_WP_DETECT)
sdev->skip_ms_page_3f = 1;

--agk 



On 05/16/2018 01:55 PM, Alan Stern wrote:
> On Wed, 16 May 2018, Alexander Kappner wrote:
> 
>> The "G-Drive" (sold

[PATCH v2 2/2] [usb-storage] Add compatibility quirk flags for G-Technologies G-Drive

2018-05-18 Thread Alexander Kappner
The "G-Drive" (sold by G-Technology) external USB 3.0 drive
 hangs on write access under UAS and usb-storage:

[  136.079121] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE
[  136.079144] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current]
[  136.079152] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb
[  136.079176] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 
00 00 00 00 00 08 00 00
[  136.079180] print_req_error: critical target error, dev sdi, sector 0
[  136.079183] Buffer I/O error on dev sdi, logical block 0, lost sync page 
write
[  136.173148] EXT4-fs (sdi): mounted filesystem with ordered data mode. Opts: 
(null)
[  140.583998] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE
[  140.584010] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current]
[  140.584016] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb
[  140.584022] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 
00 18 00 00 00 08 00 00
[  140.584025] print_req_error: critical target error, dev sdi, sector 
3905159192
[  140.584044] print_req_error: critical target error, dev sdi, sector 
3905159192
[  140.584052] Aborting journal on device sdi-8.

The proposed patch adds compatibility quirks. Because the drive requires two
quirks (one to work with UAS, and another to work with usb-storage), adding this
under unusual_devs.h and not just unusual_uas.h so kernels compiled without UAS
receive the quirk. With the patch, the drive works reliably on UAS and usb-
storage.
(tested on NEC Corporation uPD720200 USB 3.0 host controller).

Signed-off-by: Alexander Kappner <a...@godking.net>
---
 drivers/usb/storage/unusual_devs.h | 9 +
 drivers/usb/storage/unusual_uas.h  | 9 +
 2 files changed, 18 insertions(+)

diff --git a/drivers/usb/storage/unusual_devs.h 
b/drivers/usb/storage/unusual_devs.h
index 747d3a9..22fcfcc 100644
--- a/drivers/usb/storage/unusual_devs.h
+++ b/drivers/usb/storage/unusual_devs.h
@@ -2321,6 +2321,15 @@ UNUSUAL_DEV(  0x4146, 0xba01, 0x0100, 0x0100,
"Micro Mini 1GB",
USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NOT_LOCKABLE ),
 
+/* "G-DRIVE" external HDD hangs on write without these.
+ * Patch submitted by Alexander Kappner <a...@godking.net>
+ */
+UNUSUAL_DEV(0x4971, 0x8024, 0x, 0x,
+   "SimpleTech",
+   "External HDD",
+   USB_SC_DEVICE, USB_PR_DEVICE, NULL,
+   US_FL_ALWAYS_SYNC),
+
 /*
  * Nick Bowler <nbow...@elliptictech.com>
  * SCSI stack spams (otherwise harmless) error messages.
diff --git a/drivers/usb/storage/unusual_uas.h 
b/drivers/usb/storage/unusual_uas.h
index 38434d8..d0bdebd 100644
--- a/drivers/usb/storage/unusual_uas.h
+++ b/drivers/usb/storage/unusual_uas.h
@@ -107,3 +107,12 @@ UNUSUAL_DEV(0x4971, 0x8017, 0x, 0x,
"External HDD",
USB_SC_DEVICE, USB_PR_DEVICE, NULL,
US_FL_NO_REPORT_OPCODES),
+
+/* "G-DRIVE" external HDD hangs on write without these.
+ * Patch submitted by Alexander Kappner <a...@godking.net>
+ */
+UNUSUAL_DEV(0x4971, 0x8024, 0x, 0x,
+   "SimpleTech",
+   "External HDD",
+   USB_SC_DEVICE, USB_PR_DEVICE, NULL,
+   US_FL_ALWAYS_SYNC),
-- 
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


[PATCH v2 1/2] [usb-storage] Add support for FL_ALWAYS_SYNC flag in the UAS driver

2018-05-18 Thread Alexander Kappner
The ALWAYS_SYNC flag is currently honored by the usb-storage driver but not UAS
and is required to work around devices that become unstable upon being
queried for cache. This code is taken straight from:
drivers/usb/storage/scsiglue.c:284

Signed-off-by: Alexander Kappner <a...@godking.net>
---
 drivers/usb/storage/uas.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 6034c39..9e9de54 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -836,6 +836,12 @@ static int uas_slave_configure(struct scsi_device *sdev)
if (devinfo->flags & US_FL_BROKEN_FUA)
sdev->broken_fua = 1;
 
+   /* UAS also needs to support FL_ALWAYS_SYNC */
+   if (devinfo->flags & US_FL_ALWAYS_SYNC) {
+   sdev->skip_ms_page_3f = 1;
+   sdev->skip_ms_page_8 = 1;
+   sdev->wce_default_on = 1;
+   }
scsi_change_queue_depth(sdev, devinfo->qdepth - 2);
return 0;
 }
-- 
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


[PATCH v2 0/2] Re: usb-storage: Add quirks to make G-Technology "G-Drive" work

2018-05-18 Thread Alexander Kappner
v2 of this patch implements the FL_ALWAYS_SYNC flag in the UAS driver, and then
adds the flag as quirks for the device at issue. This allows the G-Drive to work
under both UAS and usb-storage.

Alexander Kappner (2):
  [usb-storage] Add support for FL_ALWAYS_SYNC flag in the UAS driver
  [usb-storage] Add compatibility quirk flags for G-Technologies G-Drive

 drivers/usb/storage/uas.c  | 6 ++
 drivers/usb/storage/unusual_devs.h | 9 +
 drivers/usb/storage/unusual_uas.h  | 9 +
 3 files changed, 24 insertions(+)

-- 
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: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work

2018-05-18 Thread Alexander Kappner
> Was this tested with uas or usb-storage?
On both. dd works either way.  

> Are you certain Oliver's new code was executed?  If you put 
> US_FL_NO_WP_DETECT only in unusual_devs.h and not in ususual_uas.h then 
> it would not affect the uas driver.
Yes, the code ran.

Further debugging shows that the code that causes the device to hang is in 
drivers/scsi/sd.c:2698. So the reason why usb-storage works and UAS does 
not is because the device setting both skip_ms_page_3f=1 and 
skip_ms_page_8=1 is required. The US_FL_NO_WP_DETECT flag does the former, 
and usb-storage (but not UAS) by default does the latter 
(drivers/usb/storage/scsiglue.c:198):

 /*
 * A number of devices have problems with MODE SENSE for
 * page x08, so we will skip it.
 */
sdev->skip_ms_page_8 = 1;


Forcing both skip_ms_page_3f and skip_ms_page_8 to 1 results in UAS and 
usb-storage working. Oliver's code only set skip_ms_page_3f. 

Do we want a patch to introduce a quirk flag for skip_ms_page_8,  similar to 
the US_FL_NO_WP_DETECT quirk bit for skip_ms_page_3f? This may even resolve 
the issues with other devices in unusual_uas.h that currently have all UAS 
support disabled. I'd be happy to write the patch, but I'm not sure we want 
to reserve a quirk bit for what's currently only a single device known to 
have this issue.  Please advise. 


On 05/17/2018 12:13 PM, Alan Stern wrote:
> On Thu, 17 May 2018, Alexander Kappner wrote:
> 
>> Oliver and Alan,
>>
>> thank for investigating.
>>
>>> this is suspicious. You do not actually whether US_FL_NO_WP_DETECT
>>> by itself would make the device work. Can you please test that?
>>
>> US_FL_NO_WP_DETECT without US_FL_IGNORE_UAS does not make a difference, 
>> even with the patch you included applied:
> 
> Are you certain Oliver's new code was executed?  If you put 
> US_FL_NO_WP_DETECT only in unusual_devs.h and not in ususual_uas.h then 
> it would not affect the uas driver.
> 
>>> That's bizarre too.  Even though the only difference is a MODE SENSE 
>>> command, the command that actually faliled was WRITE(16).
>> It looks to me like the MODE SENSE simply hangs the drive, so anything 
>> issued after that will fail. Of course the drive says it's the "current 
>> command" that caused the failure, but I wouldn't give too much credence to 
>> that. FYI -- this device is a consumer grade rotational drive that you can 
>> get for less than $200, so I wouldn't be surprised if the implementations 
>> have issues. 
>>
>> Also, I noticed that copying onto the drive with dd works fine, whereas 
>> trying to mount a filesystem immediately crashes it. I suspect this is 
>> because check_disk_change is called on mount (which eventually calls down 
>> to sd_read_write_protect_flag, which is where the US_FL_NO_WP_DETECT flag 
>> comes into play).
> 
> Was this tested with uas or usb-storage?
> 
> 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
> 
--
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-storage: Add quirks to make G-Technology "G-Drive" work

2018-05-17 Thread Alexander Kappner
Oliver and Alan,

thank for investigating.

> this is suspicious. You do not actually whether US_FL_NO_WP_DETECT
> by itself would make the device work. Can you please test that?

US_FL_NO_WP_DETECT without US_FL_IGNORE_UAS does not make a difference, 
even with the patch you included applied:

[   44.108417] JBD2: Clearing recovery information on journal
[   44.119593] sd 2:0:0:0: [sda] tag#1 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE
[   44.121605] sd 2:0:0:0: [sda] tag#1 Sense Key : Illegal Request 
[current] 
[   44.123254] sd 2:0:0:0: [sda] tag#1 Add. Sense: Invalid field in cdb
[   44.124798] sd 2:0:0:0: [sda] tag#1 CDB: Write(16) 8a 08 00 00 00 00 e8 
c4 00 00 00 00 00 08 00 00
[   44.126847] print_req_error: critical target error, dev sda, sector 
3905159168
[   44.128450] Buffer I/O error on dev sda, logical block 488144896, lost 
sync page write
[   44.130776] JBD2: Error -5 detected when updating journal superblock for 
sda-8.
[   44.141059] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE
[   44.141376] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request 
[current] 
[   44.141376] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb
[   44.141376] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 
00 00 00 00 00 00 08 00 00
[   44.141376] print_req_error: critical target error, dev sda, sector 0
[   44.141376] Buffer I/O error on dev sda, logical block 0, lost sync page 
write

> That's bizarre too.  Even though the only difference is a MODE SENSE 
> command, the command that actually faliled was WRITE(16).
It looks to me like the MODE SENSE simply hangs the drive, so anything 
issued after that will fail. Of course the drive says it's the "current 
command" that caused the failure, but I wouldn't give too much credence to 
that. FYI -- this device is a consumer grade rotational drive that you can 
get for less than $200, so I wouldn't be surprised if the implementations 
have issues. 

Also, I noticed that copying onto the drive with dd works fine, whereas 
trying to mount a filesystem immediately crashes it. I suspect this is 
because check_disk_change is called on mount (which eventually calls down 
to sd_read_write_protect_flag, which is where the US_FL_NO_WP_DETECT flag 
comes into play).



On 05/17/2018 05:58 AM, Oliver Neukum wrote:
> Am Donnerstag, den 17.05.2018, 01:15 -0700 schrieb Alexander Kappner:
>> Yes. Without this flag, the device keeps throwing similar errors on 
>> usb-storage. That's the same result I get on a host that doesn't have UAS 
>> compiled in. Here's a dmesg:
> 
> Hi,
> 
> this is suspicious. You do not actually whether US_FL_NO_WP_DETECT
> by itself would make the device work. Can you please test that?
> You will need the attached patch for the quirk to be supported.
> 
>   Regards
>   Oliver
> 
--
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-storage: Add quirks to make G-Technology "G-Drive" work

2018-05-16 Thread Alexander Kappner
The "G-Drive" (sold by G-Technology) external USB 3.0 drive
 hangs on write access under UAS:

[  136.079121] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE
[  136.079144] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current]
[  136.079152] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb
[  136.079176] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 
00 00 00 00 00 08 00 00
[  136.079180] print_req_error: critical target error, dev sdi, sector 0
[  136.079183] Buffer I/O error on dev sdi, logical block 0, lost sync page 
write
[  136.173148] EXT4-fs (sdi): mounted filesystem with ordered data mode. Opts: 
(null)
[  140.583998] sd 15:0:0:0: [sdi] tag#0 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE
[  140.584010] sd 15:0:0:0: [sdi] tag#0 Sense Key : Illegal Request [current]
[  140.584016] sd 15:0:0:0: [sdi] tag#0 Add. Sense: Invalid field in cdb
[  140.584022] sd 15:0:0:0: [sdi] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 
00 18 00 00 00 08 00 00
[  140.584025] print_req_error: critical target error, dev sdi, sector 
3905159192
[  140.584044] print_req_error: critical target error, dev sdi, sector 
3905159192
[  140.584052] Aborting journal on device sdi-8.

The proposed patch adds compatibility quirks. Because the drive requires two
quirks (one to disable UAS, and another to work with usb-storage), adding this
under unusual_devs.h and not unusual_uas.h so kernels compiled without UAS
receive the quirk. With the patch, the drive works reliably
(tested on NEC Corporation uPD720200 USB 3.0 host controller).

Signed-off-by: Alexander Kappner <a...@godking.net>
---
 drivers/usb/storage/unusual_devs.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/storage/unusual_devs.h 
b/drivers/usb/storage/unusual_devs.h
index 747d3a9..b8661a1 100644
--- a/drivers/usb/storage/unusual_devs.h
+++ b/drivers/usb/storage/unusual_devs.h
@@ -2321,6 +2321,15 @@ UNUSUAL_DEV(  0x4146, 0xba01, 0x0100, 0x0100,
"Micro Mini 1GB",
USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NOT_LOCKABLE ),
 
+/* "G-DRIVE" external HDD hangs on write without these.
+ * Reported-by: Alexander Kappner <a...@godking.net>
+ */
+UNUSUAL_DEV(0x4971, 0x8024, 0x, 0x,
+   "SimpleTech",
+   "External HDD",
+   USB_SC_DEVICE, USB_PR_DEVICE, NULL,
+   US_FL_IGNORE_UAS | US_FL_NO_WP_DETECT),
+
 /*
  * Nick Bowler <nbow...@elliptictech.com>
  * SCSI stack spams (otherwise harmless) error messages.
-- 
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: [BUG] SD card reader disappears after suspend

2018-01-01 Thread Alexander Kappner
I've seen similar issues caused by USB persist. Try disabling it (echo 0 > 
/sys/bus/usb/devices//power/persist), and then resuming. 

On 01/01/2018 10:53 PM, Samuel Sadok wrote:
> Hello!
> 
> I'm observing a bug where the most prominent symptom is that the
> built-in SD card reader disappears after waking up from suspend.
> 
> Laptop: MacBookPro11,3
> USB controller: Intel Corporation 8 Series/C220 Series Chipset Family
> USB xHCI (rev 05) (prog-if 30 [XHCI])
> SD card reader: Apple Card Reader 05ac:8406
> 
> The bug was reported previously on bugzilla but never followed up on:
> https://bugzilla.kernel.org/show_bug.cgi?id=111201
> 
> 
> Steps to reproduce:
> $ lsusb
> Bus 002 Device 002: ID 05ac:8406 Apple, Inc.
> Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 001 Device 006: ID 05ac:8289 Apple, Inc.
> Bus 001 Device 002: ID 0a5c:4500 Broadcom Corp. BCM2046B1 USB 2.0 Hub
> (part of BCM2046 Bluetooth)
> Bus 001 Device 003: ID 05ac:0263 Apple, Inc. Apple Internal Keyboard /
> Trackpad (MacBook Retina)
> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> $ sudo systemctl suspend
> [Wait 10s, then wake the system up]
> $ lsusb
> Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 001 Device 006: ID 05ac:8289 Apple, Inc.
> Bus 001 Device 002: ID 0a5c:4500 Broadcom Corp. BCM2046B1 USB 2.0 Hub
> (part of BCM2046 Bluetooth)
> Bus 001 Device 003: ID 05ac:0263 Apple, Inc. Apple Internal Keyboard /
> Trackpad (MacBook Retina)
> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> $ lsusb -vvv
> 
> Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> [long hang, on the order of three minutes]
> can't get hub descriptor, LIBUSB_ERROR_TIMEOUT (Resource temporarily
> unavailable)
> [and a couple of more errors]
> 
> Expected result:
> The output of both lsusb commands should be equal.
> 
> Actual results:
>  - The SD card reader (05ac:8406) is missing from the second lsusb.
>  - Running lsusb -vvv hangs for a long time
>  - lots of error messages in kernel log
> 
> Logs:
> https://gist.github.com/samuelsadok/6d7b3e3015d3370a92ed4702e4d3c4b5
> 
> I'm happy to provide more data, do tests or patch around the kernel if it 
> helps.
> 
> Best,
> Samuel
> --
> 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
> 
--
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