Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756109AbcLBC3y (ORCPT ); Thu, 1 Dec 2016 21:29:54 -0500 Received: from mga14.intel.com ([192.55.52.115]:9414 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751427AbcLBC3x (ORCPT ); Thu, 1 Dec 2016 21:29:53 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,284,1477983600"; d="scan'208";a="1066911837" From: Lu Baolu To: Mathias Nyman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Lu Baolu , Subject: [PATCH 1/1] usb: xhci: fix possible wild pointer Date: Fri, 2 Dec 2016 10:29:27 +0800 Message-Id: <1480645767-10931-1-git-send-email-baolu.lu@linux.intel.com> X-Mailer: git-send-email 2.1.4 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2148 Lines: 57 handle_cmd_completion() frees a command structure which might be still referenced by xhci->current_cmd. This might cause problem when xhci->current_cmd is accessed after that. A real-life case could be like this. The host takes a very long time to respond to a command, and the command timer is fired at the same time when the command completion event arrives. The command completion handler frees xhci->current_cmd before the timer function can grab xhci->lock. Afterward, timer function grabs the lock and go ahead with checking and setting members of xhci->current_cmd. Cc: # v3.16+ Signed-off-by: Lu Baolu --- drivers/usb/host/xhci-ring.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index bdf6b13..13e05f6 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1267,14 +1267,16 @@ void xhci_handle_command_timeout(unsigned long data) bool second_timeout = false; xhci = (struct xhci_hcd *) data; - /* mark this command to be cancelled */ spin_lock_irqsave(&xhci->lock, flags); - if (xhci->current_cmd) { - if (xhci->current_cmd->status == COMP_CMD_ABORT) - second_timeout = true; - xhci->current_cmd->status = COMP_CMD_ABORT; + if (!xhci->current_cmd) { + spin_unlock_irqrestore(&xhci->lock, flags); + return; } + if (xhci->current_cmd->status == COMP_CMD_ABORT) + second_timeout = true; + xhci->current_cmd->status = COMP_CMD_ABORT; + /* Make sure command ring is running before aborting it */ hw_ring_state = xhci_read_64(xhci, &xhci->op_regs->cmd_ring); if ((xhci->cmd_ring_state & CMD_RING_STATE_RUNNING) && @@ -1422,6 +1424,10 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, xhci->current_cmd = list_entry(cmd->cmd_list.next, struct xhci_command, cmd_list); mod_timer(&xhci->cmd_timer, jiffies + XHCI_CMD_DEFAULT_TIMEOUT); + } else if (xhci->current_cmd == cmd) { + xhci->current_cmd = NULL; + } else { + xhci_warn(xhci, "WARN current_cmd doesn't match command\n"); } event_handled: -- 2.1.4