diff mbox series

[5/5] qedf: fc_rport_priv reference counting fixes

Message ID 20180905135347.138195-6-hare@suse.de (mailing list archive)
State Changes Requested
Headers show
Series qedf: fixup NULL pointer reference in fc_lookup_rport() | expand

Commit Message

Hannes Reinecke Sept. 5, 2018, 1:53 p.m. UTC
The fc_rport_priv structure is reference counted, so we need to
ensure that the reference is increased before accessing the structure.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/qedf/qedf_els.c  | 12 ++++++++++--
 drivers/scsi/qedf/qedf_io.c   | 32 ++++++++++++++++++++++----------
 drivers/scsi/qedf/qedf_main.c |  5 +++++
 3 files changed, 37 insertions(+), 12 deletions(-)
diff mbox series

Patch

diff --git a/drivers/scsi/qedf/qedf_els.c b/drivers/scsi/qedf/qedf_els.c
index f93379a4510c..b028c1330ed8 100644
--- a/drivers/scsi/qedf/qedf_els.c
+++ b/drivers/scsi/qedf/qedf_els.c
@@ -356,12 +356,18 @@  void qedf_restart_rport(struct qedf_rport *fcport)
 	spin_unlock_irqrestore(&fcport->rport_lock, flags);
 
 	rdata = fcport->rdata;
-	if (rdata) {
+	if (rdata && !kref_get_unless_zero(&rdata->kref)) {
+		fcport->rdata = NULL;
+		rdata = NULL;
+	}
+
+	if (rdata && rdata->rp_state == RPORT_ST_READY) {
 		lport = fcport->qedf->lport;
 		port_id = rdata->ids.port_id;
 		QEDF_ERR(&(fcport->qedf->dbg_ctx),
 		    "LOGO port_id=%x.\n", port_id);
 		fc_rport_logoff(rdata);
+		kref_put(&rdata->kref, fc_rport_destroy);
 		/* Recreate the rport and log back in */
 		mutex_lock(&lport->disc.disc_mutex);
 		rdata = fc_rport_create(lport, port_id);
@@ -369,8 +375,10 @@  void qedf_restart_rport(struct qedf_rport *fcport)
 			mutex_unlock(&lport->disc.disc_mutex);
 			fc_rport_login(rdata);
 			fcport->rdata = rdata;
-		} else
+		} else {
 			mutex_unlock(&lport->disc.disc_mutex);
+			fcport->rdata = NULL;
+		}
 	}
 	clear_bit(QEDF_RPORT_IN_RESET, &fcport->flags);
 }
diff --git a/drivers/scsi/qedf/qedf_io.c b/drivers/scsi/qedf/qedf_io.c
index 6bbc38b1b465..9086f10b24bf 100644
--- a/drivers/scsi/qedf/qedf_io.c
+++ b/drivers/scsi/qedf/qedf_io.c
@@ -1560,34 +1560,39 @@  int qedf_initiate_abts(struct qedf_ioreq *io_req, bool return_scsi_cmd_on_abts)
 		goto abts_err;
 	}
 
+	qedf = fcport->qedf;
 	rdata = fcport->rdata;
+	if (!rdata || !kref_get_unless_zero(&rdata->kref)) {
+		QEDF_ERR(&(qedf->dbg_ctx), "stale rport\n");
+		rc = 1;
+		goto abts_err;
+	}
 	r_a_tov = rdata->r_a_tov;
-	qedf = fcport->qedf;
 	lport = qedf->lport;
 
 	if (lport->state != LPORT_ST_READY || !(lport->link_up)) {
 		QEDF_ERR(&(qedf->dbg_ctx), "link is not ready\n");
 		rc = 1;
-		goto abts_err;
+		goto drop_kref;
 	}
 
 	if (atomic_read(&qedf->link_down_tmo_valid) > 0) {
 		QEDF_ERR(&(qedf->dbg_ctx), "link_down_tmo active.\n");
 		rc = 1;
-		goto abts_err;
+		goto drop_kref;
 	}
 
 	/* Ensure room on SQ */
 	if (!atomic_read(&fcport->free_sqes)) {
 		QEDF_ERR(&(qedf->dbg_ctx), "No SQ entries available\n");
 		rc = 1;
-		goto abts_err;
+		goto drop_kref;
 	}
 
 	if (test_bit(QEDF_RPORT_UPLOADING_CONNECTION, &fcport->flags)) {
 		QEDF_ERR(&qedf->dbg_ctx, "fcport is uploading.\n");
-		rc = 1;
-		goto out;
+		kref_put(&rdata->kref, fc_rport_destroy);
+		return 1;
 	}
 
 	if (!test_bit(QEDF_CMD_OUTSTANDING, &io_req->flags) ||
@@ -1596,8 +1601,8 @@  int qedf_initiate_abts(struct qedf_ioreq *io_req, bool return_scsi_cmd_on_abts)
 		QEDF_ERR(&(qedf->dbg_ctx), "io_req xid=0x%x already in "
 			  "cleanup or abort processing or already "
 			  "completed.\n", io_req->xid);
-		rc = 1;
-		goto out;
+		kref_put(&rdata->kref, fc_rport_destroy);
+		return 1;
 	}
 
 	kref_get(&io_req->refcount);
@@ -1632,13 +1637,14 @@  int qedf_initiate_abts(struct qedf_ioreq *io_req, bool return_scsi_cmd_on_abts)
 	spin_unlock_irqrestore(&fcport->rport_lock, flags);
 
 	return rc;
+drop_kref:
+	kref_put(&rdata->kref, fc_rport_destroy);
 abts_err:
 	/*
 	 * If the ABTS task fails to queue then we need to cleanup the
 	 * task at the firmware.
 	 */
 	qedf_initiate_cleanup(io_req, return_scsi_cmd_on_abts);
-out:
 	return rc;
 }
 
@@ -1925,6 +1931,7 @@  static int qedf_execute_tmf(struct qedf_rport *fcport, struct scsi_cmnd *sc_cmd,
 	unsigned long flags;
 	struct fcoe_wqe *sqe;
 	u16 sqe_idx;
+	struct fc_rport_priv *rdata = fcport->rdata;
 
 	if (!sc_cmd) {
 		QEDF_ERR(&(qedf->dbg_ctx), "invalid arg\n");
@@ -1937,8 +1944,12 @@  static int qedf_execute_tmf(struct qedf_rport *fcport, struct scsi_cmnd *sc_cmd,
 		return FAILED;
 	}
 
+	if (!rdata || !kref_get_unless_zero(&rdata->kref)) {
+		QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_SCSI_TM, "stale rport\n");
+		return FAILED;
+	}
 	QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_SCSI_TM, "portid = 0x%x "
-		   "tm_flags = %d\n", fcport->rdata->ids.port_id, tm_flags);
+		   "tm_flags = %d\n", rdata->ids.port_id, tm_flags);
 
 	io_req = qedf_alloc_cmd(fcport, QEDF_TASK_MGMT_CMD);
 	if (!io_req) {
@@ -2019,6 +2030,7 @@  static int qedf_execute_tmf(struct qedf_rport *fcport, struct scsi_cmnd *sc_cmd,
 		rc = SUCCESS;
 	}
 reset_tmf_err:
+	kref_put(&rdata->kref, fc_rport_destroy);
 	return rc;
 }
 
diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
index 2bedad13f3f4..91d724678e15 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -1215,6 +1215,8 @@  static void qedf_upload_connection(struct qedf_ctx *qedf,
 static void qedf_cleanup_fcport(struct qedf_ctx *qedf,
 	struct qedf_rport *fcport)
 {
+	struct fc_rport_priv *rdata = fcport->rdata;
+
 	QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_CONN, "Cleaning up portid=%06x.\n",
 	    fcport->rdata->ids.port_id);
 
@@ -1226,6 +1228,7 @@  static void qedf_cleanup_fcport(struct qedf_ctx *qedf,
 	qedf_free_sq(qedf, fcport);
 	fcport->rdata = NULL;
 	fcport->qedf = NULL;
+	kref_put(&rdata->kref, fc_rport_destroy);
 }
 
 /**
@@ -1301,6 +1304,8 @@  static void qedf_rport_event_handler(struct fc_lport *lport,
 			break;
 		}
 
+		/* Initial reference held on entry, so this can't fail */
+		kref_get(&rdata->kref);
 		fcport->rdata = rdata;
 		fcport->rport = rport;