> On Oct 26, 2015, at 20:03, Ilya Dryomov <idryomov@xxxxxxxxx> wrote:
>
> Commit ae385eaf24dc ("libceph: store session key in cephx authorizer")
> introduced ceph_x_authorizer::session_key, but didn't update all the
> exit/error paths. Introduce ceph_x_authorizer_cleanup() to encapsulate
> ceph_x_authorizer cleanup and switch to it. This fixes ceph_x_destroy(),
> which currently always leaks key and ceph_x_build_authorizer() error
> paths.
>
> Cc: Yan, Zheng <zyan@xxxxxxxxxx>
> Signed-off-by: Ilya Dryomov <idryomov@xxxxxxxxx>
> ---
> net/ceph/auth_x.c | 28 +++++++++++++++++-----------
> net/ceph/crypto.h | 4 +++-
> 2 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c
> index ba6eb17226da..65054fd31b97 100644
> --- a/net/ceph/auth_x.c
> +++ b/net/ceph/auth_x.c
> @@ -279,6 +279,15 @@ bad:
> return -EINVAL;
> }
>
> +static void ceph_x_authorizer_cleanup(struct ceph_x_authorizer *au)
> +{
> + ceph_crypto_key_destroy(&au->session_key);
> + if (au->buf) {
> + ceph_buffer_put(au->buf);
> + au->buf = NULL;
> + }
> +}
> +
> static int ceph_x_build_authorizer(struct ceph_auth_client *ac,
> struct ceph_x_ticket_handler *th,
> struct ceph_x_authorizer *au)
> @@ -297,7 +306,7 @@ static int ceph_x_build_authorizer(struct ceph_auth_client *ac,
> ceph_crypto_key_destroy(&au->session_key);
> ret = ceph_crypto_key_clone(&au->session_key, &th->session_key);
> if (ret)
> - return ret;
> + goto out_au;
>
> maxlen = sizeof(*msg_a) + sizeof(msg_b) +
> ceph_x_encrypt_buflen(ticket_blob_len);
> @@ -309,8 +318,8 @@ static int ceph_x_build_authorizer(struct ceph_auth_client *ac,
> if (!au->buf) {
> au->buf = ceph_buffer_new(maxlen, GFP_NOFS);
> if (!au->buf) {
> - ceph_crypto_key_destroy(&au->session_key);
> - return -ENOMEM;
> + ret = -ENOMEM;
> + goto out_au;
> }
> }
> au->service = th->service;
> @@ -340,7 +349,7 @@ static int ceph_x_build_authorizer(struct ceph_auth_client *ac,
> ret = ceph_x_encrypt(&au->session_key, &msg_b, sizeof(msg_b),
> p, end - p);
> if (ret < 0)
> - goto out_buf;
> + goto out_au;
> p += ret;
> au->buf->vec.iov_len = p - au->buf->vec.iov_base;
> dout(" built authorizer nonce %llx len %d\n", au->nonce,
> @@ -348,9 +357,8 @@ static int ceph_x_build_authorizer(struct ceph_auth_client *ac,
> BUG_ON(au->buf->vec.iov_len > maxlen);
> return 0;
>
> -out_buf:
> - ceph_buffer_put(au->buf);
> - au->buf = NULL;
> +out_au:
> + ceph_x_authorizer_cleanup(au);
> return ret;
> }
>
> @@ -624,8 +632,7 @@ static void ceph_x_destroy_authorizer(struct ceph_auth_client *ac,
> {
> struct ceph_x_authorizer *au = (void *)a;
>
> - ceph_crypto_key_destroy(&au->session_key);
> - ceph_buffer_put(au->buf);
> + ceph_x_authorizer_cleanup(au);
> kfree(au);
> }
>
> @@ -653,8 +660,7 @@ static void ceph_x_destroy(struct ceph_auth_client *ac)
> remove_ticket_handler(ac, th);
> }
>
> - if (xi->auth_authorizer.buf)
> - ceph_buffer_put(xi->auth_authorizer.buf);
> + ceph_x_authorizer_cleanup(&xi->auth_authorizer);
>
> kfree(ac->private);
> ac->private = NULL;
> diff --git a/net/ceph/crypto.h b/net/ceph/crypto.h
> index d1498224c49d..2e9cab09f37b 100644
> --- a/net/ceph/crypto.h
> +++ b/net/ceph/crypto.h
> @@ -16,8 +16,10 @@ struct ceph_crypto_key {
>
> static inline void ceph_crypto_key_destroy(struct ceph_crypto_key *key)
> {
> - if (key)
> + if (key) {
> kfree(key->key);
> + key->key = NULL;
> + }
> }
>
> int ceph_crypto_key_clone(struct ceph_crypto_key *dst,
> —
> 2.4.3
>
Reviewed-by: Yan, Zheng <zyan@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
