LU-4629 gss: fix few issues found by Klocwork Insight tool

Array 'message_buf' of size 500 may use index value(s) -1

Object 'enc_key.data' was freed at line 164 after being freed
by calling 'free' at line 150. Also there are 3 similar errors
on line(s) 164.

Suspicious dereference of pointer 'vmsg' before NULL check at
line 187. Also there are 2 similar errors on line(s) 196, 205.

Suspicious dereference of pointer 'rmsg' before NULL check at
line 191. Also there are 2 similar errors on line(s) 200, 209.

Signed-off-by: Dmitry Eremin <dmitry.eremin@intel.com>
Change-Id: I50905ea99d904123df30ba7078b180b44b8a6e06
Reviewed-on: http://review.whamcloud.com/9274
Tested-by: Jenkins
Reviewed-by: John L. Hammond <john.hammond@intel.com>
Reviewed-by: James Simmons <uja.ornl@gmail.com>
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/lustre/ptlrpc/gss/gss_bulk.c b/lustre/ptlrpc/gss/gss_bulk.c
index 1e7596f..edc5928 100644 (file)
--- a/lustre/ptlrpc/gss/gss_bulk.c
+++ b/lustre/ptlrpc/gss/gss_bulk.c
@@ -183,31 +183,31 @@ int gss_cli_ctx_unwrap_bulk(struct ptlrpc_cli_ctx *ctx,
         switch (SPTLRPC_FLVR_SVC(req->rq_flvr.sf_rpc)) {
         case SPTLRPC_SVC_NULL:
                 vmsg = req->rq_repdata;
         switch (SPTLRPC_FLVR_SVC(req->rq_flvr.sf_rpc)) {
         case SPTLRPC_SVC_NULL:
                 vmsg = req->rq_repdata;

+               LASSERT(vmsg != NULL && vmsg->lm_bufcount >= 3);

                 voff = vmsg->lm_bufcount - 1;
                 voff = vmsg->lm_bufcount - 1;

-                LASSERT(vmsg && vmsg->lm_bufcount >= 3);

 
                 rmsg = req->rq_reqbuf;
 
                 rmsg = req->rq_reqbuf;

+               LASSERT(rmsg != NULL && rmsg->lm_bufcount >= 3);

                 roff = rmsg->lm_bufcount - 1; /* last segment */
                 roff = rmsg->lm_bufcount - 1; /* last segment */

-                LASSERT(rmsg && rmsg->lm_bufcount >= 3);

                 break;
         case SPTLRPC_SVC_AUTH:
         case SPTLRPC_SVC_INTG:
                 vmsg = req->rq_repdata;
                 break;
         case SPTLRPC_SVC_AUTH:
         case SPTLRPC_SVC_INTG:
                 vmsg = req->rq_repdata;

+               LASSERT(vmsg != NULL && vmsg->lm_bufcount >= 4);

                 voff = vmsg->lm_bufcount - 2;
                 voff = vmsg->lm_bufcount - 2;

-                LASSERT(vmsg && vmsg->lm_bufcount >= 4);

 
                 rmsg = req->rq_reqbuf;
 
                 rmsg = req->rq_reqbuf;

+               LASSERT(rmsg != NULL && rmsg->lm_bufcount >= 4);

                 roff = rmsg->lm_bufcount - 2; /* second last segment */
                 roff = rmsg->lm_bufcount - 2; /* second last segment */

-                LASSERT(rmsg && rmsg->lm_bufcount >= 4);

                 break;
         case SPTLRPC_SVC_PRIV:
                 vmsg = req->rq_repdata;
                 break;
         case SPTLRPC_SVC_PRIV:
                 vmsg = req->rq_repdata;

+               LASSERT(vmsg != NULL && vmsg->lm_bufcount >= 2);

                 voff = vmsg->lm_bufcount - 1;
                 voff = vmsg->lm_bufcount - 1;

-                LASSERT(vmsg && vmsg->lm_bufcount >= 2);

 
                 rmsg = req->rq_clrbuf;
 
                 rmsg = req->rq_clrbuf;

+               LASSERT(rmsg != NULL && rmsg->lm_bufcount >= 2);

                 roff = rmsg->lm_bufcount - 1; /* last segment */
                 roff = rmsg->lm_bufcount - 1; /* last segment */

-                LASSERT(rmsg && rmsg->lm_bufcount >= 2);

                 break;
         default:
                 LBUG();
                 break;
         default:
                 LBUG();

diff --git a/lustre/utils/gss/context_lucid.c b/lustre/utils/gss/context_lucid.c
index e95d11e..bc303b2 100644 (file)
--- a/lustre/utils/gss/context_lucid.c
+++ b/lustre/utils/gss/context_lucid.c
@@ -90,7 +90,6 @@ prepare_krb5_rfc1964_buffer(gss_krb5_lucid_context_v1_t *lctx,
        gss_krb5_lucid_key_t enc_key;
        int i;
        char *skd, *dkd;
        gss_krb5_lucid_key_t enc_key;
        int i;
        char *skd, *dkd;

-       gss_buffer_desc fakeoid;

 
        /*
         * The new Kerberos interface to get the gss context
 
        /*
         * The new Kerberos interface to get the gss context


@@ -100,7 +99,6 @@ prepare_krb5_rfc1964_buffer(gss_krb5_lucid_context_v1_t *lctx,
         * interface to the kernel.
         */
        memset(&enc_key, 0, sizeof(enc_key));
         * interface to the kernel.
         */
        memset(&enc_key, 0, sizeof(enc_key));

-       memset(&fakeoid, 0, sizeof(fakeoid));

 
        if (!(buf->value = calloc(1, MAX_CTX_LEN)))
                goto out_err;
 
        if (!(buf->value = calloc(1, MAX_CTX_LEN)))
                goto out_err;


@@ -146,20 +144,20 @@ prepare_krb5_rfc1964_buffer(gss_krb5_lucid_context_v1_t *lctx,
        dkd = (char *) enc_key.data;
        for (i = 0; i < enc_key.length; i++)
                dkd[i] = skd[i] ^ 0xf0;
        dkd = (char *) enc_key.data;
        for (i = 0; i < enc_key.length; i++)
                dkd[i] = skd[i] ^ 0xf0;

-       if (write_lucid_keyblock(&p, end, &enc_key)) {
-               free(enc_key.data);
+       if (write_lucid_keyblock(&p, end, &enc_key))

                goto out_err;
                goto out_err;

-       }
-       free(enc_key.data);
-

        if (write_lucid_keyblock(&p, end, &lctx->rfc1964_kd.ctx_key))
                goto out_err;
        if (write_lucid_keyblock(&p, end, &lctx->rfc1964_kd.ctx_key))
                goto out_err;

+       free(enc_key.data);

 
        buf->length = p - (char *)buf->value;
        return 0;
 out_err:
        printerr(0, "ERROR: failed serializing krb5 context for kernel\n");
 
        buf->length = p - (char *)buf->value;
        return 0;
 out_err:
        printerr(0, "ERROR: failed serializing krb5 context for kernel\n");

-       if (buf->value) free(buf->value);
+       if (buf->value) {
+               free(buf->value);
+               buf->value = NULL;
+       }

        buf->length = 0;
        if (enc_key.data) free(enc_key.data);
        return -1;
        buf->length = 0;
        if (enc_key.data) free(enc_key.data);
        return -1;

diff --git a/lustre/utils/gss/err_util.c b/lustre/utils/gss/err_util.c
index 376fb59..2629444 100644 (file)
--- a/lustre/utils/gss/err_util.c
+++ b/lustre/utils/gss/err_util.c
@@ -81,7 +81,8 @@ void printerr(int priority, char *format, ...)
                strcat(message_buf, "...\n");
                goto printit;
        }
                strcat(message_buf, "...\n");
                goto printit;
        }

-       if (message_buf[strlen(message_buf) - 1] == '\n')
+       buf_used = strlen(message_buf);
+       if (buf_used > 0 && message_buf[buf_used - 1] == '\n')

                goto printit;
        return;
 printit:
                goto printit;
        return;
 printit: