[PATCH v4 08/10] mpi3mr: Fix SAS port allocation and registration error handling

Ranjan Kumar <[email protected]>
Newsgroups org.kernel.vger.linux-scsi
Message-ID <[email protected]>
During SAS port creation, the driver does not verify successful port
allocation before attempting registration, which can lead to a NULL
pointer dereference. Additionally, if registration fails, the allocated
port is not freed, resulting in a memory leak.

Fix this by adding a NULL check after allocation and freeing the port
when registration fails.

Additional fixes in the error handling path include:

1. Fixing similar missing NULL checks for rphy allocations.
2. Cleaning up after a failed rphy registration tried to remove a
   device that was never added, causing a crash. The rphy is now
   freed directly instead.
3. A failed rphy registration left the target device with a dangling
   pointer and a stuck pending flag. Both are now cleared.
4. Phys removed on error kept an internal flag set, permanently
   blocking them from being added to a port again. Now cleared
   alongside the list removal.
5. Could block in the SCSI mid-layer after a stop or reset had
   already begun, the same ABBA deadlock class fixed elsewhere. Both
   paths now stop before that call once that is detected.

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

diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c
index b309cfdf6687..db9cb0b03b9f 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_transport.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c
@@ -1429,9 +1429,15 @@ static struct mpi3mr_sas_port *mpi3mr_sas_port_add(struct mpi3mr_ioc *mrioc,
 	}
 
 	port = sas_port_alloc_num(mr_sas_node->parent_dev);
+	if (!port) {
+		ioc_err(mrioc, "failure at %s:%d/%s() (sas_port_alloc)!\n",
+		    __FILE__, __LINE__, __func__);
+		goto out_fail;
+	}
 	if ((sas_port_add(port))) {
 		ioc_err(mrioc, "failure at %s:%d/%s()!\n",
 		    __FILE__, __LINE__, __func__);
+		sas_port_free(port);
 		goto out_fail;
 	}
 
@@ -1451,14 +1457,32 @@ static struct mpi3mr_sas_port *mpi3mr_sas_port_add(struct mpi3mr_ioc *mrioc,
 	mr_sas_port->port = port;
 	if (mr_sas_port->remote_identify.device_type == SAS_END_DEVICE) {
 		rphy = sas_end_device_alloc(port);
+		if (!rphy) {
+			ioc_err(mrioc, "failure at %s:%d/%s() (sas_end_device_alloc)!\n",
+			    __FILE__, __LINE__, __func__);
+			sas_port_delete(port);
+			goto out_fail;
+		}
 		tgtdev->dev_spec.sas_sata_inf.rphy = rphy;
 	} else {
 		rphy = sas_expander_alloc(port,
 		    mr_sas_port->remote_identify.device_type);
+		if (!rphy) {
+			ioc_err(mrioc, "failure at %s:%d/%s() (sas_expander_alloc)!\n",
+			    __FILE__, __LINE__, __func__);
+			sas_port_delete(port);
+			goto out_fail;
+		}
 	}
 	rphy->identify = mr_sas_port->remote_identify;
 
 	spin_lock_irqsave(&mrioc->fwevt_lock, flags);
+	if (mrioc->stop_drv_processing || mrioc->reset_in_progress) {
+		spin_unlock_irqrestore(&mrioc->fwevt_lock, flags);
+		sas_rphy_free(rphy);
+		sas_port_delete(port);
+		goto out_fail;
+	}
 	if (mrioc->current_event)
 		mrioc->current_event->pending_at_sml = 1;
 	spin_unlock_irqrestore(&mrioc->fwevt_lock, flags);
@@ -1466,6 +1490,18 @@ static struct mpi3mr_sas_port *mpi3mr_sas_port_add(struct mpi3mr_ioc *mrioc,
 	if ((sas_rphy_add(rphy))) {
 		ioc_err(mrioc, "failure at %s:%d/%s()!\n",
 		    __FILE__, __LINE__, __func__);
+		spin_lock_irqsave(&mrioc->fwevt_lock, flags);
+		if (mrioc->current_event) {
+			mrioc->current_event->pending_at_sml = 0;
+			discard = mrioc->current_event->discard;
+		}
+		spin_unlock_irqrestore(&mrioc->fwevt_lock, flags);
+		if (discard)
+			mpi3mr_print_device_event_notice(mrioc, true);
+		sas_rphy_unlink(rphy);
+		sas_rphy_free(rphy);
+		sas_port_delete(port);
+		goto out_fail;
 	}
 	if (mr_sas_port->remote_identify.device_type == SAS_END_DEVICE) {
 		tgtdev->dev_spec.sas_sata_inf.pend_sas_rphy_add = 0;
@@ -1504,9 +1540,17 @@ static struct mpi3mr_sas_port *mpi3mr_sas_port_add(struct mpi3mr_ioc *mrioc,
 	return mr_sas_port;
 
  out_fail:
+	if (tgtdev) {
+		tgtdev->dev_spec.sas_sata_inf.pend_sas_rphy_add = 0;
+		tgtdev->dev_spec.sas_sata_inf.rphy = NULL;
+		mpi3mr_tgtdev_put(tgtdev);
+	}
+
 	list_for_each_entry_safe(mr_sas_phy, next, &mr_sas_port->phy_list,
-	    port_siblings)
+	    port_siblings) {
+		mr_sas_phy->phy_belongs_to_port = 0;
 		list_del(&mr_sas_phy->port_siblings);
+	}
 	kfree(mr_sas_port);
 	return NULL;
 }
@@ -1600,12 +1644,12 @@ static void mpi3mr_sas_port_remove(struct mpi3mr_ioc *mrioc, u64 sas_address,
 			    mr_sas_port->remote_identify.sas_address,
 			    mr_sas_phy->phy_id);
 		mr_sas_phy->phy_belongs_to_port = 0;
-		if (!mrioc->stop_drv_processing)
+		if (!mrioc->stop_drv_processing && !mrioc->reset_in_progress)
 			sas_port_delete_phy(mr_sas_port->port,
 			    mr_sas_phy->phy);
 		list_del(&mr_sas_phy->port_siblings);
 	}
-	if (!mrioc->stop_drv_processing)
+	if (!mrioc->stop_drv_processing && !mrioc->reset_in_progress)
 		sas_port_delete(mr_sas_port->port);
 	ioc_info(mrioc, "%s: removed sas_address(0x%016llx)\n",
 	    __func__, (unsigned long long)sas_address);
-- 
2.47.3
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.