ceph-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] libceph: don't pass result into ac->ops->handle_reply()
@ 2021-06-23 15:12 Ilya Dryomov
  2021-06-24 16:44 ` Jeff Layton
  0 siblings, 1 reply; 2+ messages in thread
From: Ilya Dryomov @ 2021-06-23 15:12 UTC (permalink / raw)
  To: ceph-devel; +Cc: Sage Weil

There is no result to pass in msgr2 case because authentication
failures are reported through auth_bad_method frame and in MAuth
case an error is returned immediately.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---
 include/linux/ceph/auth.h |  2 +-
 net/ceph/auth.c           | 15 ++++++++++-----
 net/ceph/auth_none.c      |  4 ++--
 net/ceph/auth_x.c         |  6 ++----
 4 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/include/linux/ceph/auth.h b/include/linux/ceph/auth.h
index 71b5d481c653..39425e2f7cb2 100644
--- a/include/linux/ceph/auth.h
+++ b/include/linux/ceph/auth.h
@@ -50,7 +50,7 @@ struct ceph_auth_client_ops {
 	 * another request.
 	 */
 	int (*build_request)(struct ceph_auth_client *ac, void *buf, void *end);
-	int (*handle_reply)(struct ceph_auth_client *ac, int result,
+	int (*handle_reply)(struct ceph_auth_client *ac,
 			    void *buf, void *end, u8 *session_key,
 			    int *session_key_len, u8 *con_secret,
 			    int *con_secret_len);
diff --git a/net/ceph/auth.c b/net/ceph/auth.c
index b824a48a4c47..d07c8cd6cb46 100644
--- a/net/ceph/auth.c
+++ b/net/ceph/auth.c
@@ -255,14 +255,19 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac,
 		ac->negotiating = false;
 	}
 
-	ret = ac->ops->handle_reply(ac, result, payload, payload_end,
+	if (result) {
+		pr_err("auth protocol '%s' mauth authentication failed: %d\n",
+		       ceph_auth_proto_name(ac->protocol), result);
+		ret = result;
+		goto out;
+	}
+
+	ret = ac->ops->handle_reply(ac, payload, payload_end,
 				    NULL, NULL, NULL, NULL);
 	if (ret == -EAGAIN) {
 		ret = build_request(ac, true, reply_buf, reply_len);
 		goto out;
 	} else if (ret) {
-		pr_err("auth protocol '%s' mauth authentication failed: %d\n",
-		       ceph_auth_proto_name(ac->protocol), result);
 		goto out;
 	}
 
@@ -475,7 +480,7 @@ int ceph_auth_handle_reply_more(struct ceph_auth_client *ac, void *reply,
 	int ret;
 
 	mutex_lock(&ac->mutex);
-	ret = ac->ops->handle_reply(ac, 0, reply, reply + reply_len,
+	ret = ac->ops->handle_reply(ac, reply, reply + reply_len,
 				    NULL, NULL, NULL, NULL);
 	if (ret == -EAGAIN)
 		ret = build_request(ac, false, buf, buf_len);
@@ -493,7 +498,7 @@ int ceph_auth_handle_reply_done(struct ceph_auth_client *ac,
 	int ret;
 
 	mutex_lock(&ac->mutex);
-	ret = ac->ops->handle_reply(ac, 0, reply, reply + reply_len,
+	ret = ac->ops->handle_reply(ac, reply, reply + reply_len,
 				    session_key, session_key_len,
 				    con_secret, con_secret_len);
 	if (!ret)
diff --git a/net/ceph/auth_none.c b/net/ceph/auth_none.c
index dbf22df10a85..533a2d85dbb9 100644
--- a/net/ceph/auth_none.c
+++ b/net/ceph/auth_none.c
@@ -69,7 +69,7 @@ static int build_request(struct ceph_auth_client *ac, void *buf, void *end)
  * the generic auth code decode the global_id, and we carry no actual
  * authenticate state, so nothing happens here.
  */
-static int handle_reply(struct ceph_auth_client *ac, int result,
+static int handle_reply(struct ceph_auth_client *ac,
 			void *buf, void *end, u8 *session_key,
 			int *session_key_len, u8 *con_secret,
 			int *con_secret_len)
@@ -77,7 +77,7 @@ static int handle_reply(struct ceph_auth_client *ac, int result,
 	struct ceph_auth_none_info *xi = ac->private;
 
 	xi->starting = false;
-	return result;
+	return 0;
 }
 
 static void ceph_auth_none_destroy_authorizer(struct ceph_authorizer *a)
diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c
index 79641c4afee9..cab99c5581b0 100644
--- a/net/ceph/auth_x.c
+++ b/net/ceph/auth_x.c
@@ -661,7 +661,7 @@ static int handle_auth_session_key(struct ceph_auth_client *ac,
 	return -EINVAL;
 }
 
-static int ceph_x_handle_reply(struct ceph_auth_client *ac, int result,
+static int ceph_x_handle_reply(struct ceph_auth_client *ac,
 			       void *buf, void *end,
 			       u8 *session_key, int *session_key_len,
 			       u8 *con_secret, int *con_secret_len)
@@ -669,13 +669,11 @@ static int ceph_x_handle_reply(struct ceph_auth_client *ac, int result,
 	struct ceph_x_info *xi = ac->private;
 	struct ceph_x_ticket_handler *th;
 	int len = end - buf;
+	int result;
 	void *p;
 	int op;
 	int ret;
 
-	if (result)
-		return result;  /* XXX hmm? */
-
 	if (xi->starting) {
 		/* it's a hello */
 		struct ceph_x_server_challenge *sc = buf;
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: [PATCH] libceph: don't pass result into ac->ops->handle_reply()
  2021-06-23 15:12 [PATCH] libceph: don't pass result into ac->ops->handle_reply() Ilya Dryomov
@ 2021-06-24 16:44 ` Jeff Layton
  0 siblings, 0 replies; 2+ messages in thread
From: Jeff Layton @ 2021-06-24 16:44 UTC (permalink / raw)
  To: Ilya Dryomov, ceph-devel; +Cc: Sage Weil

On Wed, 2021-06-23 at 17:12 +0200, Ilya Dryomov wrote:
> There is no result to pass in msgr2 case because authentication
> failures are reported through auth_bad_method frame and in MAuth
> case an error is returned immediately.
> 
> Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
> ---
>  include/linux/ceph/auth.h |  2 +-
>  net/ceph/auth.c           | 15 ++++++++++-----
>  net/ceph/auth_none.c      |  4 ++--
>  net/ceph/auth_x.c         |  6 ++----
>  4 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/ceph/auth.h b/include/linux/ceph/auth.h
> index 71b5d481c653..39425e2f7cb2 100644
> --- a/include/linux/ceph/auth.h
> +++ b/include/linux/ceph/auth.h
> @@ -50,7 +50,7 @@ struct ceph_auth_client_ops {
>  	 * another request.
>  	 */
>  	int (*build_request)(struct ceph_auth_client *ac, void *buf, void *end);
> -	int (*handle_reply)(struct ceph_auth_client *ac, int result,
> +	int (*handle_reply)(struct ceph_auth_client *ac,
>  			    void *buf, void *end, u8 *session_key,
>  			    int *session_key_len, u8 *con_secret,
>  			    int *con_secret_len);
> diff --git a/net/ceph/auth.c b/net/ceph/auth.c
> index b824a48a4c47..d07c8cd6cb46 100644
> --- a/net/ceph/auth.c
> +++ b/net/ceph/auth.c
> @@ -255,14 +255,19 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac,
>  		ac->negotiating = false;
>  	}
>  
> -	ret = ac->ops->handle_reply(ac, result, payload, payload_end,
> +	if (result) {
> +		pr_err("auth protocol '%s' mauth authentication failed: %d\n",
> +		       ceph_auth_proto_name(ac->protocol), result);
> +		ret = result;
> +		goto out;
> +	}
> +
> +	ret = ac->ops->handle_reply(ac, payload, payload_end,
>  				    NULL, NULL, NULL, NULL);
>  	if (ret == -EAGAIN) {
>  		ret = build_request(ac, true, reply_buf, reply_len);
>  		goto out;
>  	} else if (ret) {
> -		pr_err("auth protocol '%s' mauth authentication failed: %d\n",
> -		       ceph_auth_proto_name(ac->protocol), result);
>  		goto out;
>  	}
>  
> @@ -475,7 +480,7 @@ int ceph_auth_handle_reply_more(struct ceph_auth_client *ac, void *reply,
>  	int ret;
>  
>  	mutex_lock(&ac->mutex);
> -	ret = ac->ops->handle_reply(ac, 0, reply, reply + reply_len,
> +	ret = ac->ops->handle_reply(ac, reply, reply + reply_len,
>  				    NULL, NULL, NULL, NULL);
>  	if (ret == -EAGAIN)
>  		ret = build_request(ac, false, buf, buf_len);
> @@ -493,7 +498,7 @@ int ceph_auth_handle_reply_done(struct ceph_auth_client *ac,
>  	int ret;
>  
>  	mutex_lock(&ac->mutex);
> -	ret = ac->ops->handle_reply(ac, 0, reply, reply + reply_len,
> +	ret = ac->ops->handle_reply(ac, reply, reply + reply_len,
>  				    session_key, session_key_len,
>  				    con_secret, con_secret_len);
>  	if (!ret)
> diff --git a/net/ceph/auth_none.c b/net/ceph/auth_none.c
> index dbf22df10a85..533a2d85dbb9 100644
> --- a/net/ceph/auth_none.c
> +++ b/net/ceph/auth_none.c
> @@ -69,7 +69,7 @@ static int build_request(struct ceph_auth_client *ac, void *buf, void *end)
>   * the generic auth code decode the global_id, and we carry no actual
>   * authenticate state, so nothing happens here.
>   */
> -static int handle_reply(struct ceph_auth_client *ac, int result,
> +static int handle_reply(struct ceph_auth_client *ac,
>  			void *buf, void *end, u8 *session_key,
>  			int *session_key_len, u8 *con_secret,
>  			int *con_secret_len)
> @@ -77,7 +77,7 @@ static int handle_reply(struct ceph_auth_client *ac, int result,
>  	struct ceph_auth_none_info *xi = ac->private;
>  
>  	xi->starting = false;
> -	return result;
> +	return 0;
>  }
>  
>  static void ceph_auth_none_destroy_authorizer(struct ceph_authorizer *a)
> diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c
> index 79641c4afee9..cab99c5581b0 100644
> --- a/net/ceph/auth_x.c
> +++ b/net/ceph/auth_x.c
> @@ -661,7 +661,7 @@ static int handle_auth_session_key(struct ceph_auth_client *ac,
>  	return -EINVAL;
>  }
>  
> -static int ceph_x_handle_reply(struct ceph_auth_client *ac, int result,
> +static int ceph_x_handle_reply(struct ceph_auth_client *ac,
>  			       void *buf, void *end,
>  			       u8 *session_key, int *session_key_len,
>  			       u8 *con_secret, int *con_secret_len)
> @@ -669,13 +669,11 @@ static int ceph_x_handle_reply(struct ceph_auth_client *ac, int result,
>  	struct ceph_x_info *xi = ac->private;
>  	struct ceph_x_ticket_handler *th;
>  	int len = end - buf;
> +	int result;
>  	void *p;
>  	int op;
>  	int ret;
>  
> -	if (result)
> -		return result;  /* XXX hmm? */
> -
>  	if (xi->starting) {
>  		/* it's a hello */
>  		struct ceph_x_server_challenge *sc = buf;

Nice cleanup.

Reviewed-by: Jeff Layton <jlayton@kernel.org>


^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-06-24 16:44 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-23 15:12 [PATCH] libceph: don't pass result into ac->ops->handle_reply() Ilya Dryomov
2021-06-24 16:44 ` Jeff Layton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).