From patchwork Tue Sep 8 08:15:13 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Wagner X-Patchwork-Id: 11762927 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A406959D for ; Tue, 8 Sep 2020 08:15:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 964212076C for ; Tue, 8 Sep 2020 08:15:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729896AbgIHIPb (ORCPT ); Tue, 8 Sep 2020 04:15:31 -0400 Received: from mx2.suse.de ([195.135.220.15]:37144 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729744AbgIHIPZ (ORCPT ); Tue, 8 Sep 2020 04:15:25 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 39646AD19; Tue, 8 Sep 2020 08:15:24 +0000 (UTC) From: Daniel Wagner To: linux-scsi@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Nilesh Javali , Martin Wilck , Arun Easi , Daniel Wagner Subject: [PATCH v3 1/4] qla2xxx: Warn if done() or free() are called on an already freed srb Date: Tue, 8 Sep 2020 10:15:13 +0200 Message-Id: <20200908081516.8561-2-dwagner@suse.de> X-Mailer: git-send-email 2.16.4 In-Reply-To: <20200908081516.8561-1-dwagner@suse.de> References: <20200908081516.8561-1-dwagner@suse.de> Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Emit a warning when ->done or ->free are called on an already freed srb. There is a hidden use-after-free bug in the driver which corrupts the srb memory pool which originates from the cleanup callbacks. An extensive search didn't bring any lights on the real problem. The initial fix was to set both pointers to NULL and try to catch invalid accesses. But instead the memory corruption was gone and the driver didn't crash. Since not all calling places check for NULL pointer, add explicitly default handlers. With this we workaround the memory corruption and add a debug help. Reviewed-by: Martin Wilck Signed-off-by: Daniel Wagner --- drivers/scsi/qla2xxx/qla_init.c | 10 ++++++++++ drivers/scsi/qla2xxx/qla_inline.h | 5 +++++ 2 files changed, 15 insertions(+) diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 57a2d76aa691..fb7d57dc4e69 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -63,6 +63,16 @@ void qla2x00_sp_free(srb_t *sp) qla2x00_rel_sp(sp); } +void qla2xxx_rel_done_warning(srb_t *sp, int res) +{ + WARN_ONCE(1, "Calling done() of an already freed srb %p object\n", sp); +} + +void qla2xxx_rel_free_warning(srb_t *sp) +{ + WARN_ONCE(1, "Calling free() of an already freed srb %p object\n", sp); +} + /* Asynchronous Login/Logout Routines -------------------------------------- */ unsigned long diff --git a/drivers/scsi/qla2xxx/qla_inline.h b/drivers/scsi/qla2xxx/qla_inline.h index 861dc522723c..2aa6f81f87c4 100644 --- a/drivers/scsi/qla2xxx/qla_inline.h +++ b/drivers/scsi/qla2xxx/qla_inline.h @@ -207,10 +207,15 @@ qla2xxx_get_qpair_sp(scsi_qla_host_t *vha, struct qla_qpair *qpair, return sp; } +void qla2xxx_rel_done_warning(srb_t *sp, int res); +void qla2xxx_rel_free_warning(srb_t *sp); + static inline void qla2xxx_rel_qpair_sp(struct qla_qpair *qpair, srb_t *sp) { sp->qpair = NULL; + sp->done = qla2xxx_rel_done_warning; + sp->free = qla2xxx_rel_free_warning; mempool_free(sp, qpair->srb_mempool); QLA_QPAIR_MARK_NOT_BUSY(qpair); } From patchwork Tue Sep 8 08:15:14 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Wagner X-Patchwork-Id: 11762931 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 20D9059D for ; Tue, 8 Sep 2020 08:15:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 13810218AC for ; Tue, 8 Sep 2020 08:15:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729856AbgIHIPa (ORCPT ); Tue, 8 Sep 2020 04:15:30 -0400 Received: from mx2.suse.de ([195.135.220.15]:37128 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729626AbgIHIPZ (ORCPT ); Tue, 8 Sep 2020 04:15:25 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 3B48DB691; Tue, 8 Sep 2020 08:15:24 +0000 (UTC) From: Daniel Wagner To: linux-scsi@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Nilesh Javali , Martin Wilck , Arun Easi , Daniel Wagner Subject: [PATCH v3 2/4] qla2xxx: Simplify return value logic in qla2x00_get_sp_from_handle() Date: Tue, 8 Sep 2020 10:15:14 +0200 Message-Id: <20200908081516.8561-3-dwagner@suse.de> X-Mailer: git-send-email 2.16.4 In-Reply-To: <20200908081516.8561-1-dwagner@suse.de> References: <20200908081516.8561-1-dwagner@suse.de> Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Refactor qla2x00_get_sp_from_handle() to avoid the unecessary goto if early returns are used. With this we can also avoid preinitilzing the sp pointer. Reviewed-by: Martin Wilck Signed-off-by: Daniel Wagner --- drivers/scsi/qla2xxx/qla_isr.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c index 27bcd346af7c..5d278155e4e7 100644 --- a/drivers/scsi/qla2xxx/qla_isr.c +++ b/drivers/scsi/qla2xxx/qla_isr.c @@ -1716,7 +1716,7 @@ qla2x00_get_sp_from_handle(scsi_qla_host_t *vha, const char *func, { struct qla_hw_data *ha = vha->hw; sts_entry_t *pkt = iocb; - srb_t *sp = NULL; + srb_t *sp; uint16_t index; index = LSW(pkt->handle); @@ -1728,13 +1728,13 @@ qla2x00_get_sp_from_handle(scsi_qla_host_t *vha, const char *func, set_bit(FCOE_CTX_RESET_NEEDED, &vha->dpc_flags); else set_bit(ISP_ABORT_NEEDED, &vha->dpc_flags); - goto done; + return NULL; } sp = req->outstanding_cmds[index]; if (!sp) { ql_log(ql_log_warn, vha, 0x5032, "Invalid completion handle (%x) -- timed-out.\n", index); - return sp; + return NULL; } if (sp->handle != index) { ql_log(ql_log_warn, vha, 0x5033, @@ -1743,8 +1743,6 @@ qla2x00_get_sp_from_handle(scsi_qla_host_t *vha, const char *func, } req->outstanding_cmds[index] = NULL; - -done: return sp; } From patchwork Tue Sep 8 08:15:15 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Wagner X-Patchwork-Id: 11762929 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E9C8F1599 for ; Tue, 8 Sep 2020 08:15:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DB7EF206A5 for ; Tue, 8 Sep 2020 08:15:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729872AbgIHIPa (ORCPT ); Tue, 8 Sep 2020 04:15:30 -0400 Received: from mx2.suse.de ([195.135.220.15]:37140 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729722AbgIHIPZ (ORCPT ); Tue, 8 Sep 2020 04:15:25 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 3D708B6CD; Tue, 8 Sep 2020 08:15:24 +0000 (UTC) From: Daniel Wagner To: linux-scsi@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Nilesh Javali , Martin Wilck , Arun Easi , Daniel Wagner Subject: [PATCH v3 3/4] qla2xxx: Log calling function name in qla2x00_get_sp_from_handle() Date: Tue, 8 Sep 2020 10:15:15 +0200 Message-Id: <20200908081516.8561-4-dwagner@suse.de> X-Mailer: git-send-email 2.16.4 In-Reply-To: <20200908081516.8561-1-dwagner@suse.de> References: <20200908081516.8561-1-dwagner@suse.de> Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Commit 7c3df1320e5e ("[SCSI] qla2xxx: Code changes to support new dynamic logging infrastructure.") removed the use of the func argument. Let's add it back. Signed-off-by: Daniel Wagner --- drivers/scsi/qla2xxx/qla_isr.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c index 5d278155e4e7..b0b6dd2b608d 100644 --- a/drivers/scsi/qla2xxx/qla_isr.c +++ b/drivers/scsi/qla2xxx/qla_isr.c @@ -1722,8 +1722,8 @@ qla2x00_get_sp_from_handle(scsi_qla_host_t *vha, const char *func, index = LSW(pkt->handle); if (index >= req->num_outstanding_cmds) { ql_log(ql_log_warn, vha, 0x5031, - "Invalid command index (%x) type %8ph.\n", - index, iocb); + "%s: Invalid command index (%x) type %8ph.\n", + func, index, iocb); if (IS_P3P_TYPE(ha)) set_bit(FCOE_CTX_RESET_NEEDED, &vha->dpc_flags); else @@ -1733,12 +1733,14 @@ qla2x00_get_sp_from_handle(scsi_qla_host_t *vha, const char *func, sp = req->outstanding_cmds[index]; if (!sp) { ql_log(ql_log_warn, vha, 0x5032, - "Invalid completion handle (%x) -- timed-out.\n", index); + "%s: Invalid completion handle (%x) -- timed-out.\n", + func, index); return NULL; } if (sp->handle != index) { ql_log(ql_log_warn, vha, 0x5033, - "SRB handle (%x) mismatch %x.\n", sp->handle, index); + "%s: SRB handle (%x) mismatch %x.\n", func, + sp->handle, index); return NULL; } From patchwork Tue Sep 8 08:15:16 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Wagner X-Patchwork-Id: 11762941 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id B863C1599 for ; Tue, 8 Sep 2020 08:16:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AAFCA21D81 for ; Tue, 8 Sep 2020 08:16:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729724AbgIHIP6 (ORCPT ); Tue, 8 Sep 2020 04:15:58 -0400 Received: from mx2.suse.de ([195.135.220.15]:37146 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729733AbgIHIPZ (ORCPT ); Tue, 8 Sep 2020 04:15:25 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 3E022B732; Tue, 8 Sep 2020 08:15:24 +0000 (UTC) From: Daniel Wagner To: linux-scsi@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Nilesh Javali , Martin Wilck , Arun Easi , Daniel Wagner Subject: [PATCH v3 4/4] qla2xxx: Handle incorrect entry_type entries Date: Tue, 8 Sep 2020 10:15:16 +0200 Message-Id: <20200908081516.8561-5-dwagner@suse.de> X-Mailer: git-send-email 2.16.4 In-Reply-To: <20200908081516.8561-1-dwagner@suse.de> References: <20200908081516.8561-1-dwagner@suse.de> Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org It was observed on an ISP8324 16Gb HBA with fw=8.08.203 (d0d5) in a PowerPC64 machine that pkt->entry_type was MBX_IOCB_TYPE/0x39 with an sp->type SRB_SCSI_CMD which is invalid and should not be possible. Reading the entry_type from the crash dump shows the expected value of STATUS_TYPE/0x03 but the call trace shows that qla24xx_mbx_iocb_entry() is used. Add a check to verify for consistency and reset the HBA if an invalid state is reached. Obviously, this is only a workaround until the real problem is solved. Signed-off-by: Daniel Wagner --- drivers/scsi/qla2xxx/qla_isr.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c index b0b6dd2b608d..f953564cbed8 100644 --- a/drivers/scsi/qla2xxx/qla_isr.c +++ b/drivers/scsi/qla2xxx/qla_isr.c @@ -3406,6 +3406,32 @@ void qla24xx_nvme_ls4_iocb(struct scsi_qla_host *vha, sp->done(sp, comp_status); } +static void qla24xx_process_mbx_iocb_response(struct scsi_qla_host *vha, + struct rsp_que *rsp, struct sts_entry_24xx *pkt) +{ + struct qla_hw_data *ha = vha->hw; + srb_t *sp; + const char func[] = "MBX-IOCB2"; + + sp = qla2x00_get_sp_from_handle(vha, func, rsp->req, pkt); + if (!sp) + return; + + if (sp->type == SRB_SCSI_CMD || + sp->type == SRB_NVME_CMD || + sp->type == SRB_TM_CMD) { + ql_log(ql_log_warn, vha, 0x509d, + "Inconsistent event entry type %d\n", sp->type); + if (IS_P3P_TYPE(ha)) + set_bit(FCOE_CTX_RESET_NEEDED, &vha->dpc_flags); + else + set_bit(ISP_ABORT_NEEDED, &vha->dpc_flags); + return; + } + + qla24xx_mbx_iocb_entry(vha, rsp->req, (struct mbx_24xx_entry *)pkt); +} + /** * qla24xx_process_response_queue() - Process response queue entries. * @vha: SCSI driver HA context @@ -3513,8 +3539,7 @@ void qla24xx_process_response_queue(struct scsi_qla_host *vha, (struct abort_entry_24xx *)pkt); break; case MBX_IOCB_TYPE: - qla24xx_mbx_iocb_entry(vha, rsp->req, - (struct mbx_24xx_entry *)pkt); + qla24xx_process_mbx_iocb_response(vha, rsp, pkt); break; case VP_CTRL_IOCB_TYPE: qla_ctrlvp_completed(vha, rsp->req,