linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org,
	syzbot <syzbot+a93fba6d384346a761e3@syzkaller.appspotmail.com>,
	syzbot <syzbot+bf1a360e305ee719e364@syzkaller.appspotmail.com>,
	syzbot <syzbot+95ce4b142579611ef0a9@syzkaller.appspotmail.com>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Tom Seewald <tseewald@gmail.com>
Subject: [PATCH 4.14 45/68] usbip: fix vudc usbip_sockfd_store races leading to gpf
Date: Thu, 15 Apr 2021 16:47:26 +0200	[thread overview]
Message-ID: <20210415144415.946778464@linuxfoundation.org> (raw)
In-Reply-To: <20210415144414.464797272@linuxfoundation.org>

From: Shuah Khan <skhan@linuxfoundation.org>

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 <syzbot+a93fba6d384346a761e3@syzkaller.appspotmail.com>
Reported-by: syzbot <syzbot+bf1a360e305ee719e364@syzkaller.appspotmail.com>
Reported-by: syzbot <syzbot+95ce4b142579611ef0a9@syzkaller.appspotmail.com>
Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Link: https://lore.kernel.org/r/b1c08b983ffa185449c9f0f7d1021dc8c8454b60.1615171203.git.skhan@linuxfoundation.org
Signed-off-by: Tom Seewald <tseewald@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 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");



  parent reply	other threads:[~2021-04-15 15:05 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-15 14:46 [PATCH 4.14 00/68] 4.14.231-rc1 review Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 01/68] ALSA: aloop: Fix initialization of controls Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 02/68] ASoC: intel: atom: Stop advertising non working S24LE support Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 03/68] nfc: fix refcount leak in llcp_sock_bind() Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 04/68] nfc: fix refcount leak in llcp_sock_connect() Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 05/68] nfc: fix memory " Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 06/68] nfc: Avoid endless loops caused by repeated llcp_sock_connect() Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 07/68] xen/evtchn: Change irq_info lock to raw_spinlock_t Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 08/68] net: ipv6: check for validity before dereferencing cfg->fc_nlinfo.nlh Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 09/68] ia64: fix user_stack_pointer() for ptrace() Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 10/68] ocfs2: fix deadlock between setattr and dio_end_io_write Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 11/68] fs: direct-io: fix missing sdio->boundary Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 12/68] parisc: parisc-agp requires SBA IOMMU driver Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 13/68] parisc: avoid a warning on u8 cast for cmpxchg on u8 pointers Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 14/68] ARM: dts: turris-omnia: configure LED[2]/INTn pin as interrupt pin Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 15/68] batman-adv: initialize "struct batadv_tvlv_tt_vlan_data"->reserved field Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 16/68] net: ensure mac header is set in virtio_net_hdr_to_skb() Greg Kroah-Hartman
2021-04-16  8:49   ` Balazs Nemeth
2021-04-16  9:32     ` Eric Dumazet
2021-04-29  9:01       ` Balazs Nemeth
2021-04-15 14:46 ` [PATCH 4.14 17/68] net: sched: sch_teql: fix null-pointer dereference Greg Kroah-Hartman
2021-04-15 14:46 ` [PATCH 4.14 18/68] usbip: add sysfs_lock to synchronize sysfs code paths Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 19/68] usbip: stub-dev " Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 20/68] usbip: synchronize event handler with " Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 21/68] i2c: turn recovery error on init to debug Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 22/68] regulator: bd9571mwv: Fix AVS and DVFS voltage range Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 23/68] ASoC: wm8960: Fix wrong bclk and lrclk with pll enabled for some chips Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 24/68] amd-xgbe: Update DMA coherency values Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 25/68] sch_red: fix off-by-one checks in red_check_params() Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 26/68] gianfar: Handle error code at MAC address change Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 27/68] net:tipc: Fix a double free in tipc_sk_mcast_rcv Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 28/68] ARM: dts: imx6: pbab01: Set vmmc supply for both SD interfaces Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 29/68] net/ncsi: Avoid channel_monitor hrtimer deadlock Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 30/68] ASoC: sunxi: sun4i-codec: fill ASoC card owner Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 31/68] soc/fsl: qbman: fix conflicting alignment attributes Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 32/68] clk: fix invalid usage of list cursor in register Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 33/68] clk: fix invalid usage of list cursor in unregister Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 34/68] workqueue: Move the position of debug_work_activate() in __queue_work() Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 35/68] s390/cpcmd: fix inline assembly register clobbering Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 36/68] net/mlx5: Fix placement of log_max_flow_counter Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 37/68] RDMA/cxgb4: check for ipv6 address properly while destroying listener Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 38/68] clk: socfpga: fix iomem pointer cast on 64-bit Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 39/68] net/ncsi: Make local function ncsi_get_filter() static Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 40/68] net/ncsi: Improve general state logging Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 41/68] net/ncsi: Dont return error on normal response Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 42/68] net/ncsi: Add generic netlink family Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 43/68] net/ncsi: Refactor MAC, VLAN filters Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 44/68] net/ncsi: Avoid GFP_KERNEL in response handler Greg Kroah-Hartman
2021-04-15 14:47 ` Greg Kroah-Hartman [this message]
2021-07-28 12:56   ` [PATCH 4.14 45/68] usbip: fix vudc usbip_sockfd_store races leading to gpf Krzysztof Kozlowski
2021-04-15 14:47 ` [PATCH 4.14 46/68] cfg80211: remove WARN_ON() in cfg80211_sme_connect Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 47/68] net: tun: set tun->dev->addr_len during TUNSETLINK processing Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 48/68] drivers: net: fix memory leak in atusb_probe Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 49/68] drivers: net: fix memory leak in peak_usb_create_dev Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 50/68] net: mac802154: Fix general protection fault Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 51/68] net: ieee802154: nl-mac: fix check on panid Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 52/68] net: ieee802154: fix nl802154 del llsec key Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 53/68] net: ieee802154: fix nl802154 del llsec dev Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 54/68] net: ieee802154: fix nl802154 add llsec key Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 55/68] net: ieee802154: fix nl802154 del llsec devkey Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 56/68] net: ieee802154: forbid monitor for set llsec params Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 57/68] net: ieee802154: forbid monitor for del llsec seclevel Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 58/68] net: ieee802154: stop dump llsec params for monitors Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 59/68] Revert "cifs: Set CIFS_MOUNT_USE_PREFIX_PATH flag on setting cifs_sb->prepath." Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 60/68] KVM: arm64: Hide system instruction access to Trace registers Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 61/68] KVM: arm64: Disable guest access to trace filter controls Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 62/68] drm/imx: imx-ldb: fix out of bounds array access warning Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 63/68] gfs2: report "already frozen/thawed" errors Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 64/68] block: only update parent bi_status when bio fail Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 65/68] net: phy: broadcom: Only advertise EEE for supported modes Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 66/68] netfilter: x_tables: fix compat match/target pad out-of-bound write Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 67/68] perf map: Tighten snprintf() string precision to pass gcc check on some 32-bit arches Greg Kroah-Hartman
2021-04-15 14:47 ` [PATCH 4.14 68/68] xen/events: fix setting irq affinity Greg Kroah-Hartman
2021-04-15 22:44 ` [PATCH 4.14 00/68] 4.14.231-rc1 review Shuah Khan
2021-04-15 22:45 ` Shuah Khan
2021-04-16 11:33 ` Naresh Kamboju
2021-04-17  1:00 ` Samuel Zou

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210415144415.946778464@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=skhan@linuxfoundation.org \
    --cc=stable@vger.kernel.org \
    --cc=syzbot+95ce4b142579611ef0a9@syzkaller.appspotmail.com \
    --cc=syzbot+a93fba6d384346a761e3@syzkaller.appspotmail.com \
    --cc=syzbot+bf1a360e305ee719e364@syzkaller.appspotmail.com \
    --cc=tseewald@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).