[PATCH v9 9/9] scsi: mvumi: bound the firmware handshake retry loop

[Yehyeong Lee]

mvumi_check_handshake() drives the handshake by calling
mvumi_handshake_event() until mhba->fw_state reaches FW_STATE_STARTED,
with nothing bounding how many times that can happen:

	do {
		if (mvumi_handshake_event(mhba)) {
			...
			return -1;
		}
	} while (mhba->fw_state != FW_STATE_STARTED);

Each call advances the state machine by one step and has its own
FW_MAX_DELAY timeout, but that timeout is sampled on entry to
mvumi_handshake_event(), so it only bounds one step.  A firmware that
keeps asserting DRBL_HANDSHAKE_ISR, or that reports the same handshake
state every round, is answered every round and the loop does not end.

The firmware does not have to be hostile to keep the loop going.  The
page walk advances hs_header->page_code, which lives in the coherent DMA
handshake page, so a controller that writes that field back can keep the
walk at the same page.  The restart that mvumi_handshake() takes when the
firmware reports a status other than HS_STATUS_OK also returns to
HS_S_START without ending the loop.

Count the steps and give up after MVUMI_HS_MAX_STEPS.  A handshake that
completes needs four fixed steps plus one per page the firmware asks for
in the u8 total_pages, so 259 at most, and a restart adds two; 1024
leaves room for both and still terminates.

This does not change what the driver writes to the firmware.  In
particular, the HS_S_ABORT that mvumi_handshake() stores into its local
hs_fun before returning -1 is still never written to the doorbell, so the
FW_STATE_STARTING restart continues to work as before.

Fixes: f0c568a478f0 ("[SCSI] mvumi: Add Marvell UMI driver")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Yehyeong Lee <yhlee@xxxxxxxxxxxxxxxxxx>
---
 drivers/scsi/mvumi.c | 9 ++++++++-
 drivers/scsi/mvumi.h | 9 +++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c
index 994c04902fa12..a2759dbeedf75 100644
--- a/drivers/scsi/mvumi.c
+++ b/drivers/scsi/mvumi.c
@@ -1334,7 +1334,7 @@ static unsigned char mvumi_handshake_event(struct mvumi_hba *mhba)
 
 static unsigned char mvumi_check_handshake(struct mvumi_hba *mhba)
 {
-	unsigned int tmp;
+	unsigned int tmp, steps;
 	unsigned long before;
 
 	before = jiffies;
@@ -1355,6 +1355,7 @@ static unsigned char mvumi_check_handshake(struct mvumi_hba *mhba)
 
 	mhba->fw_state = FW_STATE_STARTING;
 	dev_dbg(&mhba->pdev->dev, "start firmware handshake...\n");
+	steps = 0;
 	do {
 		if (mvumi_handshake_event(mhba)) {
 			dev_err(&mhba->pdev->dev,
@@ -1362,6 +1363,12 @@ static unsigned char mvumi_check_handshake(struct mvumi_hba *mhba)
 						mhba->fw_state);
 			return -1;
 		}
+		if (++steps > MVUMI_HS_MAX_STEPS) {
+			dev_err(&mhba->pdev->dev,
+				"handshake did not finish in %u steps, state 0x%x.\n",
+				MVUMI_HS_MAX_STEPS, mhba->fw_state);
+			return -1;
+		}
 	} while (mhba->fw_state != FW_STATE_STARTED);
 
 	dev_dbg(&mhba->pdev->dev, "firmware handshake done\n");
diff --git a/drivers/scsi/mvumi.h b/drivers/scsi/mvumi.h
index d998c65c26472..9597e87a08167 100644
--- a/drivers/scsi/mvumi.h
+++ b/drivers/scsi/mvumi.h
@@ -399,6 +399,15 @@ struct mvumi_hs_header {
 #define HS_PAGE_CL_INFO		0x4
 #define HS_PAGE_TOTAL		0x5
 
+/*
+ * Each mvumi_handshake() call advances the state machine by one step: four
+ * fixed steps, plus one per page the firmware asks for in the u8 total_pages,
+ * plus a restart if it ever reports a status other than HS_STATUS_OK.  This
+ * caps how many steps the driver will take before giving up, so that a
+ * firmware that keeps the handshake going cannot keep it going forever.
+ */
+#define MVUMI_HS_MAX_STEPS	1024
+
 #define HSP_SIZE(i)	sizeof(struct mvumi_hs_page##i)
 
 #define HSP_MAX_SIZE ({					\
-- 
2.43.0