From patchwork Fri Dec 1 15:06:47 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathias Nyman X-Patchwork-Id: 13475956 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="cRnHKRMi" Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.24]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 220CAD6C for ; Fri, 1 Dec 2023 07:06:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1701443175; x=1732979175; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=w0DnSPmZj85cgbMbS+eDufLxzKMrnYRs0INaeLDar+8=; b=cRnHKRMiBqYJhUXK+KqbWR2GbxrKENUqfwap77t/w3C/w6YSMrXx+qIf gt/Osn8egvXwzNO9aJ+C9RkWcVhnv7m1gbsIWLk9nKj51C2HOh+K9aMQZ rKG8J4eDPsfHubflR3ChbQQjhG9A8EAFSdYot9fQGQQ6PEMg8/T8TiQUL a9D5LRjTTpb3sYpsqHhp4RkC/4amqewoMJiUEWzxTewGSl0KVXKzHSwqd EPOGQ8txbT1FZSVFlef7JmOLNvV/eE/2zjhEQCkDaW3yUzlaDIc9q/lP4 dbqgp1/cfZSjKuWX0890i4LMtyVbXsfvkfRC+6lzrexzn95ZeQURXWurC Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10911"; a="396309790" X-IronPort-AV: E=Sophos;i="6.04,241,1695711600"; d="scan'208";a="396309790" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Dec 2023 07:06:14 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10911"; a="943112679" X-IronPort-AV: E=Sophos;i="6.04,241,1695711600"; d="scan'208";a="943112679" Received: from mattu-haswell.fi.intel.com ([10.237.72.199]) by orsmga005.jf.intel.com with ESMTP; 01 Dec 2023 07:06:12 -0800 From: Mathias Nyman To: Cc: , Mathias Nyman , Kuen-Han Tsai Subject: [PATCH 19/19] xhci: fix possible null pointer deref during xhci urb enqueue Date: Fri, 1 Dec 2023 17:06:47 +0200 Message-Id: <20231201150647.1307406-20-mathias.nyman@linux.intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20231201150647.1307406-1-mathias.nyman@linux.intel.com> References: <20231201150647.1307406-1-mathias.nyman@linux.intel.com> Precedence: bulk X-Mailing-List: linux-usb@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 There is a short gap between urb being submitted and actually added to the endpoint queue (linked). If the device is disconnected during this time then usb core is not yet aware of the pending urb, and device may be freed just before xhci_urq_enqueue() continues, dereferencing the freed device. Freeing the device is protected by the xhci spinlock, so make sure we take and keep the lock while checking that device exists, dereference it, and add the urb to the queue. Remove the unnecessary URB check, usb core checks it before calling xhci_urb_enqueue() Suggested-by: Kuen-Han Tsai Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci.c | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index df31d44498d6..4929c4396e9e 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1521,24 +1521,7 @@ static int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag struct urb_priv *urb_priv; int num_tds; - if (!urb) - return -EINVAL; - ret = xhci_check_args(hcd, urb->dev, urb->ep, - true, true, __func__); - if (ret <= 0) - return ret ? ret : -EINVAL; - - slot_id = urb->dev->slot_id; ep_index = xhci_get_endpoint_index(&urb->ep->desc); - ep_state = &xhci->devs[slot_id]->eps[ep_index].ep_state; - - if (!HCD_HW_ACCESSIBLE(hcd)) - return -ESHUTDOWN; - - if (xhci->devs[slot_id]->flags & VDEV_PORT_ERROR) { - xhci_dbg(xhci, "Can't queue urb, port error, link inactive\n"); - return -ENODEV; - } if (usb_endpoint_xfer_isoc(&urb->ep->desc)) num_tds = urb->number_of_packets; @@ -1562,12 +1545,35 @@ static int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag spin_lock_irqsave(&xhci->lock, flags); + ret = xhci_check_args(hcd, urb->dev, urb->ep, + true, true, __func__); + if (ret <= 0) { + ret = ret ? ret : -EINVAL; + goto free_priv; + } + + slot_id = urb->dev->slot_id; + + if (!HCD_HW_ACCESSIBLE(hcd)) { + ret = -ESHUTDOWN; + goto free_priv; + } + + if (xhci->devs[slot_id]->flags & VDEV_PORT_ERROR) { + xhci_dbg(xhci, "Can't queue urb, port error, link inactive\n"); + ret = -ENODEV; + goto free_priv; + } + if (xhci->xhc_state & XHCI_STATE_DYING) { xhci_dbg(xhci, "Ep 0x%x: URB %p submitted for non-responsive xHCI host.\n", urb->ep->desc.bEndpointAddress, urb); ret = -ESHUTDOWN; goto free_priv; } + + ep_state = &xhci->devs[slot_id]->eps[ep_index].ep_state; + if (*ep_state & (EP_GETTING_STREAMS | EP_GETTING_NO_STREAMS)) { xhci_warn(xhci, "WARN: Can't enqueue URB, ep in streams transition state %x\n", *ep_state);