| From 4e9c93af7279b059faf5bb1897ee90512b258a12 Mon Sep 17 00:00:00 2001 |
| From: Shuah Khan <skhan@linuxfoundation.org> |
| Date: Mon, 29 Mar 2021 19:36:48 -0600 |
| Subject: usbip: add sysfs_lock to synchronize sysfs code paths |
| |
| From: Shuah Khan <skhan@linuxfoundation.org> |
| |
| commit 4e9c93af7279b059faf5bb1897ee90512b258a12 upstream. |
| |
| 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> |
| --- |
| drivers/usb/usbip/usbip_common.h | 3 +++ |
| drivers/usb/usbip/vhci_hcd.c | 1 + |
| drivers/usb/usbip/vhci_sysfs.c | 30 +++++++++++++++++++++++++----- |
| 3 files changed, 29 insertions(+), 5 deletions(-) |
| |
| --- a/drivers/usb/usbip/usbip_common.h |
| +++ b/drivers/usb/usbip/usbip_common.h |
| @@ -263,6 +263,9 @@ struct usbip_device { |
| /* lock for status */ |
| spinlock_t lock; |
| |
| + /* mutex for synchronizing sysfs store paths */ |
| + struct mutex sysfs_lock; |
| + |
| int sockfd; |
| struct socket *tcp_socket; |
| |
| --- a/drivers/usb/usbip/vhci_hcd.c |
| +++ b/drivers/usb/usbip/vhci_hcd.c |
| @@ -1096,6 +1096,7 @@ static void vhci_device_init(struct vhci |
| vdev->ud.side = USBIP_VHCI; |
| vdev->ud.status = VDEV_ST_NULL; |
| spin_lock_init(&vdev->ud.lock); |
| + mutex_init(&vdev->ud.sysfs_lock); |
| |
| INIT_LIST_HEAD(&vdev->priv_rx); |
| INIT_LIST_HEAD(&vdev->priv_tx); |
| --- a/drivers/usb/usbip/vhci_sysfs.c |
| +++ b/drivers/usb/usbip/vhci_sysfs.c |
| @@ -185,6 +185,8 @@ static int vhci_port_disconnect(struct v |
| |
| usbip_dbg_vhci_sysfs("enter\n"); |
| |
| + mutex_lock(&vdev->ud.sysfs_lock); |
| + |
| /* lock */ |
| spin_lock_irqsave(&vhci->lock, flags); |
| spin_lock(&vdev->ud.lock); |
| @@ -195,6 +197,7 @@ static int vhci_port_disconnect(struct v |
| /* unlock */ |
| spin_unlock(&vdev->ud.lock); |
| spin_unlock_irqrestore(&vhci->lock, flags); |
| + mutex_unlock(&vdev->ud.sysfs_lock); |
| |
| return -EINVAL; |
| } |
| @@ -205,6 +208,8 @@ static int vhci_port_disconnect(struct v |
| |
| usbip_event_add(&vdev->ud, VDEV_EVENT_DOWN); |
| |
| + mutex_unlock(&vdev->ud.sysfs_lock); |
| + |
| return 0; |
| } |
| |
| @@ -349,30 +354,36 @@ static ssize_t attach_store(struct devic |
| else |
| vdev = &vhci->vhci_hcd_hs->vdev[rhport]; |
| |
| + mutex_lock(&vdev->ud.sysfs_lock); |
| + |
| /* Extract socket from fd. */ |
| socket = sockfd_lookup(sockfd, &err); |
| if (!socket) { |
| dev_err(dev, "failed to lookup sock"); |
| - return -EINVAL; |
| + err = -EINVAL; |
| + goto unlock_mutex; |
| } |
| if (socket->type != SOCK_STREAM) { |
| dev_err(dev, "Expecting SOCK_STREAM - found %d", |
| socket->type); |
| sockfd_put(socket); |
| - return -EINVAL; |
| + err = -EINVAL; |
| + goto unlock_mutex; |
| } |
| |
| /* create threads before locking */ |
| tcp_rx = kthread_create(vhci_rx_loop, &vdev->ud, "vhci_rx"); |
| if (IS_ERR(tcp_rx)) { |
| sockfd_put(socket); |
| - return -EINVAL; |
| + err = -EINVAL; |
| + goto unlock_mutex; |
| } |
| tcp_tx = kthread_create(vhci_tx_loop, &vdev->ud, "vhci_tx"); |
| if (IS_ERR(tcp_tx)) { |
| kthread_stop(tcp_rx); |
| sockfd_put(socket); |
| - return -EINVAL; |
| + err = -EINVAL; |
| + goto unlock_mutex; |
| } |
| |
| /* get task structs now */ |
| @@ -397,7 +408,8 @@ static ssize_t attach_store(struct devic |
| * Will be retried from userspace |
| * if there's another free port. |
| */ |
| - return -EBUSY; |
| + err = -EBUSY; |
| + goto unlock_mutex; |
| } |
| |
| dev_info(dev, "pdev(%u) rhport(%u) sockfd(%d)\n", |
| @@ -422,7 +434,15 @@ static ssize_t attach_store(struct devic |
| |
| rh_port_connect(vdev, speed); |
| |
| + dev_info(dev, "Device attached\n"); |
| + |
| + mutex_unlock(&vdev->ud.sysfs_lock); |
| + |
| return count; |
| + |
| +unlock_mutex: |
| + mutex_unlock(&vdev->ud.sysfs_lock); |
| + return err; |
| } |
| static DEVICE_ATTR_WO(attach); |
| |