From 6377f9bf2cd131668569acaaff54b94d4b12e810 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 12 Apr 2021 09:23:13 +0200 Subject: [PATCH] 4.14-stable patches added patches: usbip-fix-vudc-usbip_sockfd_store-races-leading-to-gpf.patch --- queue-4.14/series | 1 + ...ip_sockfd_store-races-leading-to-gpf.patch | 154 ++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 queue-4.14/usbip-fix-vudc-usbip_sockfd_store-races-leading-to-gpf.patch diff --git a/queue-4.14/series b/queue-4.14/series index be5e110ff64..af26c6c5dee 100644 --- a/queue-4.14/series +++ b/queue-4.14/series @@ -42,3 +42,4 @@ net-ncsi-don-t-return-error-on-normal-response.patch net-ncsi-add-generic-netlink-family.patch net-ncsi-refactor-mac-vlan-filters.patch net-ncsi-avoid-gfp_kernel-in-response-handler.patch +usbip-fix-vudc-usbip_sockfd_store-races-leading-to-gpf.patch diff --git a/queue-4.14/usbip-fix-vudc-usbip_sockfd_store-races-leading-to-gpf.patch b/queue-4.14/usbip-fix-vudc-usbip_sockfd_store-races-leading-to-gpf.patch new file mode 100644 index 00000000000..c3698913e54 --- /dev/null +++ b/queue-4.14/usbip-fix-vudc-usbip_sockfd_store-races-leading-to-gpf.patch @@ -0,0 +1,154 @@ +From 46613c9dfa964c0c60b5385dbdf5aaa18be52a9c Mon Sep 17 00:00:00 2001 +From: Shuah Khan +Date: Sun, 7 Mar 2021 20:53:31 -0700 +Subject: usbip: fix vudc usbip_sockfd_store races leading to gpf + +From: Shuah Khan + +commit 46613c9dfa964c0c60b5385dbdf5aaa18be52a9c upstream. + +usbip_sockfd_store() is invoked when user requests attach (import) +detach (unimport) usb gadget device from usbip host. vhci_hcd sends +import request and usbip_sockfd_store() exports the device if it is +free for export. + +Export and unexport are governed by local state and shared state +- Shared state (usbip device status, sockfd) - sockfd and Device + status are used to determine if stub should be brought up or shut + down. Device status is shared between host and client. +- Local state (tcp_socket, rx and tx thread task_struct ptrs) + A valid tcp_socket controls rx and tx thread operations while the + device is in exported state. +- While the device is exported, device status is marked used and socket, + sockfd, and thread pointers are valid. + +Export sequence (stub-up) includes validating the socket and creating +receive (rx) and transmit (tx) threads to talk to the client to provide +access to the exported device. rx and tx threads depends on local and +shared state to be correct and in sync. + +Unexport (stub-down) sequence shuts the socket down and stops the rx and +tx threads. Stub-down sequence relies on local and shared states to be +in sync. + +There are races in updating the local and shared status in the current +stub-up sequence resulting in crashes. These stem from starting rx and +tx threads before local and global state is updated correctly to be in +sync. + +1. Doesn't handle kthread_create() error and saves invalid ptr in local + state that drives rx and tx threads. +2. Updates tcp_socket and sockfd, starts stub_rx and stub_tx threads + before updating usbip_device status to SDEV_ST_USED. This opens up a + race condition between the threads and usbip_sockfd_store() stub up + and down handling. + +Fix the above problems: +- Stop using kthread_get_run() macro to create/start threads. +- Create threads and get task struct reference. +- Add kthread_create() failure handling and bail out. +- Hold usbip_device lock to update local and shared states after + creating rx and tx threads. +- Update usbip_device status to SDEV_ST_USED. +- Update usbip_device tcp_socket, sockfd, tcp_rx, and tcp_tx +- Start threads after usbip_device (tcp_socket, sockfd, tcp_rx, tcp_tx, + and status) is complete. + +Credit goes to syzbot and Tetsuo Handa for finding and root-causing the +kthread_get_run() improper error handling problem and others. This is a +hard problem to find and debug since the races aren't seen in a normal +case. Fuzzing forces the race window to be small enough for the +kthread_get_run() error path bug and starting threads before updating the +local and shared state bug in the stub-up sequence. + +Fixes: 9720b4bc76a83807 ("staging/usbip: convert to kthread") +Cc: stable@vger.kernel.org +Reported-by: syzbot +Reported-by: syzbot +Reported-by: syzbot +Reported-by: Tetsuo Handa +Signed-off-by: Shuah Khan +Link: https://lore.kernel.org/r/b1c08b983ffa185449c9f0f7d1021dc8c8454b60.1615171203.git.skhan@linuxfoundation.org +Signed-off-by: Tom Seewald +Signed-off-by: Greg Kroah-Hartman +--- + drivers/usb/usbip/vudc_sysfs.c | 42 +++++++++++++++++++++++++++++++++-------- + 1 file changed, 34 insertions(+), 8 deletions(-) + +--- a/drivers/usb/usbip/vudc_sysfs.c ++++ b/drivers/usb/usbip/vudc_sysfs.c +@@ -103,8 +103,9 @@ unlock: + } + static BIN_ATTR_RO(dev_desc, sizeof(struct usb_device_descriptor)); + +-static ssize_t store_sockfd(struct device *dev, struct device_attribute *attr, +- const char *in, size_t count) ++static ssize_t store_sockfd(struct device *dev, ++ struct device_attribute *attr, ++ const char *in, size_t count) + { + struct vudc *udc = (struct vudc *) dev_get_drvdata(dev); + int rv; +@@ -113,6 +114,8 @@ static ssize_t store_sockfd(struct devic + struct socket *socket; + unsigned long flags; + int ret; ++ struct task_struct *tcp_rx = NULL; ++ struct task_struct *tcp_tx = NULL; + + rv = kstrtoint(in, 0, &sockfd); + if (rv != 0) +@@ -158,24 +161,47 @@ static ssize_t store_sockfd(struct devic + goto sock_err; + } + +- udc->ud.tcp_socket = socket; +- ++ /* unlock and create threads and get tasks */ + spin_unlock_irq(&udc->ud.lock); + spin_unlock_irqrestore(&udc->lock, flags); + +- udc->ud.tcp_rx = kthread_get_run(&v_rx_loop, +- &udc->ud, "vudc_rx"); +- udc->ud.tcp_tx = kthread_get_run(&v_tx_loop, +- &udc->ud, "vudc_tx"); ++ tcp_rx = kthread_create(&v_rx_loop, &udc->ud, "vudc_rx"); ++ if (IS_ERR(tcp_rx)) { ++ sockfd_put(socket); ++ return -EINVAL; ++ } ++ tcp_tx = kthread_create(&v_tx_loop, &udc->ud, "vudc_tx"); ++ if (IS_ERR(tcp_tx)) { ++ kthread_stop(tcp_rx); ++ sockfd_put(socket); ++ return -EINVAL; ++ } ++ ++ /* get task structs now */ ++ get_task_struct(tcp_rx); ++ get_task_struct(tcp_tx); + ++ /* lock and update udc->ud state */ + spin_lock_irqsave(&udc->lock, flags); + spin_lock_irq(&udc->ud.lock); ++ ++ udc->ud.tcp_socket = socket; ++ udc->ud.tcp_rx = tcp_rx; ++ udc->ud.tcp_rx = tcp_tx; + udc->ud.status = SDEV_ST_USED; ++ + spin_unlock_irq(&udc->ud.lock); + + do_gettimeofday(&udc->start_time); + v_start_timer(udc); + udc->connected = 1; ++ ++ spin_unlock_irqrestore(&udc->lock, flags); ++ ++ wake_up_process(udc->ud.tcp_rx); ++ wake_up_process(udc->ud.tcp_tx); ++ return count; ++ + } else { + if (!udc->connected) { + dev_err(dev, "Device not connected"); -- 2.47.3