LU-11233 utils: fix double-free of params fields

Call find_param_fini() on error so that the params are not leaked
during initialization if there is an intermediate error.

Zero out the parameters as they are freed, so if find_param_fini()
is called multiple times (as it is in some error paths) it does
not corrupt the heap by double freeing pointers.  This can be hit
by calling "lfs getstripe -m" on multiple pathnames, some of which
do not exist.

Test-Parameters: trivial
Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
Change-Id: Ie0d7e9ee134deb0633af2f8052b8a458333ebbe5
Reviewed-on: https://review.whamcloud.com/34711
Tested-by: Jenkins
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Wang Shilong <wshilong@ddn.com>
Reviewed-by: James Simmons <uja.ornl@yahoo.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index ff1e175..e351eee 100755 (executable)
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -1573,6 +1573,16 @@ test_27g() {
 }
 run_test 27g "$LFS getstripe with no objects"
 
 }
 run_test 27g "$LFS getstripe with no objects"
 

+test_27ga() {
+       test_mkdir $DIR/$tdir
+       touch $DIR/$tdir/$tfile || error "touch failed"
+       ln -s bogus $DIR/$tdir/$tfile.2 || error "ln failed"
+       $LFS getstripe -m $DIR/$tdir/$tfile $DIR/$tdir/$tfile.2
+       local rc=$?
+       (( rc == 2 )) || error "getstripe did not return ENOENT"
+}
+run_test 27ga "$LFS getstripe with missing file (should return error)"
+

 test_27i() {
        test_mkdir $DIR/$tdir
        touch $DIR/$tdir/$tfile || error "touch failed"
 test_27i() {
        test_mkdir $DIR/$tdir
        touch $DIR/$tdir/$tfile || error "touch failed"

diff --git a/lustre/utils/liblustreapi.c b/lustre/utils/liblustreapi.c
index c7a8281..c5098fd 100644 (file)
--- a/lustre/utils/liblustreapi.c
+++ b/lustre/utils/liblustreapi.c
@@ -1650,6 +1650,27 @@ typedef int (semantic_func_t)(char *path, DIR *parent, DIR **d,
 
 #define OBD_NOT_FOUND           (-1)
 
 
 #define OBD_NOT_FOUND           (-1)
 

+static void find_param_fini(struct find_param *param)
+{
+       if (param->fp_migrate)
+               return;
+
+       if (param->fp_obd_indexes) {
+               free(param->fp_obd_indexes);
+               param->fp_obd_indexes = NULL;
+       }
+
+       if (param->fp_lmd) {
+               free(param->fp_lmd);
+               param->fp_lmd = NULL;
+       }
+
+       if (param->fp_lmv_md) {
+               free(param->fp_lmv_md);
+               param->fp_lmv_md = NULL;
+       }
+}
+

 static int common_param_init(struct find_param *param, char *path)
 {
        int lum_size = get_mds_md_size(path);
 static int common_param_init(struct find_param *param, char *path)
 {
        int lum_size = get_mds_md_size(path);


@@ -1682,6 +1703,7 @@ static int common_param_init(struct find_param *param, char *path)
                            "error: allocation of %d bytes for ioctl",
                            lmv_user_md_size(param->fp_lmv_stripe_count,
                                             LMV_USER_MAGIC_SPECIFIC));
                            "error: allocation of %d bytes for ioctl",
                            lmv_user_md_size(param->fp_lmv_stripe_count,
                                             LMV_USER_MAGIC_SPECIFIC));

+               find_param_fini(param);

                return -ENOMEM;
        }
 
                return -ENOMEM;
        }
 


@@ -1692,21 +1714,6 @@ static int common_param_init(struct find_param *param, char *path)
        return 0;
 }
 
        return 0;
 }
 

-static void find_param_fini(struct find_param *param)
-{
-       if (param->fp_migrate)
-               return;
-
-       if (param->fp_obd_indexes)
-               free(param->fp_obd_indexes);
-
-       if (param->fp_lmd)
-               free(param->fp_lmd);
-
-       if (param->fp_lmv_md)
-               free(param->fp_lmv_md);
-}
-

 static int cb_common_fini(char *path, DIR *parent, DIR **dirp, void *data,
                          struct dirent64 *de)
 {
 static int cb_common_fini(char *path, DIR *parent, DIR **dirp, void *data,
                          struct dirent64 *de)
 {


@@ -3586,8 +3593,7 @@ int llapi_file_get_stripe(const char *path, struct lov_user_md *lum)
        fd = open(dname, O_RDONLY | O_NONBLOCK);
        if (fd == -1) {
                rc = -errno;
        fd = open(dname, O_RDONLY | O_NONBLOCK);
        if (fd == -1) {
                rc = -errno;

-               free(dname);
-               return rc;
+               goto out_free;

        }
 
        strcpy((char *)lum, fname);
        }
 
        strcpy((char *)lum, fname);


@@ -3597,6 +3603,7 @@ int llapi_file_get_stripe(const char *path, struct lov_user_md *lum)
        if (close(fd) == -1 && rc == 0)
                rc = -errno;
 
        if (close(fd) == -1 && rc == 0)
                rc = -errno;
 

+out_free:

        free(dname);
        return rc;
 }
        free(dname);
        return rc;
 }


@@ -3974,7 +3981,7 @@ static int find_check_comp_options(struct find_param *param)
                break;
        }
 out:
                break;
        }
 out:

-       if (forged_v1 != NULL)
+       if (forged_v1)

                free(forged_v1);
        return ret;
 }
                free(forged_v1);
        return ret;
 }