Received: by 2002:a05:6a10:9848:0:0:0:0 with SMTP id x8csp3524977pxf; Mon, 15 Mar 2021 11:29:24 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyoojFCalFayhQwNPrkCr7wdmZTkOxQ0mCZs/HX3LcZrElAhpwMUHl2WxOVnLIEeVoPbF2s X-Received: by 2002:a17:906:eb4e:: with SMTP id mc14mr24601678ejb.169.1615832964252; Mon, 15 Mar 2021 11:29:24 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1615832964; cv=none; d=google.com; s=arc-20160816; b=iUhklzWb3OjjnU4KtGDAFhEGOPn5j7MoMKVgrlfoNHEjxnzrI2dO0d1TreDfH87sIZ a2qe4DU3nt3vKBfCQshvpI8r1lKsl5Nd7GbyPmzRjrDHitJH2zhtzr6pfDA0KYwQfLrd 2vTKXB7eyU331r9ysm2eNNXcLQPQLC+KdHSAcbHQB1QN6NtBgpil5UF5rc8brsEmE5px 3vxCeh0v9riaRNIn2KpU740gmQBVzkczt0ZM2CzlhLalaouCeVdQzwXIm5A8nR15sVTE oQCG+f9vsPpvtbalXOvUXTLU2ez7Q6NRcq813PCFjtqa2dH5XMc9PtlTyjmnxpcnvOH4 /Shw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=HaqtNbr8EfRgIXjnzHSRN9kRr256qN7PEdmJtT75BA4=; b=liEk84OUeCpzDAE4x9wxIC56pL/PMN31ILqKcMZfeffclh/j5vHYn1BaTzX9+d7zDb TTIvvBc6jPL7mfVMc15R+M9KddLR5q4xTv4BQLT2Uo2rXahLgNvgRP9Gt82tA1lsk+RP x8L63i6cXVR6qNQiRBfGcOqNRXp1xJ8zw/9AHWgJYcYVUkfgQpkqROaVv40Kv+AaW8yg m9IMdJmsu3XQf2FrKt0N+ju3NF1Iku96j6PubdW7Y97fRI3+xQt/vwZamuBlnvdRHiS/ pfwSLSD6+T+mmjlz+lmXLJLamu0XU1wppIFSl3jzNRAUOs5lj+31iwobx13Rv5Vj1jV2 bM2Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=lz0tAfCn; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id z16si11607433edc.65.2021.03.15.11.28.56; Mon, 15 Mar 2021 11:29:24 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=lz0tAfCn; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240053AbhCOOd0 (ORCPT + 99 others); Mon, 15 Mar 2021 10:33:26 -0400 Received: from mail.kernel.org ([198.145.29.99]:38284 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233135AbhCOOAp (ORCPT ); Mon, 15 Mar 2021 10:00:45 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id B3CF464F46; Mon, 15 Mar 2021 14:00:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1615816815; bh=y9MWJwAHKI1yTKJrTrQ3s9kibterPfHI/FArkPmAavI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lz0tAfCnb9XxMWdu+sJFWHsv2dT/1cHaYfOMF0tv0ETEVYhMF59afE5k2jkNqAaz5 JAPpchYh1n6XGQsDa7sOcBVqc7JO3qiEMLyMcSOLysY/n7wv9vc7/bxkeODIVaMTDW 53tsABmKohJ4j+tKvv2iP07SfUCYsKfCfXrPl+Zk= From: gregkh@linuxfoundation.org To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, syzbot , syzbot , syzbot , Tetsuo Handa , Shuah Khan Subject: [PATCH 4.19 086/120] usbip: fix stub_dev usbip_sockfd_store() races leading to gpf Date: Mon, 15 Mar 2021 14:57:17 +0100 Message-Id: <20210315135722.779571726@linuxfoundation.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210315135720.002213995@linuxfoundation.org> References: <20210315135720.002213995@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Greg Kroah-Hartman From: Shuah Khan commit 9380afd6df70e24eacbdbde33afc6a3950965d22 upstream. usbip_sockfd_store() is invoked when user requests attach (import) detach (unimport) usb 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. - 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. Tested with syzbot reproducer: - https://syzkaller.appspot.com/text?tag=ReproC&x=14801034d00000 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/268a0668144d5ff36ec7d87fdfa90faf583b7ccc.1615171203.git.skhan@linuxfoundation.org Signed-off-by: Greg Kroah-Hartman --- drivers/usb/usbip/stub_dev.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) --- a/drivers/usb/usbip/stub_dev.c +++ b/drivers/usb/usbip/stub_dev.c @@ -46,6 +46,8 @@ static ssize_t usbip_sockfd_store(struct int sockfd = 0; struct socket *socket; int rv; + struct task_struct *tcp_rx = NULL; + struct task_struct *tcp_tx = NULL; if (!sdev) { dev_err(dev, "sdev is null\n"); @@ -80,20 +82,36 @@ static ssize_t usbip_sockfd_store(struct goto sock_err; } - sdev->ud.tcp_socket = socket; - sdev->ud.sockfd = sockfd; - + /* unlock and create threads and get tasks */ spin_unlock_irq(&sdev->ud.lock); + tcp_rx = kthread_create(stub_rx_loop, &sdev->ud, "stub_rx"); + if (IS_ERR(tcp_rx)) { + sockfd_put(socket); + return -EINVAL; + } + tcp_tx = kthread_create(stub_tx_loop, &sdev->ud, "stub_tx"); + if (IS_ERR(tcp_tx)) { + kthread_stop(tcp_rx); + sockfd_put(socket); + return -EINVAL; + } - sdev->ud.tcp_rx = kthread_get_run(stub_rx_loop, &sdev->ud, - "stub_rx"); - sdev->ud.tcp_tx = kthread_get_run(stub_tx_loop, &sdev->ud, - "stub_tx"); + /* get task structs now */ + get_task_struct(tcp_rx); + get_task_struct(tcp_tx); + /* lock and update sdev->ud state */ spin_lock_irq(&sdev->ud.lock); + sdev->ud.tcp_socket = socket; + sdev->ud.sockfd = sockfd; + sdev->ud.tcp_rx = tcp_rx; + sdev->ud.tcp_tx = tcp_tx; sdev->ud.status = SDEV_ST_USED; spin_unlock_irq(&sdev->ud.lock); + wake_up_process(sdev->ud.tcp_rx); + wake_up_process(sdev->ud.tcp_tx); + } else { dev_info(dev, "stub down\n");