[PATCH v4 06/10] mpi3mr: Fix memory leak on operational queue creation failure

When operational queue creation fails after one or more queues have
been created, the error path frees the queue information arrays but
does not release the DMA memory segments associated with the created
queues, resulting in a memory leak. Fix this by ensuring that partially
allocated segments are freed immediately if a queue fails to create.

Additionally, resolve the following issues in the queue segment
free/alloc paths:

1. Clear mrioc->intr_info[].op_reply_q with WRITE_ONCE() and follow it
   with synchronize_irq() before freeing segments, and have the ISR
   paths read it once via READ_ONCE() into a local, to close a race
   where the ISR could use the pointer while it is being freed.

2. Free q_segment_list before checking q_segments in both free
   functions, since a kzalloc_objs() failure on q_segments left
   q_segment_list leaked via the early return.

Reported-by: Sashiko <[email protected]>
Closes: https://sashiko.dev/#/patchset/[email protected]?part=6
Closes: https://sashiko.dev/#/patchset/[email protected]?part=6
Closes: https://sashiko.dev/#/patchset/[email protected]?part=6
Signed-off-by: Chandrakanth Patil <[email protected]>
Signed-off-by: Ranjan Kumar <[email protected]>
---
 drivers/scsi/mpi3mr/mpi3mr_fw.c | 83 ++++++++++++++++++++++++---------
 1 file changed, 60 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index e6050b41e15a..5a780eb7fd43 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -698,6 +698,7 @@ static irqreturn_t mpi3mr_isr_primary(int irq, void *privdata)
 {
 	struct mpi3mr_intr_info *intr_info = privdata;
 	struct mpi3mr_ioc *mrioc;
+	struct op_reply_qinfo *op_reply_q;
 	u16 midx;
 	u32 num_admin_replies = 0, num_op_reply = 0;
 
@@ -713,9 +714,9 @@ static irqreturn_t mpi3mr_isr_primary(int irq, void *privdata)
 
 	if (!midx)
 		num_admin_replies = mpi3mr_process_admin_reply_q(mrioc);
-	if (intr_info->op_reply_q)
-		num_op_reply = mpi3mr_process_op_reply_q(mrioc,
-		    intr_info->op_reply_q);
+	op_reply_q = READ_ONCE(intr_info->op_reply_q);
+	if (op_reply_q)
+		num_op_reply = mpi3mr_process_op_reply_q(mrioc, op_reply_q);
 
 	if (num_admin_replies || num_op_reply)
 		return IRQ_HANDLED;
@@ -728,6 +729,7 @@ static irqreturn_t mpi3mr_isr_primary(int irq, void *privdata)
 static irqreturn_t mpi3mr_isr(int irq, void *privdata)
 {
 	struct mpi3mr_intr_info *intr_info = privdata;
+	struct op_reply_qinfo *op_reply_q;
 	int ret;
 
 	if (!intr_info)
@@ -740,11 +742,12 @@ static irqreturn_t mpi3mr_isr(int irq, void *privdata)
 	 * If more IOs are expected, schedule IRQ polling thread.
 	 * Otherwise exit from ISR.
 	 */
-	if ((threaded_isr_poll == false) || !intr_info->op_reply_q)
+	op_reply_q = READ_ONCE(intr_info->op_reply_q);
+	if ((threaded_isr_poll == false) || !op_reply_q)
 		return ret;
 
-	if (!intr_info->op_reply_q->enable_irq_poll ||
-	    !atomic_read(&intr_info->op_reply_q->pend_ios))
+	if (!op_reply_q->enable_irq_poll ||
+	    !atomic_read(&op_reply_q->pend_ios))
 		return ret;
 
 	disable_irq_nosync(intr_info->os_irq);
@@ -766,10 +769,15 @@ static irqreturn_t mpi3mr_isr_poll(int irq, void *privdata)
 {
 	struct mpi3mr_intr_info *intr_info = privdata;
 	struct mpi3mr_ioc *mrioc;
+	struct op_reply_qinfo *op_reply_q;
 	u16 midx;
 	u32 num_op_reply = 0;
 
-	if (!intr_info || !intr_info->op_reply_q)
+	if (!intr_info)
+		return IRQ_NONE;
+
+	op_reply_q = READ_ONCE(intr_info->op_reply_q);
+	if (!op_reply_q)
 		return IRQ_NONE;
 
 	mrioc = intr_info->mrioc;
@@ -780,20 +788,23 @@ static irqreturn_t mpi3mr_isr_poll(int irq, void *privdata)
 		if (!mrioc->intr_enabled || mrioc->unrecoverable)
 			break;
 
+		op_reply_q = READ_ONCE(intr_info->op_reply_q);
+		if (!op_reply_q)
+			break;
+
 		if (!midx)
 			mpi3mr_process_admin_reply_q(mrioc);
-		if (intr_info->op_reply_q)
-			num_op_reply +=
-			    mpi3mr_process_op_reply_q(mrioc,
-				intr_info->op_reply_q);
-		if (!atomic_read(&intr_info->op_reply_q->pend_ios))
+		num_op_reply +=
+		    mpi3mr_process_op_reply_q(mrioc, op_reply_q);
+		if (!atomic_read(&op_reply_q->pend_ios))
 			break;
 
 		usleep_range(MPI3MR_IRQ_POLL_SLEEP, 10 * MPI3MR_IRQ_POLL_SLEEP);
 
 	} while (num_op_reply < mrioc->max_host_ios);
 
-	intr_info->op_reply_q->enable_irq_poll = false;
+	if (op_reply_q)
+		op_reply_q->enable_irq_poll = false;
 	enable_irq(intr_info->os_irq);
 
 	return IRQ_HANDLED;
@@ -1973,10 +1984,6 @@ static void mpi3mr_free_op_req_q_segments(struct mpi3mr_ioc *mrioc, u16 q_idx)
 	int size;
 	struct segments *segments;
 
-	segments = mrioc->req_qinfo[q_idx].q_segments;
-	if (!segments)
-		return;
-
 	if (mrioc->enable_segqueue) {
 		size = MPI3MR_OP_REQ_Q_SEG_SIZE;
 		if (mrioc->req_qinfo[q_idx].q_segment_list) {
@@ -1990,6 +1997,10 @@ static void mpi3mr_free_op_req_q_segments(struct mpi3mr_ioc *mrioc, u16 q_idx)
 		size = mrioc->req_qinfo[q_idx].segment_qd *
 		    mrioc->facts.op_req_sz;
 
+	segments = mrioc->req_qinfo[q_idx].q_segments;
+	if (!segments)
+		return;
+
 	for (j = 0; j < mrioc->req_qinfo[q_idx].num_segments; j++) {
 		if (!segments[j].segment)
 			continue;
@@ -2016,10 +2027,17 @@ static void mpi3mr_free_op_reply_q_segments(struct mpi3mr_ioc *mrioc, u16 q_idx)
 	u16 j;
 	int size;
 	struct segments *segments;
+	u16 midx = REPLY_QUEUE_IDX_TO_MSIX_IDX(q_idx, mrioc->op_reply_q_offset);
 
-	segments = mrioc->op_reply_qinfo[q_idx].q_segments;
-	if (!segments)
-		return;
+	/*
+	 * Stop the ISR/poll thread from picking up this queue before its
+	 * segments are freed below, and wait for any in-flight handler
+	 * that already has the old pointer to finish using it.
+	 */
+	if (midx < mrioc->intr_info_count) {
+		WRITE_ONCE(mrioc->intr_info[midx].op_reply_q, NULL);
+		synchronize_irq(pci_irq_vector(mrioc->pdev, midx));
+	}
 
 	if (mrioc->enable_segqueue) {
 		size = MPI3MR_OP_REP_Q_SEG_SIZE;
@@ -2034,6 +2052,10 @@ static void mpi3mr_free_op_reply_q_segments(struct mpi3mr_ioc *mrioc, u16 q_idx)
 		size = mrioc->op_reply_qinfo[q_idx].segment_qd *
 		    mrioc->op_reply_desc_sz;
 
+	segments = mrioc->op_reply_qinfo[q_idx].q_segments;
+	if (!segments)
+		return;
+
 	for (j = 0; j < mrioc->op_reply_qinfo[q_idx].num_segments; j++) {
 		if (!segments[j].segment)
 			continue;
@@ -2500,7 +2522,7 @@ static int mpi3mr_create_op_req_q(struct mpi3mr_ioc *mrioc, u16 idx,
 static int mpi3mr_create_op_queues(struct mpi3mr_ioc *mrioc)
 {
 	int retval = 0;
-	u16 num_queues = 0, i = 0, msix_count_op_q = 1;
+	u16 num_queues = 0, i = 0, j = 0, msix_count_op_q = 1;
 	u32 ioc_status;
 	enum mpi3mr_iocstate ioc_state;
 
@@ -2552,6 +2574,13 @@ static int mpi3mr_create_op_queues(struct mpi3mr_ioc *mrioc)
 		}
 	}
 
+	if (i < num_queues) {
+		for (j = i; j < num_queues; j++) {
+			mpi3mr_free_op_req_q_segments(mrioc, j);
+			mpi3mr_free_op_reply_q_segments(mrioc, j);
+		}
+	}
+
 	if (i == 0) {
 		/* Not even one queue is created successfully*/
 		retval = -1;
@@ -2573,11 +2602,19 @@ static int mpi3mr_create_op_queues(struct mpi3mr_ioc *mrioc)
 
 	return retval;
 out_failed:
-	kfree(mrioc->req_qinfo);
-	mrioc->req_qinfo = NULL;
+	if (mrioc->req_qinfo) {
+		for (j = 0; j < i; j++) {
+			mpi3mr_free_op_req_q_segments(mrioc, j);
+			mpi3mr_free_op_reply_q_segments(mrioc, j);
+		}
+		kfree(mrioc->req_qinfo);
+		mrioc->req_qinfo = NULL;
+	}
+	mrioc->num_op_req_q = 0;
 
 	kfree(mrioc->op_reply_qinfo);
 	mrioc->op_reply_qinfo = NULL;
+	mrioc->num_op_reply_q = 0;
 
 	return retval;
 }
-- 
2.47.3