Commit graph

12 commits

Author SHA1 Message Date
Shuah Khan
061e0399b5 usbip: usbip_host: fix stub_dev lock context imbalance regression
commit 3ea3091f1bd8586125848c62be295910e9802af0 upstream.

Fix the following sparse context imbalance regression introduced in
a patch that fixed sleeping function called from invalid context bug.

kbuild test robot reported on:

tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git  usb-linus

Regressions in current branch:

drivers/usb/usbip/stub_dev.c:399:9: sparse: sparse: context imbalance in 'stub_probe' - different lock contexts for basic block
drivers/usb/usbip/stub_dev.c:418:13: sparse: sparse: context imbalance in 'stub_disconnect' - different lock contexts for basic block
drivers/usb/usbip/stub_dev.c:464:1-10: second lock on line 476

Error ids grouped by kconfigs:

recent_errors
├── i386-allmodconfig
│   └── drivers-usb-usbip-stub_dev.c:second-lock-on-line
├── x86_64-allmodconfig
│   ├── drivers-usb-usbip-stub_dev.c:sparse:sparse:context-imbalance-in-stub_disconnect-different-lock-contexts-for-basic-block
│   └── drivers-usb-usbip-stub_dev.c:sparse:sparse:context-imbalance-in-stub_probe-different-lock-contexts-for-basic-block
└── x86_64-allyesconfig
    └── drivers-usb-usbip-stub_dev.c:second-lock-on-line

This is a real problem in an error leg where spin_lock() is called on an
already held lock.

Fix the imbalance in stub_probe() and stub_disconnect().

Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Fixes: 0c9e8b3cad65 ("usbip: usbip_host: fix BUG: sleeping function called from invalid context")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-06-11 12:22:37 +02:00
Shuah Khan
450273e65d usbip: usbip_host: fix BUG: sleeping function called from invalid context
commit 0c9e8b3cad654bfc499c10b652fbf8f0b890af8f upstream.

stub_probe() and stub_disconnect() call functions which could call
sleeping function in invalid context whil holding busid_lock.

Fix the problem by refining the lock holds to short critical sections
to change the busid_priv fields. This fix restructures the code to
limit the lock holds in stub_probe() and stub_disconnect().

stub_probe():

[15217.927028] BUG: sleeping function called from invalid context at mm/slab.h:418
[15217.927038] in_atomic(): 1, irqs_disabled(): 0, pid: 29087, name: usbip
[15217.927044] 5 locks held by usbip/29087:
[15217.927047]  #0: 0000000091647f28 (sb_writers#6){....}, at: vfs_write+0x191/0x1c0
[15217.927062]  #1: 000000008f9ba75b (&of->mutex){....}, at: kernfs_fop_write+0xf7/0x1b0
[15217.927072]  #2: 00000000872e5b4b (&dev->mutex){....}, at: __device_driver_lock+0x3b/0x50
[15217.927082]  #3: 00000000e74ececc (&dev->mutex){....}, at: __device_driver_lock+0x46/0x50
[15217.927090]  #4: 00000000b20abbe0 (&(&busid_table[i].busid_lock)->rlock){....}, at: get_busid_priv+0x48/0x60 [usbip_host]
[15217.927103] CPU: 3 PID: 29087 Comm: usbip Tainted: G        W         5.1.0-rc6+ #40
[15217.927106] Hardware name: Dell Inc. OptiPlex 790/0HY9JP, BIOS A18 09/24/2013
[15217.927109] Call Trace:
[15217.927118]  dump_stack+0x63/0x85
[15217.927127]  ___might_sleep+0xff/0x120
[15217.927133]  __might_sleep+0x4a/0x80
[15217.927143]  kmem_cache_alloc_trace+0x1aa/0x210
[15217.927156]  stub_probe+0xe8/0x440 [usbip_host]
[15217.927171]  usb_probe_device+0x34/0x70

stub_disconnect():

[15279.182478] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:908
[15279.182487] in_atomic(): 1, irqs_disabled(): 0, pid: 29114, name: usbip
[15279.182492] 5 locks held by usbip/29114:
[15279.182494]  #0: 0000000091647f28 (sb_writers#6){....}, at: vfs_write+0x191/0x1c0
[15279.182506]  #1: 00000000702cf0f3 (&of->mutex){....}, at: kernfs_fop_write+0xf7/0x1b0
[15279.182514]  #2: 00000000872e5b4b (&dev->mutex){....}, at: __device_driver_lock+0x3b/0x50
[15279.182522]  #3: 00000000e74ececc (&dev->mutex){....}, at: __device_driver_lock+0x46/0x50
[15279.182529]  #4: 00000000b20abbe0 (&(&busid_table[i].busid_lock)->rlock){....}, at: get_busid_priv+0x48/0x60 [usbip_host]
[15279.182541] CPU: 0 PID: 29114 Comm: usbip Tainted: G        W         5.1.0-rc6+ #40
[15279.182543] Hardware name: Dell Inc. OptiPlex 790/0HY9JP, BIOS A18 09/24/2013
[15279.182546] Call Trace:
[15279.182554]  dump_stack+0x63/0x85
[15279.182561]  ___might_sleep+0xff/0x120
[15279.182566]  __might_sleep+0x4a/0x80
[15279.182574]  __mutex_lock+0x55/0x950
[15279.182582]  ? get_busid_priv+0x48/0x60 [usbip_host]
[15279.182587]  ? reacquire_held_locks+0xec/0x1a0
[15279.182591]  ? get_busid_priv+0x48/0x60 [usbip_host]
[15279.182597]  ? find_held_lock+0x94/0xa0
[15279.182609]  mutex_lock_nested+0x1b/0x20
[15279.182614]  ? mutex_lock_nested+0x1b/0x20
[15279.182618]  kernfs_remove_by_name_ns+0x2a/0x90
[15279.182625]  sysfs_remove_file_ns+0x15/0x20
[15279.182629]  device_remove_file+0x19/0x20
[15279.182634]  stub_disconnect+0x6d/0x180 [usbip_host]
[15279.182643]  usb_unbind_device+0x27/0x60

Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-06-11 12:22:37 +02:00
Shuah Khan (Samsung OSG)
f2a6d5f194 usbip: usbip_host: fix NULL-ptr deref and use-after-free errors
commit 22076557b07c12086eeb16b8ce2b0b735f7a27e7 upstream.

usbip_host updates device status without holding lock from stub probe,
disconnect and rebind code paths. When multiple requests to import a
device are received, these unprotected code paths step all over each
other and drive fails with NULL-ptr deref and use-after-free errors.

The driver uses a table lock to protect the busid array for adding and
deleting busids to the table. However, the probe, disconnect and rebind
paths get the busid table entry and update the status without holding
the busid table lock. Add a new finer grain lock to protect the busid
entry. This new lock will be held to search and update the busid entry
fields from get_busid_idx(), add_match_busid() and del_match_busid().

match_busid_show() does the same to access the busid entry fields.

get_busid_priv() changed to return the pointer to the busid entry holding
the busid lock. stub_probe(), stub_disconnect() and stub_device_rebind()
call put_busid_priv() to release the busid lock before returning. This
changes fixes the unprotected code paths eliminating the race conditions
in updating the busid entries.

Reported-by: Jakub Jirasek
Signed-off-by: Shuah Khan (Samsung OSG) <shuah@kernel.org>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-05-22 16:57:56 +02:00
Shuah Khan (Samsung OSG)
59ad4f5342 usbip: usbip_host: run rebind from exit when module is removed
commit 7510df3f29d44685bab7b1918b61a8ccd57126a9 upstream.

After removing usbip_host module, devices it releases are left without
a driver. For example, when a keyboard or a mass storage device are
bound to usbip_host when it is removed, these devices are no longer
bound to any driver.

Fix it to run device_attach() from the module exit routine to restore
the devices to their original drivers. This includes cleanup changes
and moving device_attach() code to a common routine to be called from
rebind_store() and usbip_host_exit().

Signed-off-by: Shuah Khan (Samsung OSG) <shuah@kernel.org>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-05-22 16:57:55 +02:00
Shuah Khan
dbab65be6b usbip: usbip_host: refine probe and disconnect debug msgs to be useful
commit 28b68acc4a88dcf91fd1dcf2577371dc9bf574cc upstream.

Refine probe and disconnect debug msgs to be useful and say what is
in progress.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-05-22 16:57:55 +02:00
Shuah Khan
8690825ea6 usbip: keep usbip_device sockfd state in sync with tcp_socket
commit 009f41aed4b3e11e6dc1e3c07377a10c20f1a5ed upstream.

Keep usbip_device sockfd state in sync with tcp_socket. When tcp_socket
is reset to null, reset sockfd to -1 to keep it in sync.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
Cc: stable@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-02-25 11:05:45 +01:00
Shuah Khan
1ef5c433b3 usbip: prevent leaking socket pointer address in messages
commit 90120d15f4c397272aaf41077960a157fc4212bf upstream.

usbip driver is leaking socket pointer address in messages. Remove
the messages that aren't useful and print sockfd in the ones that
are useful for debugging.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-01-02 20:35:15 +01:00
Alexander Popov
8c7003a3b4 usbip: fix NULL pointer dereference on errors
Fix NULL pointer dereference and obsolete comments forgotten when
usbip server was converted from an interface driver to a device driver.

Signed-off-by: Alexander Popov <alpopov@ptsecurity.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2016-04-28 12:28:08 -07:00
Nobuo Iwata
bb7871ad99 usbip: event handler as one thread
Dear all,

1. Overview

In current USB/IP implementation, event kernel threads are created for
each port. The functions of the threads are closing connection and
error handling so they don't have not so many events to handle. There's
no need to have thread for each port.

BEFORE) vhci side - VHCI_NPORTS(8) threads are created.
$ ps aux | grep usbip
root     10059  0.0  0.0      0     0 ?        S    17:06   0:00 [usbip_eh]
root     10060  0.0  0.0      0     0 ?        S    17:06   0:00 [usbip_eh]
root     10061  0.0  0.0      0     0 ?        S    17:06   0:00 [usbip_eh]
root     10062  0.0  0.0      0     0 ?        S    17:06   0:00 [usbip_eh]
root     10063  0.0  0.0      0     0 ?        S    17:06   0:00 [usbip_eh]
root     10064  0.0  0.0      0     0 ?        S    17:06   0:00 [usbip_eh]
root     10065  0.0  0.0      0     0 ?        S    17:06   0:00 [usbip_eh]
root     10066  0.0  0.0      0     0 ?        S    17:06   0:00 [usbip_eh]

BEFORE) stub side - threads will be created every bind operation.
$ ps aux | grep usbip
root      8368  0.0  0.0      0     0 ?        S    17:56   0:00 [usbip_eh]
root      8399  0.0  0.0      0     0 ?        S    17:56   0:00 [usbip_eh]

This patch put event threads of stub and vhci driver as one workqueue.

AFTER) only one event threads in each vhci and stub side.
$ ps aux | grep usbip
root     10457  0.0  0.0      0     0 ?        S<   17:47   0:00 [usbip_event]

2. Modification to usbip_event.c

BEFORE) kernel threads are created in usbip_start_eh().

AFTER) one workqueue is created in new usbip_init_eh().

Event handler which was main loop of kernel thread is modified to
workqueue handler.

Events themselves are stored in struct usbip_device - same as before.
usbip_devices which have event are listed in event_list.

The handler picks an element from the list and wakeup usbip_device. The
wakeup method is same as before.

usbip_in_eh() substitutes statement which checks whether functions are
called from eh_ops or not. In this function, the worker context is used
for the checking. The context will be set in a variable in the
beginning of first event handling. usbip_in_eh() is used in event
handler so it works well.

3. Modifications to programs using usbip_event.c

Initialization and termination of workqueue are added to init and exit
routine of usbip_core respectively.

A. version info

v2)
# Merged 1/2 event handler itself and 2/2 user programs because of auto
build fail at 1/2 casued unmodified user programs in 1/2.

Signed-off-by: Nobuo Iwata <nobuo.iwata@fujixerox.co.jp>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2016-04-19 04:33:15 +09:00
Alexey Khoroshilov
3ff6744575 usbip: fix error handling in stub_probe()
If usb_hub_claim_port() fails, no resources are deallocated and
if stub_add_files() fails, port is not released.

The patch fixes these issues and rearranges error handling code.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Acked-by: Valentina Manea <valentina.manea.m@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2014-12-02 16:15:02 -08:00
Valentina Manea
3f653c5639 usbip: remove struct usb_device_id table
This was used back when usbip-host was an interface device driver;
after the conversion to device driver, the table remained unused.
Remove it in order to stop receiving a warning about it.

Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2014-08-25 10:40:58 -07:00
Valentina Manea
96c2737716 usbip: move usbip kernel code out of staging
At this point, USB/IP kernel code is fully functional
and can be moved out of staging.

Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2014-08-25 10:40:06 -07:00
Renamed from drivers/staging/usbip/stub_dev.c (Browse further)