usb/core: usb_alloc_dev(): fix setting of ->portnum
PATCH v2 1/1 usb/core: usb_alloc_dev(): fix setting of ->portnum
With commit 69bec7259853 ("USB: core: let USB device know device node"), the port1 argument of usb_alloc_dev() gets overwritten as follows:
... usb_alloc_dev(..., unsigned port1)
{
...
if (!parent->parent) {
port1 = usb_hcd_find_raw_port_number(..., port1);
}
...
}Later on, this now overwritten port1 gets assigned to ->portnum:
dev->portnum = port1;
However, since xhci_find_raw_port_number() isn't idempotent, the aforementioned commit causes a number of KASAN splats like the following:
BUG: KASAN: slab-out-of-bounds in xhci_find_raw_port_number+0x98/0x170
at addr ffff8801d9311670
Read of size 8 by task kworker/2:1/87
[...]
Workqueue: usb_hub_wq hub_event
0000000000000188 000000005814b877 ffff8800cba17588 ffffffff8191447e
0000000041b58ab3 ffffffff82a03209 ffffffff819143a2 ffffffff82a252f4
ffff8801d93115e0 0000000000000188 ffff8801d9311628 ffff8800cba17588
Call Trace:
[<ffffffff8191447e>] dump_stack+0xdc/0x15e
[<ffffffff819143a2>] ? _atomic_dec_and_lock+0xa2/0xa2
[<ffffffff814e2cd1>] ? print_section+0x61/0xb0
[<ffffffff814e4939>] print_trailer+0x179/0x2c0
[<ffffffff814f0d84>] object_err+0x34/0x40
[<ffffffff814f4388>] kasan_report_error+0x2f8/0x8b0
[<ffffffff814eb91e>] ? __slab_alloc+0x5e/0x90
[<ffffffff812178c0>] ? __lock_is_held+0x90/0x130
[<ffffffff814f5091>] kasan_report+0x71/0xa0
[<ffffffff814ec082>] ? kmem_cache_alloc_trace+0x212/0x560
[<ffffffff81d99468>] ? xhci_find_raw_port_number+0x98/0x170
[<ffffffff814f33d4>] __asan_load8+0x64/0x70
[<ffffffff81d99468>] xhci_find_raw_port_number+0x98/0x170
[<ffffffff81db0105>] xhci_setup_addressable_virt_dev+0x235/0xa10
[<ffffffff81d9ea51>] xhci_setup_device+0x3c1/0x1430
[<ffffffff8121cddd>] ? trace_hardirqs_on+0xd/0x10
[<ffffffff81d9fac0>] ? xhci_setup_device+0x1430/0x1430
[<ffffffff81d9fad3>] xhci_address_device+0x13/0x20
[<ffffffff81d2081a>] hub_port_init+0x55a/0x1550
[<ffffffff81d28705>] hub_event+0xef5/0x24d0
[<ffffffff81d27810>] ? hub_port_debounce+0x2f0/0x2f0
[<ffffffff8195e1ee>] ? debug_object_deactivate+0x1be/0x270
[<ffffffff81210203>] ? print_rt_rq+0x53/0x2d0
[<ffffffff8121657d>] ? trace_hardirqs_off+0xd/0x10
[<ffffffff8226acfb>] ? _raw_spin_unlock_irqrestore+0x5b/0x60
[<ffffffff81250000>] ? irq_domain_set_hwirq_and_chip+0x30/0xb0
[<ffffffff81256339>] ? debug_lockdep_rcu_enabled+0x39/0x40
[<ffffffff812178c0>] ? __lock_is_held+0x90/0x130
[<ffffffff81196877>] process_one_work+0x567/0xec0
[...]Afterwards, xhci reports some functional errors:
xhci_hcd 0000:00:14.0: ERROR: unexpected setup address command completion
code 0x11.
xhci_hcd 0000:00:14.0: ERROR: unexpected setup address command completion
code 0x11.usb 4-3: device not accepting address 2, error -22
Fix this by not overwriting the port1 argument in usb_alloc_dev(), but storing the raw port number as required by OF in an additional variable, raw_port.
---
Applicable to linux-next-20160317
Changes to v1:
- Initialize raw_port with port1
drivers/usb/core/usb.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/usb/core/usb.c +3 -2
@@ -424,6 +424,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,struct usb_device *dev;struct usb_hcd *usb_hcd =bus_to_hcd (bus);unsigned root_hub =0 ; +unsigned raw_port = port1; dev =kzalloc (sizeof (*dev),GFP_KERNEL );if (!dev)
@@ -498,11 +499,11 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,if (!parent->parent) {/* device under root hub's port */ - port1 =usb_hcd_find_raw_port_number (usb_hcd, + raw_port =usb_hcd_find_raw_port_number (usb_hcd, port1); } dev->dev.of_node =usb_of_get_child_node (parent->dev.of_node, - port1); + raw_port);/* hub driver sets up TT records */ }
Signature · 1 line
2.7.2
Quoted 18 lines · Nicolai Stange
With commit
69bec7259853("USB: core: let USB device know device node"), the port1 argument ofusb_alloc_dev()gets overwritten as follows:... usb_alloc_dev(..., unsigned port1) { ... if (!parent->parent) { port1 = usb_hcd_find_raw_port_number(..., port1); } ... }Later on, this now overwritten port1 gets assigned to ->portnum:
dev->portnum = port1;However, since
xhci_find_raw_port_number()isn't idempotent, the aforementioned commit causes a number of KASAN splats like the following:
...
Quoted 11 lines
Fix this by not overwriting the port1 argument in
usb_alloc_dev(), but storing the raw port number as required by OF in an additional variable,raw_port.Fixes 69bec7259853 ("USB: core: let USB device know device node")Signed-off-by Nicolai Stange <[email protected]>---
Applicable to linux-next-20160317Changes to v1:
- Initializeraw_portwith port1
Quoted 32 lines · Alan Stern
Quoted 18 lines · Nicolai Stange
With commit
69bec7259853("USB: core: let USB device know device node"), the port1 argument ofusb_alloc_dev()gets overwritten as follows:... usb_alloc_dev(..., unsigned port1) { ... if (!parent->parent) { port1 = usb_hcd_find_raw_port_number(..., port1); } ... }Later on, this now overwritten port1 gets assigned to ->portnum:
dev->portnum = port1;However, since
xhci_find_raw_port_number()isn't idempotent, the aforementioned commit causes a number of KASAN splats like the following:...
Quoted 9 lines
Fix this by not overwriting the port1 argument in
usb_alloc_dev(), but storing the raw port number as required by OF in an additional variable,raw_port.Fixes 69bec7259853 ("USB: core: let USB device know device node")Signed-off-by Nicolai Stange <[email protected]>---
Applicable to linux-next-20160317
Quoted 5 lines
Quoted 2 lines
Changes to v1:
- Initializeraw_portwith port1Acked-by Alan Stern <[email protected]>
Thanks for fixing it in time, you are right. My patch would let the device logical port number equals to device raw port number, but it is wrong for xhci.
Signature · 2 lines
Best Regards, Peter Chen