From 34a40e0dff940ac5eba494a69b553ea571e24873 Mon Sep 17 00:00:00 2001 From: Nilesh Javali Date: Thu, 23 Jul 2026 10:33:57 +0530 Subject: [PATCH] scsi: qla2xxx: Fix queue teardown NULL dma_free and bitmap locking qla25xx_free_req_que() and qla25xx_free_rsp_que() have two pre-existing bugs exposed on the error path of qla25xx_create_{req,rsp}_que(): 1. When dma_alloc_coherent() fails during queue creation, the error path calls the free function with req->ring / rsp->ring still NULL (from kzalloc). The unconditional dma_free_coherent() with a NULL cpu_addr is undefined behavior and can panic. 2. The free functions clear req_qid_map / rsp_qid_map under vport_lock, but the create functions protect the same bitmaps with mq_lock. This provides no mutual exclusion. Additionally, the create error path clears the bit and releases mq_lock before calling the free function, creating a window where another thread can allocate the same que_id and have its ha->req_q_map entry clobbered by the subsequent lockless NULL assignment in the free function. Fix by: - Guarding dma_free_coherent() with a NULL check on the ring pointer. - Using mq_lock (the lock held by all creators) in the free functions to atomically NULL the map entry and clear the bitmap bit. - Removing the now-redundant clear_bit blocks from the create error paths since the free functions handle it atomically. Signed-off-by: Nilesh Javali Reviewed-by: Hannes Reinecke Link: https://patch.msgid.link/20260723050413.3897522-41-njavali@marvell.com Signed-off-by: Martin K. Petersen (Oracle) --- drivers/scsi/qla2xxx/qla_mid.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c index bb2521c052bf..7072af5b4217 100644 --- a/drivers/scsi/qla2xxx/qla_mid.c +++ b/drivers/scsi/qla2xxx/qla_mid.c @@ -576,16 +576,17 @@ qla25xx_free_req_que(struct scsi_qla_host *vha, struct req_que *req) uint16_t que_id = req->id; size_t req_entry_size = qla_req_entry_size(ha); - dma_free_coherent(&ha->pdev->dev, - (req->length + 1) * req_entry_size, - req->ring, req->dma); + if (req->ring) + dma_free_coherent(&ha->pdev->dev, + (req->length + 1) * req_entry_size, + req->ring, req->dma); req->ring = NULL; req->dma = 0; if (que_id) { + mutex_lock(&ha->mq_lock); ha->req_q_map[que_id] = NULL; - mutex_lock(&ha->vport_lock); clear_bit(que_id, ha->req_qid_map); - mutex_unlock(&ha->vport_lock); + mutex_unlock(&ha->mq_lock); } kfree(req->outstanding_cmds); kfree(req); @@ -605,16 +606,17 @@ qla25xx_free_rsp_que(struct scsi_qla_host *vha, struct rsp_que *rsp) rsp->msix->handle = NULL; } - dma_free_coherent(&ha->pdev->dev, - (rsp->length + 1) * rsp_entry_size, - rsp->ring, rsp->dma); + if (rsp->ring) + dma_free_coherent(&ha->pdev->dev, + (rsp->length + 1) * rsp_entry_size, + rsp->ring, rsp->dma); rsp->ring = NULL; rsp->dma = 0; if (que_id) { + mutex_lock(&ha->mq_lock); ha->rsp_q_map[que_id] = NULL; - mutex_lock(&ha->vport_lock); clear_bit(que_id, ha->rsp_qid_map); - mutex_unlock(&ha->vport_lock); + mutex_unlock(&ha->mq_lock); } kfree(rsp); } @@ -820,9 +822,6 @@ qla25xx_create_req_que(struct qla_hw_data *ha, uint16_t options, if (ret != QLA_SUCCESS) { ql_log(ql_log_fatal, base_vha, 0x00df, "%s failed.\n", __func__); - mutex_lock(&ha->mq_lock); - clear_bit(que_id, ha->req_qid_map); - mutex_unlock(&ha->mq_lock); goto que_failed; } vha->flags.qpairs_req_created = 1; @@ -942,9 +941,6 @@ qla25xx_create_rsp_que(struct qla_hw_data *ha, uint16_t options, if (ret != QLA_SUCCESS) { ql_log(ql_log_fatal, base_vha, 0x00e7, "%s failed.\n", __func__); - mutex_lock(&ha->mq_lock); - clear_bit(que_id, ha->rsp_qid_map); - mutex_unlock(&ha->mq_lock); goto que_failed; } vha->flags.qpairs_rsp_created = 1;