From patchwork Wed Mar 15 21:58:39 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian King X-Patchwork-Id: 9626787 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id EF4C560244 for ; Wed, 15 Mar 2017 21:58:46 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E090F283C9 for ; Wed, 15 Mar 2017 21:58:46 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D570728433; Wed, 15 Mar 2017 21:58:46 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 68F2B283C9 for ; Wed, 15 Mar 2017 21:58:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753989AbdCOV6q (ORCPT ); Wed, 15 Mar 2017 17:58:46 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:34221 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753867AbdCOV6p (ORCPT ); Wed, 15 Mar 2017 17:58:45 -0400 Received: from pps.filterd (m0098420.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v2FLsSff072198 for ; Wed, 15 Mar 2017 17:58:44 -0400 Received: from e18.ny.us.ibm.com (e18.ny.us.ibm.com [129.33.205.208]) by mx0b-001b2d01.pphosted.com with ESMTP id 296nfxux5j-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Wed, 15 Mar 2017 17:58:44 -0400 Received: from localhost by e18.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 15 Mar 2017 17:58:43 -0400 Received: from b03cxnp08028.gho.boulder.ibm.com (9.17.130.20) by e18.ny.us.ibm.com (146.89.104.205) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 15 Mar 2017 17:58:40 -0400 Received: from b03ledav004.gho.boulder.ibm.com (b03ledav004.gho.boulder.ibm.com [9.17.130.235]) by b03cxnp08028.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v2FLwdfp13763040; Wed, 15 Mar 2017 14:58:39 -0700 Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id BB41178056; Wed, 15 Mar 2017 15:58:39 -0600 (MDT) Received: from localhost.localdomain (unknown [9.10.86.20]) by b03ledav004.gho.boulder.ibm.com (Postfix) with ESMTP id 717E878041; Wed, 15 Mar 2017 15:58:39 -0600 (MDT) Subject: [PATCH 3/6] ipr: Fix abort path race condition To: James.Bottomley@HansenPartnership.com Cc: martin.petersen@oracle.com, linux-scsi@vger.kernel.org, wenxiong@linux.vnet.ibm.com, brking@linux.vnet.ibm.com From: Brian King Date: Wed, 15 Mar 2017 16:58:39 -0500 X-TM-AS-GCONF: 00 x-cbid: 17031521-0044-0000-0000-000002D79805 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00006789; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000206; SDB=6.00834348; UDB=6.00409730; IPR=6.00612004; BA=6.00005214; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00014664; XFM=3.00000013; UTC=2017-03-15 21:58:42 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17031521-0045-0000-0000-0000070596EC Message-Id: <20170315215839.717E878041@b03ledav004.gho.boulder.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-03-15_09:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=10 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1702020001 definitions=main-1703150166 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This fixes a race condition in the error handlomg paths of ipr. While a command is outstanding to the adapter, it is placed on a pending queue for the hrrq it is associated with, while holding the HRRQ lock. When a command is completed, it is removed from the pending queue, under HRRQ lock, and placed on a local list. This list is then iterated through without any locks and each command's done function is invoked, inside of which, the command gets returned to the free list while grabbing the HRRQ lock. This fixes two race conditions when commands have been removed from the pending list but have not yet been added to the free list. Both of these changes fix race conditions that could result in returning success from eh_abort_handler and then later calling scsi_done for the same request. The first race condition is in ipr_cancel_op. It looks through each pending queue to see if the command to be aborted is still outstanding or not. Rather than looking on the pending queue, reverse the logic to check to look for commands that are NOT on the free queue. The second race condition can occur when in ipr_wait_for_ops where we are waiting for responses for commands we've aborted. Signed-off-by: Brian King --- drivers/scsi/ipr.c | 64 ++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 17 deletions(-) diff -puN drivers/scsi/ipr.c~ipr_wait_for_ops_fixup2 drivers/scsi/ipr.c --- linux-2.6.git/drivers/scsi/ipr.c~ipr_wait_for_ops_fixup2 2017-03-13 15:55:24.902123623 -0500 +++ linux-2.6.git-bjking1/drivers/scsi/ipr.c 2017-03-13 15:55:24.908123599 -0500 @@ -5008,6 +5008,25 @@ static int ipr_match_lun(struct ipr_cmnd } /** + * ipr_cmnd_is_free - Check if a command is free or not + * @ipr_cmd ipr command struct + * + * Returns: + * true / false + **/ +static bool ipr_cmnd_is_free(struct ipr_cmnd *ipr_cmd) +{ + struct ipr_cmnd *loop_cmd; + + list_for_each_entry(loop_cmd, &ipr_cmd->hrrq->hrrq_free_q, queue) { + if (loop_cmd == ipr_cmd) + return true; + } + + return false; +} + +/** * ipr_wait_for_ops - Wait for matching commands to complete * @ipr_cmd: ipr command struct * @device: device to match (sdev) @@ -5020,7 +5039,7 @@ static int ipr_wait_for_ops(struct ipr_i int (*match)(struct ipr_cmnd *, void *)) { struct ipr_cmnd *ipr_cmd; - int wait; + int wait, i; unsigned long flags; struct ipr_hrr_queue *hrrq; signed long timeout = IPR_ABORT_TASK_TIMEOUT; @@ -5032,10 +5051,13 @@ static int ipr_wait_for_ops(struct ipr_i for_each_hrrq(hrrq, ioa_cfg) { spin_lock_irqsave(hrrq->lock, flags); - list_for_each_entry(ipr_cmd, &hrrq->hrrq_pending_q, queue) { - if (match(ipr_cmd, device)) { - ipr_cmd->eh_comp = ∁ - wait++; + for (i = hrrq->min_cmd_id; i <= hrrq->max_cmd_id; i++) { + ipr_cmd = ioa_cfg->ipr_cmnd_list[i]; + if (!ipr_cmnd_is_free(ipr_cmd)) { + if (match(ipr_cmd, device)) { + ipr_cmd->eh_comp = ∁ + wait++; + } } } spin_unlock_irqrestore(hrrq->lock, flags); @@ -5049,10 +5071,13 @@ static int ipr_wait_for_ops(struct ipr_i for_each_hrrq(hrrq, ioa_cfg) { spin_lock_irqsave(hrrq->lock, flags); - list_for_each_entry(ipr_cmd, &hrrq->hrrq_pending_q, queue) { - if (match(ipr_cmd, device)) { - ipr_cmd->eh_comp = NULL; - wait++; + for (i = hrrq->min_cmd_id; i <= hrrq->max_cmd_id; i++) { + ipr_cmd = ioa_cfg->ipr_cmnd_list[i]; + if (!ipr_cmnd_is_free(ipr_cmd)) { + if (match(ipr_cmd, device)) { + ipr_cmd->eh_comp = NULL; + wait++; + } } } spin_unlock_irqrestore(hrrq->lock, flags); @@ -5219,7 +5244,7 @@ static int __ipr_eh_dev_reset(struct scs struct ipr_ioa_cfg *ioa_cfg; struct ipr_resource_entry *res; struct ata_port *ap; - int rc = 0; + int rc = 0, i; struct ipr_hrr_queue *hrrq; ENTER; @@ -5241,10 +5266,14 @@ static int __ipr_eh_dev_reset(struct scs for_each_hrrq(hrrq, ioa_cfg) { spin_lock(&hrrq->_lock); - list_for_each_entry(ipr_cmd, &hrrq->hrrq_pending_q, queue) { + for (i = hrrq->min_cmd_id; i <= hrrq->max_cmd_id; i++) { + ipr_cmd = ioa_cfg->ipr_cmnd_list[i]; + if (ipr_cmd->ioarcb.res_handle == res->res_handle) { if (!ipr_cmd->qc) continue; + if (ipr_cmnd_is_free(ipr_cmd)) + continue; ipr_cmd->done = ipr_sata_eh_done; if (!(ipr_cmd->qc->flags & ATA_QCFLAG_FAILED)) { @@ -5394,7 +5423,7 @@ static int ipr_cancel_op(struct scsi_cmn struct ipr_resource_entry *res; struct ipr_cmd_pkt *cmd_pkt; u32 ioasc, int_reg; - int op_found = 0; + int i, op_found = 0; struct ipr_hrr_queue *hrrq; ENTER; @@ -5423,11 +5452,12 @@ static int ipr_cancel_op(struct scsi_cmn for_each_hrrq(hrrq, ioa_cfg) { spin_lock(&hrrq->_lock); - list_for_each_entry(ipr_cmd, &hrrq->hrrq_pending_q, queue) { - if (ipr_cmd->scsi_cmd == scsi_cmd) { - ipr_cmd->done = ipr_scsi_eh_done; - op_found = 1; - break; + for (i = hrrq->min_cmd_id; i <= hrrq->max_cmd_id; i++) { + if (ioa_cfg->ipr_cmnd_list[i]->scsi_cmd == scsi_cmd) { + if (!ipr_cmnd_is_free(ioa_cfg->ipr_cmnd_list[i])) { + op_found = 1; + break; + } } } spin_unlock(&hrrq->_lock);