From patchwork Thu Aug 9 15:43:29 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bart Van Assche X-Patchwork-Id: 1301381 X-Patchwork-Delegate: dave@thedillows.org Return-Path: X-Original-To: patchwork-linux-rdma@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id 2C33BDFF7B for ; Thu, 9 Aug 2012 15:43:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1031113Ab2HIPne (ORCPT ); Thu, 9 Aug 2012 11:43:34 -0400 Received: from relay04ant.iops.be ([212.53.5.219]:49123 "EHLO relay04ant.iops.be" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030916Ab2HIPnd (ORCPT ); Thu, 9 Aug 2012 11:43:33 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by relay04ant.iops.be (Postfix) with ESMTP id 03E5F61B8156; Thu, 9 Aug 2012 17:43:32 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iops.be; h= content-transfer-encoding:content-type:content-type:in-reply-to :references:subject:subject:mime-version:user-agent:from:from :date:date:message-id:received:received; s=scooby; i= postadmin@iops.be; t=1344527010; bh=W1uovmbK6+TgP4KkUBOrgQHoHPrA oQ6FamhIXuofFqo=; b=N5cZPtStPKTbVlVFXsXKoE6rArGYiYqJzCmFznOY1DQT NUnhAATEsOgMrzMEynrF1sT/eWO/DEeuUPh8nqxjCUahLGrw650nTFPAy4fM5DZ4 uqYUgl+2LexMwg+7UYZgP27m+qGHal9MNWQnDgzYbITgZfwy1vOMIEb6bhukxyY= X-Virus-Scanned: amavisd-new at iops.be Received: from relay04ant.iops.be ([127.0.0.1]) by localhost (bdell029.dcn.iops.be [127.0.0.1]) (amavisd-new, port 10026) with LMTP id LRmWX17j0ibl; Thu, 9 Aug 2012 17:43:30 +0200 (CEST) Received: from [192.168.1.65] (cust-4-33-110-94.dyn.as47377.net [94.110.33.4]) by relay04ant.iops.be (Postfix) with ESMTP id C243461B8147; Thu, 9 Aug 2012 17:43:29 +0200 (CEST) Message-ID: <5023DAA1.1040507@acm.org> Date: Thu, 09 Aug 2012 15:43:29 +0000 From: Bart Van Assche User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120713 Thunderbird/14.0 MIME-Version: 1.0 To: "linux-rdma@vger.kernel.org" CC: David Dillow , Roland Dreier , Joseph Glanville Subject: [PATCH 01/20] ib_srp: Fix a race condition References: <5023DA39.7020000@acm.org> In-Reply-To: <5023DA39.7020000@acm.org> X-Enigmail-Version: 1.5a1pre Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org Avoid that the scmnd->scsi_done(scmnd) call in srp_process_rsp() can trigger a crash by being invoked with scsi_done == NULL. That could happen if a reply is received during or after a command abort. BUG: unable to handle kernel NULL pointer dereference at (null) Call Trace: [] ? srp_handle_recv+0x216/0x480 [ib_srp] [] srp_recv_completion+0x4a/0xb0 [ib_srp] [] mlx4_ib_cq_comp+0x17/0x20 [mlx4_ib] [] mlx4_cq_completion+0x40/0x80 [mlx4_core] [] mlx4_eq_int+0x543/0x920 [mlx4_core] [] ? local_clock+0x4f/0x60 [] mlx4_msi_x_interrupt+0x14/0x20 [mlx4_core] [] handle_irq_event_percpu+0x75/0x240 [] handle_irq_event+0x4e/0x80 [] handle_edge_irq+0x85/0x130 [] handle_irq+0x25/0x40 [] do_IRQ+0x5d/0xe0 [] common_interrupt+0x6c/0x6c Kernel panic - not syncing: Fatal exception in interrupt Reported-by: Joseph Glanville Reference: http://marc.info/?l=linux-rdma&m=134314367801595 Signed-off-by: Bart Van Assche Cc: David Dillow Cc: Roland Dreier Cc: --- drivers/infiniband/ulp/srp/ib_srp.c | 82 ++++++++++++++++++++++++---------- 1 files changed, 58 insertions(+), 24 deletions(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index bcbf22e..9a61be2 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -586,24 +586,61 @@ static void srp_unmap_data(struct scsi_cmnd *scmnd, scmnd->sc_data_direction); } -static void srp_remove_req(struct srp_target_port *target, - struct srp_request *req, s32 req_lim_delta) +/** + * srp_claim_req - Take ownership of the scmnd associated with a request. + * @target: SRP target port. + * @req: SRP request. + * @scmnd: If NULL, take ownership of @req->scmnd. If not NULL, only take + * ownership of @req->scmnd if it equals @scmnd. + * @req_lim_delta: target->req_lim_delta increment. + * + * Return value: + * Either NULL or a pointer to the SCSI command the caller became owner of. + */ +static struct scsi_cmnd *srp_claim_req(struct srp_target_port *target, + struct srp_request *req, + struct scsi_cmnd *scmnd, + s32 req_lim_delta) { unsigned long flags; - srp_unmap_data(req->scmnd, target, req); spin_lock_irqsave(&target->lock, flags); target->req_lim += req_lim_delta; - req->scmnd = NULL; + if (!scmnd) + swap(scmnd, req->scmnd); + else if (req->scmnd == scmnd) + req->scmnd = NULL; + else + scmnd = NULL; + spin_unlock_irqrestore(&target->lock, flags); + + return scmnd; +} + +/** + * srp_free_req() - Unmap data and add request to the free request list. + */ +static void srp_free_req(struct srp_target_port *target, + struct srp_request *req, struct scsi_cmnd *scmnd) +{ + unsigned long flags; + + srp_unmap_data(scmnd, target, req); + + spin_lock_irqsave(&target->lock, flags); list_add_tail(&req->list, &target->free_reqs); spin_unlock_irqrestore(&target->lock, flags); } static void srp_reset_req(struct srp_target_port *target, struct srp_request *req) { - req->scmnd->result = DID_RESET << 16; - req->scmnd->scsi_done(req->scmnd); - srp_remove_req(target, req, 0); + struct scsi_cmnd *scmnd = req->scmnd; + + if (srp_claim_req(target, req, scmnd, 0)) { + scmnd->result = DID_RESET << 16; + scmnd->scsi_done(scmnd); + srp_free_req(target, req, scmnd); + } } static int srp_reconnect_target(struct srp_target_port *target) @@ -1073,11 +1110,14 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp) complete(&target->tsk_mgmt_done); } else { req = &target->req_ring[rsp->tag]; - scmnd = req->scmnd; - if (!scmnd) + scmnd = srp_claim_req(target, req, NULL, + be32_to_cpu(rsp->req_lim_delta)); + if (!scmnd) { shost_printk(KERN_ERR, target->scsi_host, "Null scmnd for RSP w/tag %016llx\n", (unsigned long long) rsp->tag); + return; + } scmnd->result = rsp->status; if (rsp->flags & SRP_RSP_FLAG_SNSVALID) { @@ -1092,7 +1132,8 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp) else if (rsp->flags & (SRP_RSP_FLAG_DIOVER | SRP_RSP_FLAG_DIUNDER)) scsi_set_resid(scmnd, be32_to_cpu(rsp->data_in_res_cnt)); - srp_remove_req(target, req, be32_to_cpu(rsp->req_lim_delta)); + srp_free_req(target, req, scmnd); + scmnd->host_scribble = NULL; scmnd->scsi_done(scmnd); } @@ -1631,25 +1672,18 @@ static int srp_abort(struct scsi_cmnd *scmnd) { struct srp_target_port *target = host_to_target(scmnd->device->host); struct srp_request *req = (struct srp_request *) scmnd->host_scribble; - int ret = SUCCESS; shost_printk(KERN_ERR, target->scsi_host, "SRP abort called\n"); - if (!req || target->qp_in_error) + if (!req || target->qp_in_error || + !srp_claim_req(target, req, scmnd, 0)) return FAILED; - if (srp_send_tsk_mgmt(target, req->index, scmnd->device->lun, - SRP_TSK_ABORT_TASK)) - return FAILED; - - if (req->scmnd) { - if (!target->tsk_mgmt_status) { - srp_remove_req(target, req, 0); - scmnd->result = DID_ABORT << 16; - } else - ret = FAILED; - } + srp_send_tsk_mgmt(target, req->index, scmnd->device->lun, + SRP_TSK_ABORT_TASK); + srp_free_req(target, req, scmnd); + scmnd->result = DID_ABORT << 16; - return ret; + return SUCCESS; } static int srp_reset_device(struct scsi_cmnd *scmnd)