LU-6541 utils: fix potential memory leaks with realloc()

The semantics of realloc are to return NULL when the allocation fails,
but won't free the original allocation:

"The realloc() function returns a pointer to the newly allocated memory,
which is suitably aligned for any built-in type and may be different
from ptr, or NULL if the request fails. If size was equal to 0, either
NULL or a pointer suitable to be passed to free() is returned. If
realloc() fails, the original block is left untouched; it is not freed
or moved."

Signed-off-by: Dmitry Eremin <dmitry.eremin@intel.com>
Change-Id: Ib348acad13c866a2904e023ac5822f23d13b38ca
Reviewed-on: http://review.whamcloud.com/14814
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Bob Glossman <bob.glossman@intel.com>
Reviewed-by: John L. Hammond <john.hammond@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/lustre/tests/multiop.c b/lustre/tests/multiop.c
index e42aeaa..1feb1ee 100644 (file)
--- a/lustre/tests/multiop.c
+++ b/lustre/tests/multiop.c
@@ -515,12 +515,15 @@ int main(int argc, char **argv)
                        if (len <= 0)
                                len = 1;
                        if (bufsize < len) {
-                               buf = realloc(buf, len + ALIGN_LEN);
-                               if (buf == NULL) {
+                               void *tmp;
+                               tmp = realloc(buf, len + ALIGN_LEN);
+                               if (tmp == NULL) {
+                                       free(buf);
                                        save_errno = errno;
                                        perror("allocating buf for read\n");
                                        exit(save_errno);
                                }
+                               buf = tmp;
                                bufsize = len;
                                buf_align = (char *)((long)(buf + ALIGN_LEN) &
                                                     ~ALIGN_LEN);
@@ -608,12 +611,15 @@ int main(int argc, char **argv)
                        if (len <= 0)
                                len = 1;
                        if (bufsize < len) {
-                               buf = realloc(buf, len + ALIGN_LEN);
-                               if (buf == NULL) {
+                               void *tmp;
+                               tmp = realloc(buf, len + ALIGN_LEN);
+                               if (tmp == NULL) {
+                                       free(buf);
                                        save_errno = errno;
                                        perror("allocating buf for write\n");
                                        exit(save_errno);
                                }
+                               buf = tmp;
                                bufsize = len;
                                buf_align = (char *)((long)(buf + ALIGN_LEN) &
                                                     ~ALIGN_LEN);

diff --git a/lustre/utils/gss/krb5_util.c b/lustre/utils/gss/krb5_util.c
index 000f7c3..2a71d47 100644 (file)
--- a/lustre/utils/gss/krb5_util.c
+++ b/lustre/utils/gss/krb5_util.c
@@ -900,15 +900,15 @@ gssd_get_krb5_machine_cred_list(char ***list)
        struct gssd_k5_kt_princ *ple;
 
        /* Assume failure */
-       retval = -1;
-       *list = (char **) NULL;
+       *list = NULL;
 
        /* Refresh machine credentials */
-       if ((retval = gssd_refresh_krb5_machine_creds())) {
+       retval = gssd_refresh_krb5_machine_creds();
+       if (retval)
                goto out;
-       }
 
-       if ((l = (char **) malloc(listsize * sizeof(char *))) == NULL) {
+       l = malloc(listsize * sizeof(char *));
+       if (l == NULL) {
                retval = ENOMEM;
                goto out;
        }
@@ -916,27 +916,33 @@ gssd_get_krb5_machine_cred_list(char ***list)
        for (ple = gssd_k5_kt_princ_list; ple; ple = ple->next) {
                if (ple->ccname) {
                        if (i + 1 > listsize) {
+                               void *tmp;
+
                                listsize += listinc;
-                               l = (char **)
-                                       realloc(l, listsize * sizeof(char *));
-                               if (l == NULL) {
+                               tmp = realloc(l, listsize * sizeof(char *));
+                               if (tmp == NULL) {
                                        retval = ENOMEM;
-                                       goto out;
+                                       goto out_free;
                                }
+                               l = tmp;
                        }
-                       if ((l[i++] = strdup(ple->ccname)) == NULL) {
+                       l[i] = strdup(ple->ccname);
+                       if (l[i++] == NULL) {
                                retval = ENOMEM;
-                               goto out;
+                               goto out_free;
                        }
                }
        }
        if (i > 0) {
                l[i] = NULL;
                *list = l;
-               retval = 0;
-               goto out;
+               return 0;
        }
-  out:
+out_free:
+       while (i > 0)
+               free(l[i--]);
+       free(l);
+out:
        return retval;
 }
 

diff --git a/lustre/utils/obd.c b/lustre/utils/obd.c
index 5381876..37d639c 100644 (file)
--- a/lustre/utils/obd.c
+++ b/lustre/utils/obd.c
@@ -3965,14 +3965,16 @@ static int get_array_idx(char *rule, char *format, int **array)
                 /* extract the 3 fields */
                 rc = sscanf(start, "%x-%x/%u", &lo, &hi, &step);
                 switch (rc) {
-                case 0: {
-                        return 0;
-                }
-                case 1: {
-                        array_sz++;
-                        *array = realloc(*array, array_sz * sizeof(int));
-                        if (*array == NULL)
-                                return 0;
+                case 0:
+                       goto err;
+               case 1: {
+                       void *tmp;
+
+                       array_sz++;
+                       tmp = realloc(*array, array_sz * sizeof(int));
+                       if (tmp == NULL)
+                               goto err;
+                       *array = tmp;
                         (*array)[array_idx] = lo;
                         array_idx++;
                         break;
@@ -3982,12 +3984,15 @@ static int get_array_idx(char *rule, char *format, int **array)
                         /* do not break to share code with case 3: */
                 }
                 case 3: {
-                        if ((hi < lo) || (step == 0))
-                                return 0;
-                        array_sz += (hi - lo) / step + 1;
-                        *array = realloc(*array, sizeof(int) * array_sz);
-                        if (*array == NULL)
-                                return 0;
+                       void *tmp;
+
+                       if ((hi < lo) || (step == 0))
+                               goto err;
+                       array_sz += (hi - lo) / step + 1;
+                       tmp = realloc(*array, array_sz * sizeof(int));
+                       if (tmp == NULL)
+                               goto err;
+                       *array = tmp;
                         for (i = lo; i <= hi; i+=step, array_idx++)
                                 (*array)[array_idx] = i;
                         break;
@@ -3999,6 +4004,12 @@ static int get_array_idx(char *rule, char *format, int **array)
 
         } while (ptr != NULL);
         return array_sz;
+err:
+       if (*array != NULL) {
+               free(*array);
+               *array = NULL;
+       }
+       return 0;
 }
 
 static int extract_fsname_poolname(char *arg, char *fsname, char *poolname)