Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

usbip: add sysfs_lock to synchronize sysfs code paths

Fuzzing uncovered race condition between sysfs code paths in usbip
drivers. Device connect/disconnect code paths initiated through
sysfs interface are prone to races if disconnect happens during
connect and vice versa.

This problem is common to all drivers while it can be reproduced easily
in vhci_hcd. Add a sysfs_lock to usbip_device struct to protect the paths.

Use this in vhci_hcd to protect sysfs paths. For a complete fix, usip_host
and usip-vudc drivers and the event handler will have to use this lock to
protect the paths. These changes will be done in subsequent patches.

Cc: stable@vger.kernel.org
Reported-and-tested-by: syzbot+a93fba6d384346a761e3@syzkaller.appspotmail.com
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Link: https://lore.kernel.org/r/b6568f7beae702bbc236a545d3c020106ca75eac.1616807117.git.skhan@linuxfoundation.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Shuah Khan and committed by
Greg Kroah-Hartman
4e9c93af 3004fcba

+29 -5
+3
drivers/usb/usbip/usbip_common.h
··· 263 263 /* lock for status */ 264 264 spinlock_t lock; 265 265 266 + /* mutex for synchronizing sysfs store paths */ 267 + struct mutex sysfs_lock; 268 + 266 269 int sockfd; 267 270 struct socket *tcp_socket; 268 271
+1
drivers/usb/usbip/vhci_hcd.c
··· 1101 1101 vdev->ud.side = USBIP_VHCI; 1102 1102 vdev->ud.status = VDEV_ST_NULL; 1103 1103 spin_lock_init(&vdev->ud.lock); 1104 + mutex_init(&vdev->ud.sysfs_lock); 1104 1105 1105 1106 INIT_LIST_HEAD(&vdev->priv_rx); 1106 1107 INIT_LIST_HEAD(&vdev->priv_tx);
+25 -5
drivers/usb/usbip/vhci_sysfs.c
··· 185 185 186 186 usbip_dbg_vhci_sysfs("enter\n"); 187 187 188 + mutex_lock(&vdev->ud.sysfs_lock); 189 + 188 190 /* lock */ 189 191 spin_lock_irqsave(&vhci->lock, flags); 190 192 spin_lock(&vdev->ud.lock); ··· 197 195 /* unlock */ 198 196 spin_unlock(&vdev->ud.lock); 199 197 spin_unlock_irqrestore(&vhci->lock, flags); 198 + mutex_unlock(&vdev->ud.sysfs_lock); 200 199 201 200 return -EINVAL; 202 201 } ··· 207 204 spin_unlock_irqrestore(&vhci->lock, flags); 208 205 209 206 usbip_event_add(&vdev->ud, VDEV_EVENT_DOWN); 207 + 208 + mutex_unlock(&vdev->ud.sysfs_lock); 210 209 211 210 return 0; 212 211 } ··· 354 349 else 355 350 vdev = &vhci->vhci_hcd_hs->vdev[rhport]; 356 351 352 + mutex_lock(&vdev->ud.sysfs_lock); 353 + 357 354 /* Extract socket from fd. */ 358 355 socket = sockfd_lookup(sockfd, &err); 359 356 if (!socket) { 360 357 dev_err(dev, "failed to lookup sock"); 361 - return -EINVAL; 358 + err = -EINVAL; 359 + goto unlock_mutex; 362 360 } 363 361 if (socket->type != SOCK_STREAM) { 364 362 dev_err(dev, "Expecting SOCK_STREAM - found %d", 365 363 socket->type); 366 364 sockfd_put(socket); 367 - return -EINVAL; 365 + err = -EINVAL; 366 + goto unlock_mutex; 368 367 } 369 368 370 369 /* create threads before locking */ 371 370 tcp_rx = kthread_create(vhci_rx_loop, &vdev->ud, "vhci_rx"); 372 371 if (IS_ERR(tcp_rx)) { 373 372 sockfd_put(socket); 374 - return -EINVAL; 373 + err = -EINVAL; 374 + goto unlock_mutex; 375 375 } 376 376 tcp_tx = kthread_create(vhci_tx_loop, &vdev->ud, "vhci_tx"); 377 377 if (IS_ERR(tcp_tx)) { 378 378 kthread_stop(tcp_rx); 379 379 sockfd_put(socket); 380 - return -EINVAL; 380 + err = -EINVAL; 381 + goto unlock_mutex; 381 382 } 382 383 383 384 /* get task structs now */ ··· 408 397 * Will be retried from userspace 409 398 * if there's another free port. 410 399 */ 411 - return -EBUSY; 400 + err = -EBUSY; 401 + goto unlock_mutex; 412 402 } 413 403 414 404 dev_info(dev, "pdev(%u) rhport(%u) sockfd(%d)\n", ··· 435 423 436 424 rh_port_connect(vdev, speed); 437 425 426 + dev_info(dev, "Device attached\n"); 427 + 428 + mutex_unlock(&vdev->ud.sysfs_lock); 429 + 438 430 return count; 431 + 432 + unlock_mutex: 433 + mutex_unlock(&vdev->ud.sysfs_lock); 434 + return err; 439 435 } 440 436 static DEVICE_ATTR_WO(attach); 441 437