From patchwork Tue Mar 20 21:30:11 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Hutchings X-Patchwork-Id: 10297975 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 4ADC8600F6 for ; Tue, 20 Mar 2018 21:30:18 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 368D12964A for ; Tue, 20 Mar 2018 21:30:18 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2B3EC2969D; Tue, 20 Mar 2018 21:30:18 +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 05C2F2964A for ; Tue, 20 Mar 2018 21:30:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751546AbeCTVaQ (ORCPT ); Tue, 20 Mar 2018 17:30:16 -0400 Received: from imap1.codethink.co.uk ([176.9.8.82]:46077 "EHLO imap1.codethink.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751372AbeCTVaP (ORCPT ); Tue, 20 Mar 2018 17:30:15 -0400 Received: from [167.98.27.229] (helo=xylophone.i.decadent.org.uk) by imap1.codethink.co.uk with esmtpsa (Exim 4.84_2 #1 (Debian)) id 1eyOpl-00066A-6l; Tue, 20 Mar 2018 21:30:17 +0000 Date: Tue, 20 Mar 2018 21:30:11 +0000 From: Ben Hutchings To: qla2xxx-upstream@qlogic.com Cc: linux-scsi@vger.kernel.org Subject: [PATCH] qla2xxx: Fix race condition between iocb timeout and initialisation Message-ID: <20180320213011.ty6mitbrc3gra5bc@xylophone.i.decadent.org.uk> MIME-Version: 1.0 Content-Disposition: inline User-Agent: NeoMutt/20170113 (1.7.2) 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 qla2x00_init_timer() calls add_timer() on the iocb timeout timer, which means the timeout function pointer and any data that the function depends on must be initialised beforehand. Move this initialisation before each call to qla2x00_init_timer(). In some cases qla2x00_init_timer() initialises a completion structure needed by the timeout function, so move the call to add_timer() after that. Signed-off-by: Ben Hutchings Cc: stable@vger.kernel.org --- drivers/scsi/qla2xxx/qla_gs.c | 12 +++++++----- drivers/scsi/qla2xxx/qla_init.c | 37 +++++++++++++++++++++++-------------- drivers/scsi/qla2xxx/qla_inline.h | 2 +- drivers/scsi/qla2xxx/qla_iocb.c | 8 +++++--- drivers/scsi/qla2xxx/qla_mbx.c | 8 ++++---- drivers/scsi/qla2xxx/qla_mid.c | 2 +- drivers/scsi/qla2xxx/qla_mr.c | 5 +++-- drivers/scsi/qla2xxx/qla_target.c | 2 +- 8 files changed, 45 insertions(+), 31 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c index d4bf787a9ea2..e98ba70b7cbe 100644 --- a/drivers/scsi/qla2xxx/qla_gs.c +++ b/drivers/scsi/qla2xxx/qla_gs.c @@ -3796,6 +3796,7 @@ int qla24xx_async_gffid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->gen1 = fcport->rscn_gen; sp->gen2 = fcport->login_gen; + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); /* CT_IU preamble */ @@ -3814,7 +3815,6 @@ int qla24xx_async_gffid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->u.iocb_cmd.u.ctarg.rsp_size = GFF_ID_RSP_SIZE; sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS; - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; sp->done = qla24xx_async_gffid_sp_done; rval = qla2x00_start_sp(sp); @@ -4121,6 +4121,8 @@ static int qla24xx_async_gnnft(scsi_qla_host_t *vha, struct srb *sp, sp->name = "gnnft"; sp->gen1 = vha->hw->base_qpair->chip_reset; sp->gen2 = fc4_type; + + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); memset(sp->u.iocb_cmd.u.ctarg.rsp, 0, sp->u.iocb_cmd.u.ctarg.rsp_size); @@ -4137,7 +4139,6 @@ static int qla24xx_async_gnnft(scsi_qla_host_t *vha, struct srb *sp, sp->u.iocb_cmd.u.ctarg.req_size = GNN_FT_REQ_SIZE; sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS; - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; sp->done = qla2x00_async_gpnft_gnnft_sp_done; rval = qla2x00_start_sp(sp); @@ -4210,6 +4211,8 @@ int qla24xx_async_gpnft(scsi_qla_host_t *vha, u8 fc4_type) sp->name = "gpnft"; sp->gen1 = vha->hw->base_qpair->chip_reset; sp->gen2 = fc4_type; + + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); sp->u.iocb_cmd.u.ctarg.req = dma_zalloc_coherent(&vha->hw->pdev->dev, @@ -4248,7 +4251,6 @@ int qla24xx_async_gpnft(scsi_qla_host_t *vha, u8 fc4_type) sp->u.iocb_cmd.u.ctarg.rsp_size = rspsz; sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS; - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; sp->done = qla2x00_async_gpnft_gnnft_sp_done; rval = qla2x00_start_sp(sp); @@ -4356,6 +4358,7 @@ int qla24xx_async_gnnid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->gen1 = fcport->rscn_gen; sp->gen2 = fcport->login_gen; + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); /* CT_IU preamble */ @@ -4377,7 +4380,6 @@ int qla24xx_async_gnnid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->u.iocb_cmd.u.ctarg.rsp_size = GNN_ID_RSP_SIZE; sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS; - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; sp->done = qla2x00_async_gnnid_sp_done; rval = qla2x00_start_sp(sp); @@ -4493,6 +4495,7 @@ int qla24xx_async_gfpnid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->gen1 = fcport->rscn_gen; sp->gen2 = fcport->login_gen; + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); /* CT_IU preamble */ @@ -4514,7 +4517,6 @@ int qla24xx_async_gfpnid(scsi_qla_host_t *vha, fc_port_t *fcport) sp->u.iocb_cmd.u.ctarg.rsp_size = GFPN_ID_RSP_SIZE; sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS; - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; sp->done = qla2x00_async_gfpnid_sp_done; rval = qla2x00_start_sp(sp); diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 608a49a419aa..ecdb78924ca8 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -187,10 +187,11 @@ qla2x00_async_login(struct scsi_qla_host *vha, fc_port_t *fcport, sp->name = "login"; sp->gen1 = fcport->rscn_gen; sp->gen2 = fcport->login_gen; - qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); lio = &sp->u.iocb_cmd; lio->timeout = qla2x00_async_iocb_timeout; + qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); + sp->done = qla2x00_async_login_sp_done; lio->u.logio.flags |= SRB_LOGIN_COND_PLOGI; @@ -249,10 +250,11 @@ qla2x00_async_logout(struct scsi_qla_host *vha, fc_port_t *fcport) sp->type = SRB_LOGOUT_CMD; sp->name = "logout"; - qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); lio = &sp->u.iocb_cmd; lio->timeout = qla2x00_async_iocb_timeout; + qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); + sp->done = qla2x00_async_logout_sp_done; rval = qla2x00_start_sp(sp); if (rval != QLA_SUCCESS) @@ -311,10 +313,11 @@ qla2x00_async_prlo(struct scsi_qla_host *vha, fc_port_t *fcport) sp->type = SRB_PRLO_CMD; sp->name = "prlo"; - qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); lio = &sp->u.iocb_cmd; lio->timeout = qla2x00_async_iocb_timeout; + qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); + sp->done = qla2x00_async_prlo_sp_done; rval = qla2x00_start_sp(sp); if (rval != QLA_SUCCESS) @@ -416,10 +419,11 @@ qla2x00_async_adisc(struct scsi_qla_host *vha, fc_port_t *fcport, sp->type = SRB_ADISC_CMD; sp->name = "adisc"; - qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); lio = &sp->u.iocb_cmd; lio->timeout = qla2x00_async_iocb_timeout; + qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); + sp->done = qla2x00_async_adisc_sp_done; if (data[1] & QLA_LOGIO_LOGIN_RETRIED) lio->u.logio.flags |= SRB_LOGIN_RETRIED; @@ -749,6 +753,8 @@ int qla24xx_async_gnl(struct scsi_qla_host *vha, fc_port_t *fcport) sp->gen1 = fcport->rscn_gen; sp->gen2 = fcport->login_gen; + mbx = &sp->u.iocb_cmd; + mbx->timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha)+2); mb = sp->u.iocb_cmd.u.mbx.out_mb; @@ -761,9 +767,6 @@ int qla24xx_async_gnl(struct scsi_qla_host *vha, fc_port_t *fcport) mb[8] = vha->gnl.size; mb[9] = vha->vp_idx; - mbx = &sp->u.iocb_cmd; - mbx->timeout = qla2x00_async_iocb_timeout; - sp->done = qla24xx_async_gnl_sp_done; rval = qla2x00_start_sp(sp); @@ -892,10 +895,11 @@ qla24xx_async_prli(struct scsi_qla_host *vha, fc_port_t *fcport) sp->type = SRB_PRLI_CMD; sp->name = "prli"; - qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); lio = &sp->u.iocb_cmd; lio->timeout = qla2x00_async_iocb_timeout; + qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); + sp->done = qla2x00_async_prli_sp_done; lio->u.logio.flags = 0; @@ -960,6 +964,9 @@ int qla24xx_async_gpdb(struct scsi_qla_host *vha, fc_port_t *fcport, u8 opt) sp->name = "gpdb"; sp->gen1 = fcport->rscn_gen; sp->gen2 = fcport->login_gen; + + mbx = &sp->u.iocb_cmd; + mbx->timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); pd = dma_pool_zalloc(ha->s_dma_pool, GFP_KERNEL, &pd_dma); @@ -979,8 +986,6 @@ int qla24xx_async_gpdb(struct scsi_qla_host *vha, fc_port_t *fcport, u8 opt) mb[9] = vha->vp_idx; mb[10] = opt; - mbx = &sp->u.iocb_cmd; - mbx->timeout = qla2x00_async_iocb_timeout; mbx->u.mbx.in = (void *)pd; mbx->u.mbx.in_dma = pd_dma; @@ -1490,13 +1495,15 @@ qla2x00_async_tm_cmd(fc_port_t *fcport, uint32_t flags, uint32_t lun, tm_iocb = &sp->u.iocb_cmd; sp->type = SRB_TM_CMD; sp->name = "tmf"; + + tm_iocb->timeout = qla2x00_tmf_iocb_timeout; + init_completion(&tm_iocb->u.tmf.comp); qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha)); + tm_iocb->u.tmf.flags = flags; tm_iocb->u.tmf.lun = lun; tm_iocb->u.tmf.data = tag; sp->done = qla2x00_tmf_sp_done; - tm_iocb->timeout = qla2x00_tmf_iocb_timeout; - init_completion(&tm_iocb->u.tmf.comp); rval = qla2x00_start_sp(sp); if (rval != QLA_SUCCESS) @@ -1570,7 +1577,11 @@ qla24xx_async_abort_cmd(srb_t *cmd_sp) abt_iocb = &sp->u.iocb_cmd; sp->type = SRB_ABT_CMD; sp->name = "abort"; + + abt_iocb->timeout = qla24xx_abort_iocb_timeout; + init_completion(&abt_iocb->u.abt.comp); qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha)); + abt_iocb->u.abt.cmd_hndl = cmd_sp->handle; if (vha->flags.qpairs_available && cmd_sp->qpair) @@ -1580,8 +1591,6 @@ qla24xx_async_abort_cmd(srb_t *cmd_sp) abt_iocb->u.abt.req_que_no = cpu_to_le16(vha->req->id); sp->done = qla24xx_abort_sp_done; - abt_iocb->timeout = qla24xx_abort_iocb_timeout; - init_completion(&abt_iocb->u.abt.comp); rval = qla2x00_start_sp(sp); if (rval != QLA_SUCCESS) diff --git a/drivers/scsi/qla2xxx/qla_inline.h b/drivers/scsi/qla2xxx/qla_inline.h index 80ac727d4401..f1d73196b7cb 100644 --- a/drivers/scsi/qla2xxx/qla_inline.h +++ b/drivers/scsi/qla2xxx/qla_inline.h @@ -272,13 +272,13 @@ qla2x00_init_timer(srb_t *sp, unsigned long tmo) timer_setup(&sp->u.iocb_cmd.timer, qla2x00_sp_timeout, 0); sp->u.iocb_cmd.timer.expires = jiffies + tmo * HZ; refcount_set(&sp->u.iocb_cmd.ref, 2); - add_timer(&sp->u.iocb_cmd.timer); sp->free = qla2x00_sp_free; init_completion(&sp->comp); if (IS_QLAFX00(sp->vha->hw) && (sp->type == SRB_FXIOCB_DCMD)) init_completion(&sp->u.iocb_cmd.u.fxiocb.fxiocb_comp); if (sp->type == SRB_ELS_DCMD) init_completion(&sp->u.iocb_cmd.u.els_logo.comp); + add_timer(&sp->u.iocb_cmd.timer); } static inline int diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c index 64104de6bdbf..bfde6ebc30d3 100644 --- a/drivers/scsi/qla2xxx/qla_iocb.c +++ b/drivers/scsi/qla2xxx/qla_iocb.c @@ -2457,8 +2457,8 @@ qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int els_opcode, sp->type = SRB_ELS_DCMD; sp->name = "ELS_DCMD"; sp->fcport = fcport; - qla2x00_init_timer(sp, ELS_DCMD_TIMEOUT); elsio->timeout = qla2x00_els_dcmd_iocb_timeout; + qla2x00_init_timer(sp, ELS_DCMD_TIMEOUT); sp->done = qla2x00_els_dcmd_sp_done; sp->free = qla2x00_els_dcmd_sp_free; @@ -2654,8 +2654,11 @@ qla24xx_els_dcmd2_iocb(scsi_qla_host_t *vha, int els_opcode, sp->type = SRB_ELS_DCMD; sp->name = "ELS_DCMD"; sp->fcport = fcport; - qla2x00_init_timer(sp, ELS_DCMD_TIMEOUT); + elsio->timeout = qla2x00_els_dcmd2_iocb_timeout; + init_completion(&elsio->u.els_plogi.comp); + qla2x00_init_timer(sp, ELS_DCMD_TIMEOUT); + sp->done = qla2x00_els_dcmd2_sp_done; sp->free = qla2x00_els_dcmd2_sp_free; @@ -2692,7 +2695,6 @@ qla24xx_els_dcmd2_iocb(scsi_qla_host_t *vha, int els_opcode, ql_dump_buffer(ql_dbg_io + ql_dbg_buffer, vha, 0x0109, (uint8_t *)elsio->u.els_plogi.els_plogi_pyld, 0x70); - init_completion(&elsio->u.els_plogi.comp); rval = qla2x00_start_sp(sp); if (rval != QLA_SUCCESS) { rval = QLA_FUNCTION_FAILED; diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c index 7397aeddd96c..4984570e210b 100644 --- a/drivers/scsi/qla2xxx/qla_mbx.c +++ b/drivers/scsi/qla2xxx/qla_mbx.c @@ -6006,14 +6006,14 @@ int qla24xx_send_mb_cmd(struct scsi_qla_host *vha, mbx_cmd_t *mcp) sp->type = SRB_MB_IOCB; sp->name = mb_to_str(mcp->mb[0]); - qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); - - memcpy(sp->u.iocb_cmd.u.mbx.out_mb, mcp->mb, SIZEOF_IOCB_MB_REG); - c = &sp->u.iocb_cmd; c->timeout = qla2x00_async_iocb_timeout; init_completion(&c->u.mbx.comp); + qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); + + memcpy(sp->u.iocb_cmd.u.mbx.out_mb, mcp->mb, SIZEOF_IOCB_MB_REG); + sp->done = qla2x00_async_mb_sp_done; rval = qla2x00_start_sp(sp); diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c index e965b16f21e3..07ed3aaedb4a 100644 --- a/drivers/scsi/qla2xxx/qla_mid.c +++ b/drivers/scsi/qla2xxx/qla_mid.c @@ -935,8 +935,8 @@ int qla24xx_control_vp(scsi_qla_host_t *vha, int cmd) sp->type = SRB_CTRL_VP; sp->name = "ctrl_vp"; sp->done = qla_ctrlvp_sp_done; - qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; + qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2); sp->u.iocb_cmd.u.ctrlvp.cmd = cmd; sp->u.iocb_cmd.u.ctrlvp.vp_index = vp_index; diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c index d5da3981cefe..134f2b8a49fe 100644 --- a/drivers/scsi/qla2xxx/qla_mr.c +++ b/drivers/scsi/qla2xxx/qla_mr.c @@ -1821,9 +1821,11 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t *fcport, uint16_t fx_type) sp->type = SRB_FXIOCB_DCMD; sp->name = "fxdisc"; - qla2x00_init_timer(sp, FXDISC_TIMEOUT); fdisc = &sp->u.iocb_cmd; + fdisc->timeout = qla2x00_fxdisc_iocb_timeout; + qla2x00_init_timer(sp, FXDISC_TIMEOUT); + switch (fx_type) { case FXDISC_GET_CONFIG_INFO: fdisc->u.fxiocb.flags = @@ -1924,7 +1926,6 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t *fcport, uint16_t fx_type) goto done_unmap_req; } - fdisc->timeout = qla2x00_fxdisc_iocb_timeout; fdisc->u.fxiocb.req_func_type = cpu_to_le16(fx_type); sp->done = qla2x00_fxdisc_sp_done; diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index b49ac85f3de2..84ca07356da1 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -664,10 +664,10 @@ int qla24xx_async_notify_ack(scsi_qla_host_t *vha, fc_port_t *fcport, sp->type = type; sp->name = "nack"; + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha)+2); sp->u.iocb_cmd.u.nack.ntfy = ntfy; - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout; sp->done = qla2x00_async_nack_sp_done; rval = qla2x00_start_sp(sp);