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 <njavali@marvell.com>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Link: https://patch.msgid.link/20260723050413.3897522-41-njavali@marvell.com
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
This commit is contained in:
Nilesh Javali
2026-07-23 10:33:57 +05:30
committed by Martin K. Petersen (Oracle)
parent 0267b4fbab
commit 34a40e0dff

View File

@@ -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;