From dbe0984d8a84e0ca0e0d42f84d856c6f1d2a5134 Mon Sep 17 00:00:00 2001 From: Kotresh HR Date: Thu, 17 May 2018 05:28:24 -0400 Subject: rpc: Don't reset auth_value in disconnect The auth_value was being reset to AUTH_GLUSTERFS_v2 during rpc disconnect. It shoud not be reset. The disconnect during portmap request can race with handshake. If handshake happens first and disconnect later, auth_value would set to default value and it never sets back to actual auth_value Back port of > BUG: 1579276 > Change-Id: Ib46c9e01a97f6defb3fd1e0423fdb4b899b4a361 > Signed-off-by: Kotresh HR (cherry picked from commit 2d5b179d1a545f5b7ae8b1b2274769691dd3468f) fixes: bz#1582063 Change-Id: Ib46c9e01a97f6defb3fd1e0423fdb4b899b4a361 Signed-off-by: Kotresh HR --- rpc/rpc-lib/src/rpc-clnt.c | 60 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/rpc/rpc-lib/src/rpc-clnt.c b/rpc/rpc-lib/src/rpc-clnt.c index 6a00e7f3408..0471a268c66 100644 --- a/rpc/rpc-lib/src/rpc-clnt.c +++ b/rpc/rpc-lib/src/rpc-clnt.c @@ -935,10 +935,20 @@ rpc_clnt_notify (rpc_transport_t *trans, void *mydata, case RPC_TRANSPORT_DISCONNECT: { rpc_clnt_handle_disconnect (clnt, conn); - /* reset auth_type to use v2 (if its not auth-null), it - would be set to appropriate type in handshake again */ - if (clnt->auth_value) - clnt->auth_value = AUTH_GLUSTERFS_v2; + /* The auth_value was being reset to AUTH_GLUSTERFS_v2. + * if (clnt->auth_value) + * clnt->auth_value = AUTH_GLUSTERFS_v2; + * It should not be reset here. The disconnect during + * portmap request can race with handshake. If handshake + * happens first and disconnect later, auth_value would set + * to default value and it never sets back to actual auth_value + * supported by server. But it's important to set to lower + * version supported in the case where the server downgrades. + * So moving this code to RPC_TRANSPORT_CONNECT. Note that + * CONNECT cannot race with handshake as by nature it is + * serialized with handhake. An handshake can happen only + * on a connected transport and hence its strictly serialized. + */ break; } @@ -999,6 +1009,12 @@ rpc_clnt_notify (rpc_transport_t *trans, void *mydata, * for just one successful attempt */ conn->config.remote_port = 0; + /* auth value should be set to lower version available + * and will be set to appropriate version supported by + * server after the handshake. + */ + if (clnt->auth_value) + clnt->auth_value = AUTH_GLUSTERFS_v2; if (clnt->notifyfn) ret = clnt->notifyfn (clnt, clnt->mydata, RPC_CLNT_CONNECT, NULL); @@ -1952,10 +1968,20 @@ rpc_clnt_disable (struct rpc_clnt *rpc) if (trans) { rpc_transport_disconnect (trans, _gf_true); - /* reset auth_type to use v2 (if its not auth-null), it - would be set to appropriate type in handshake again */ - if (rpc->auth_value) - rpc->auth_value = AUTH_GLUSTERFS_v2; + /* The auth_value was being reset to AUTH_GLUSTERFS_v2. + * if (clnt->auth_value) + * clnt->auth_value = AUTH_GLUSTERFS_v2; + * It should not be reset here. The disconnect during + * portmap request can race with handshake. If handshake + * happens first and disconnect later, auth_value would set + * to default value and it never sets back to actual auth_value + * supported by server. But it's important to set to lower + * version supported in the case where the server downgrades. + * So moving this code to RPC_TRANSPORT_CONNECT. Note that + * CONNECT cannot race with handshake as by nature it is + * serialized with handhake. An handshake can happen only + * on a connected transport and hence its strictly serialized. + */ } if (unref) @@ -2015,10 +2041,20 @@ rpc_clnt_disconnect (struct rpc_clnt *rpc) if (trans) { rpc_transport_disconnect (trans, _gf_true); - /* reset auth_type to use v2 (if its not auth-null), it - would be set to appropriate type in handshake again */ - if (rpc->auth_value) - rpc->auth_value = AUTH_GLUSTERFS_v2; + /* The auth_value was being reset to AUTH_GLUSTERFS_v2. + * if (clnt->auth_value) + * clnt->auth_value = AUTH_GLUSTERFS_v2; + * It should not be reset here. The disconnect during + * portmap request can race with handshake. If handshake + * happens first and disconnect later, auth_value would set + * to default value and it never sets back to actual auth_value + * supported by server. But it's important to set to lower + * version supported in the case where the server downgrades. + * So moving this code to RPC_TRANSPORT_CONNECT. Note that + * CONNECT cannot race with handshake as by nature it is + * serialized with handhake. An handshake can happen only + * on a connected transport and hence its strictly serialized. + */ } if (unref) rpc_clnt_unref (rpc); -- cgit