From 6e7fb2024250405d737e56985d6a86282f1175c4 Mon Sep 17 00:00:00 2001 From: Himanshu Madhani Date: Thu, 1 Aug 2019 15:55:51 -0400 Subject: [PATCH 091/124] [scsi] scsi: qla2xxx: Fix use-after-free issues in qla2xxx_qpair_sp_free_dma() Message-id: <20190801155618.12650-92-hmadhani@redhat.com> Patchwork-id: 267868 O-Subject: [RHEL 7.8 e-stor PATCH 091/118] scsi: qla2xxx: Fix use-after-free issues in qla2xxx_qpair_sp_free_dma() Bugzilla: 1729270 RH-Acked-by: Jarod Wilson RH-Acked-by: Tony Camuso From: Bart Van Assche Bugzilla 1729270 The current order for freeing memory is as follows: - struct crc_context itself. - struct crc_context member pointers. Change the freeing order into the following: - struct crc_context member pointers. - struct crc_context itself. Detected by Coverity. Cc: Himanshu Madhani Cc: Giridhar Malavali Fixes: 50b812755e97 ("scsi: qla2xxx: Fix DMA error when the DIF sg buffer crosses 4GB boundary") # v5.1-rc1. Fixes: d74595278f4a ("scsi: qla2xxx: Add multiple queue pair functionality.") # v4.10. Signed-off-by: Bart Van Assche Acked-by: Himanshu Madhani Signed-off-by: Martin K. Petersen (cherry picked from commit d8f945bf8096375f458683b5718722a2d5dda2f0) Signed-off-by: Himanshu Madhani Conflicts: drivers/scsi/qla2xxx/qla_os.c [ HM: RHEL7.8 kernel does not have wait_event_lock_irq_timeout() ] [ commit 711a08d79f71 ("scsi: qla2xxx: Change abort wait_loop ] [ from msleep to wait_event_timeout") was not pulled into src ] [ resulting into deviation from upstream ] Signed-off-by: Himanshu Madhani Signed-off-by: Jan Stancek --- drivers/scsi/qla2xxx/qla_os.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c index 83d06db7e9fa..bc72d4802844 100644 --- a/drivers/scsi/qla2xxx/qla_os.c +++ b/drivers/scsi/qla2xxx/qla_os.c @@ -811,25 +811,8 @@ qla2xxx_qpair_sp_free_dma(void *ptr) sp->flags &= ~SRB_CRC_CTX_DSD_VALID; } - if (sp->flags & SRB_CRC_CTX_DMA_VALID) { - struct crc_context *ctx0 = ctx; - - dma_pool_free(ha->dl_dma_pool, ctx, ctx0->crc_ctx_dma); - sp->flags &= ~SRB_CRC_CTX_DMA_VALID; - } - - if (sp->flags & SRB_FCP_CMND_DMA_VALID) { - struct ct6_dsd *ctx1 = ctx; - dma_pool_free(ha->fcp_cmnd_dma_pool, ctx1->fcp_cmnd, - ctx1->fcp_cmnd_dma); - list_splice(&ctx1->dsd_list, &ha->gbl_dsd_list); - ha->gbl_dsd_inuse -= ctx1->dsd_use_cnt; - ha->gbl_dsd_avail += ctx1->dsd_use_cnt; - mempool_free(ctx1, ha->ctx_mempool); - sp->flags &= ~SRB_FCP_CMND_DMA_VALID; - } if (sp->flags & SRB_DIF_BUNDL_DMA_VALID) { - struct crc_context *difctx = sp->u.scmd.ctx; + struct crc_context *difctx = ctx; struct dsd_dma *dif_dsd, *nxt_dsd; list_for_each_entry_safe(dif_dsd, nxt_dsd, @@ -864,6 +847,25 @@ qla2xxx_qpair_sp_free_dma(void *ptr) sp->flags &= ~SRB_DIF_BUNDL_DMA_VALID; } + if (sp->flags & SRB_FCP_CMND_DMA_VALID) { + struct ct6_dsd *ctx1 = ctx; + + dma_pool_free(ha->fcp_cmnd_dma_pool, ctx1->fcp_cmnd, + ctx1->fcp_cmnd_dma); + list_splice(&ctx1->dsd_list, &ha->gbl_dsd_list); + ha->gbl_dsd_inuse -= ctx1->dsd_use_cnt; + ha->gbl_dsd_avail += ctx1->dsd_use_cnt; + mempool_free(ctx1, ha->ctx_mempool); + sp->flags &= ~SRB_FCP_CMND_DMA_VALID; + } + + if (sp->flags & SRB_CRC_CTX_DMA_VALID) { + struct crc_context *ctx0 = ctx; + + dma_pool_free(ha->dl_dma_pool, ctx, ctx0->crc_ctx_dma); + sp->flags &= ~SRB_CRC_CTX_DMA_VALID; + } + end: CMD_SP(cmd) = NULL; qla2xxx_rel_qpair_sp(sp->qpair, sp); -- 2.13.6