[Devel] [PATCH VZ10 v2] fs/fuse kio: track pending kRPC connect via state machine, not a pointer

[Liu Kui]

Rework the previous fix ("fs/fuse kio: fix kRPC connect issues") to not
require the new struct pcs_krpc member "connect_req", so the fix can be
shipped as a livepatch: struct pcs_krpc objects are long-lived, and a
patched kernel would dereference the new member on objects allocated
before the livepatch was loaded, reading unallocated slab space.

Both things connect_req was tracking are already derivable from the
existing state machine once PCS_KRPC_STATE_CONNECT is made to mean
exactly "a connect req is in flight":

 - krpc_connect_done() settles a failed connect back to UNCONN instead
   of leaving the state in CONNECT forever;

 - pcs_krpc_abort() no longer resets CONNECT to UNCONN: the req is
   still in flight, and only its completion settles the state.  It
   advances gen instead, disowning the pending req: when the req
   completes, krpc_connect_done() settles the state to UNCONN without
   committing the dead session, even if the late connect succeeded;

 - pcs_krpc_connect() proceeds only from UNCONN or ABORTED, refusing
   new connects (-EPERM) while a req is in flight - at most one connect
   req exists at a time, same as with the connect_req check;

 - pcs_krpc_poll() reports EPOLLERR on UNCONN: poll bails out earlier
   unless ctx->gen == krpc->gen, and the current session can only be in
   UNCONN if its connect failed, which is what the (CONNECT && !connect_req)
   test used to detect.

Within CONNECT the pending req carries the current gen unless the
session was aborted, so a gen mismatch in krpc_connect_done() reliably
identifies a disowned req.

https://virtuozzo.atlassian.net/browse/VSTOR-135626

Signed-off-by: Liu Kui <kui.liu at virtuozzo.com>
---
 fs/fuse/kio/pcs/pcs_krpc.c | 63 ++++++++++++++++++++++++++++----------
 fs/fuse/kio/pcs/pcs_krpc.h |  2 --
 2 files changed, 46 insertions(+), 19 deletions(-)

diff --git a/fs/fuse/kio/pcs/pcs_krpc.c b/fs/fuse/kio/pcs/pcs_krpc.c
index 0930fb4adf12..9d534b037cd1 100644
--- a/fs/fuse/kio/pcs/pcs_krpc.c
+++ b/fs/fuse/kio/pcs/pcs_krpc.c
@@ -787,8 +787,15 @@ static int pcs_krpc_abort(struct pcs_krpc *krpc)
 	spin_lock(&krpc->lock);
 
 	if (krpc->state != PCS_KRPC_STATE_CONNECTED) {
+		/*
+		 * A pending connect req stays in flight and the state stays
+		 * CONNECT, refusing new connects until the req completes.
+		 * Advancing gen disowns the req, so that a late completion
+		 * settles the state to UNCONN in krpc_connect_done() instead
+		 * of committing this dead session on success.
+		 */
 		if (krpc->state == PCS_KRPC_STATE_CONNECT)
-			krpc->state = PCS_KRPC_STATE_UNCONN;
+			krpc->gen++;
 		spin_unlock(&krpc->lock);
 		return 0;
 	}
@@ -949,15 +956,15 @@ static __poll_t pcs_krpc_poll(struct file *file, poll_table *wait)
 
 	poll_wait(file, &krpc->poll_wait, wait);
 
-	if (unlikely(ctx->gen != krpc->gen)) {
-		pollflags |= EPOLLERR;
-		return pollflags;
-	}
-
 	spin_lock(&krpc->lock);
 
-	if (krpc->state == PCS_KRPC_STATE_ABORTED ||
-	    (krpc->state == PCS_KRPC_STATE_CONNECT && !krpc->connect_req)) {
+	/*
+	 * when ctx->gen == krpc->gen, UNCONN here can only mean its
+	 * connect attempt has failed (see krpc_connect_done()).
+	 */
+	if (ctx->gen != krpc->gen ||
+	    krpc->state == PCS_KRPC_STATE_ABORTED ||
+	    krpc->state == PCS_KRPC_STATE_UNCONN) {
 		pollflags |= EPOLLERR;
 	} else if (krpc->state == PCS_KRPC_STATE_CONNECTED) {
 		pollflags |= EPOLLOUT;
@@ -1047,7 +1054,6 @@ int pcs_krpc_create(struct pcs_krpc_set *krpcs, PCS_NODE_ID_T *id,
 	krpc->gen = 0;
 	krpc->state = PCS_KRPC_STATE_UNCONN;
 	krpc->cs = NULL;
-	krpc->connect_req = NULL;
 
 	krpc->rpc = pcs_rpc_clnt_create(&cc_from_krpcset(krpcs)->eng, id, addr, cs_flags);
 	if (!krpc->rpc) {
@@ -1099,10 +1105,20 @@ static void krpc_connect_done(struct pcs_msg *msg)
 	}
 
 	spin_lock(&krpc->lock);
-	if (krpc->connect_req == req)
-		krpc->connect_req = NULL;
-	/* from a stale session, do nothing  */
-	if (req->gen != krpc->gen || krpc->state != PCS_KRPC_STATE_CONNECT) {
+	/* the session was aborted or destroyed, nothing to settle */
+	if (krpc->state != PCS_KRPC_STATE_CONNECT) {
+		spin_unlock(&krpc->lock);
+		goto out;
+	}
+
+	if (req->gen != krpc->gen) {
+		/*
+		 * The session that started this connect was aborted while the
+		 * req was in flight (pcs_krpc_abort() advanced gen): settle
+		 * the state so a new connect is allowed, but never commit the
+		 * dead session, even on success.
+		 */
+		krpc->state = PCS_KRPC_STATE_UNCONN;
 		spin_unlock(&krpc->lock);
 		goto out;
 	}
@@ -1110,6 +1126,14 @@ static void krpc_connect_done(struct pcs_msg *msg)
 	if (!pcs_if_error(&msg->error)) {
 		krpc->state = PCS_KRPC_STATE_CONNECTED;
 		pollflags = EPOLLOUT;
+	} else {
+		/*
+		 * Connect failed: settle back to UNCONN so that a new connect
+		 * is allowed again, and report the failure to poll().  Since
+		 * gen is unchanged, the current session's poll sees UNCONN
+		 * and returns EPOLLERR.
+		 */
+		krpc->state = PCS_KRPC_STATE_UNCONN;
 	}
 	spin_unlock(&krpc->lock);
 
@@ -1167,9 +1191,15 @@ int pcs_krpc_connect(struct pcs_krpc_set *krpcs, PCS_NODE_ID_T *id)
 	}
 
 	spin_lock(&krpc->lock);
-	if (krpc->state == PCS_KRPC_STATE_CONNECTED ||
-	    krpc->state == PCS_KRPC_STATE_DESTROYED ||
-	    krpc->connect_req) {
+	/*
+	 * A connect is allowed only when there is neither an established
+	 * session nor a connect req in flight (CONNECT state, see
+	 * krpc_connect_done()).  This limits connect reqs to one at a time:
+	 * if userspace gave up on a connect and retries, the new connect
+	 * fails immediately until the old req completes.
+	 */
+	if (krpc->state != PCS_KRPC_STATE_UNCONN &&
+	    krpc->state != PCS_KRPC_STATE_ABORTED) {
 		spin_unlock(&krpc->lock);
 		err = -EPERM;
 		/* fput() drops ctx and its krpc reference via pcs_krpc_release() */
@@ -1181,7 +1211,6 @@ int pcs_krpc_connect(struct pcs_krpc_set *krpcs, PCS_NODE_ID_T *id)
 	connect_req->gen = krpc->gen;
 	connect_req->krpc = pcs_krpc_get(krpc);
 	krpc->state = PCS_KRPC_STATE_CONNECT;
-	krpc->connect_req = connect_req;
 	spin_unlock(&krpc->lock);
 
 	/* publish the fd only after the connect is committed */
diff --git a/fs/fuse/kio/pcs/pcs_krpc.h b/fs/fuse/kio/pcs/pcs_krpc.h
index 803376895cc5..96b4815abf86 100644
--- a/fs/fuse/kio/pcs/pcs_krpc.h
+++ b/fs/fuse/kio/pcs/pcs_krpc.h
@@ -84,8 +84,6 @@ struct pcs_krpc {
 	/** Wait queue head for poll */
 	wait_queue_head_t		poll_wait;
 	struct pcs_cs			*cs;
-
-	struct krpc_connect_req *connect_req;
 };
 
 struct pcs_krpc_context {
-- 
2.50.1 (Apple Git-155)