⬢ DragonFlyBSD Kernel Audit
DF-4335 / fix.diff
← back to finding ↓ download raw
--- a/sys/dev/raid/twa/tw_cl_io.c
+++ b/sys/dev/raid/twa/tw_cl_io.c
@@ -807,9 +807,17 @@
 		0x1101, 0x1, TW_CL_SEVERITY_ERROR_STRING,
 		"get_param failed",
 		"error = %d", error);
-	if (param)
+	/*
+	 * On TW_OSL_ETIMEDOUT the request was already re-queued by
+	 * tw_cli_submit_and_poll_request (PENDING case), or is still
+	 * owned by the firmware and will be re-queued exactly once by
+	 * tw_cli_poll_requeue_callback from the next controller reset
+	 * (BUSY case).  Re-queueing here would link one request into
+	 * the free queue twice (DF-4335).
+	 */
+	if ((param) && (error != TW_OSL_ETIMEDOUT))
 		ctlr->internal_req_busy = TW_CL_FALSE;
-	if (req)
+	if ((req) && (error != TW_OSL_ETIMEDOUT))
 		tw_cli_req_q_insert_tail(req, TW_CLI_FREE_Q);
 	return(1);
 }
@@ -924,9 +932,14 @@
 		0x1102, 0x1, TW_CL_SEVERITY_ERROR_STRING,
 		"set_param failed",
 		"error = %d", error);
-	if (param)
+	/*
+	 * See the matching comment in tw_cli_get_param(): never re-queue
+	 * on TW_OSL_ETIMEDOUT -- ownership stayed with
+	 * tw_cli_submit_and_poll_request / the firmware (DF-4335).
+	 */
+	if ((param) && (error != TW_OSL_ETIMEDOUT))
 		ctlr->internal_req_busy = TW_CL_FALSE;
-	if (req)
+	if ((req) && (error != TW_OSL_ETIMEDOUT))
 		tw_cli_req_q_insert_tail(req, TW_CLI_FREE_Q);
 	return(error);
 }
@@ -934,6 +947,28 @@
 
 
 /*
+ * Function name:	tw_cli_poll_requeue_callback
+ * Description:		Completion hook for poll-mode (callback == NULL)
+ *			internal requests that timed out while still owned
+ *			by the firmware.  The next controller reset drains
+ *			the busy queue and calls this, re-queueing the
+ *			request exactly once.  Without it,
+ *			tw_cli_drain_busy_queue's
+ *			"if (req->tw_cli_callback)" would drop poll-mode
+ *			requestes on the floor (leak), and the poll callers
+ *			unconditional re-queue links one request into two
+ *			queues (DF-4335).
+ */
+static TW_VOID
+tw_cli_poll_requeue_callback(struct tw_cli_req_context *req)
+{
+	if (req->data)
+		req->ctlr->internal_req_busy = TW_CL_FALSE;
+	tw_cli_req_q_insert_tail(req, TW_CLI_FREE_Q);
+}
+
+
+/*
  * Function name:	tw_cli_submit_and_poll_request
  * Description:		Sends down a firmware cmd, and waits for the completion
  *			in a tight loop.
@@ -1027,6 +1062,18 @@
 		if (req->data)
 			ctlr->internal_req_busy = TW_CL_FALSE;
 		tw_cli_req_q_insert_tail(req, TW_CLI_FREE_Q);
+	} else if (req->state == TW_CLI_REQ_STATE_BUSY) {
+		/*
+		 * Still owned by the firmware: the caller must NOT clean
+		 * up (see the ownership contract above).  Poll-mode
+		 * requests have no callback, so install a requeue hook
+		 * that the next reset's tw_cli_drain_busy_queue will
+		 * invoke exactly once.
+		 */
+		if (req->tw_cli_callback == TW_CL_NULL) {
+			req->error_code = TW_OSL_ETIMEDOUT;
+			req->tw_cli_callback = tw_cli_poll_requeue_callback;
+		}
 	}
 
 	return(TW_OSL_ETIMEDOUT);