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.

Lustre-change: https://review.whamcloud.com/34711
Lustre-commit: 7c7c39d84c98df3c6fe33c04c9e391529a86db53

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

diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index e9ddf88..afd1a18 100755 (executable)
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -1570,6 +1570,16 @@ test_27g() {
 }
 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"

diff --git a/lustre/utils/liblustreapi.c b/lustre/utils/liblustreapi.c
index 426f8e7..8811fe5 100644 (file)
--- a/lustre/utils/liblustreapi.c
+++ b/lustre/utils/liblustreapi.c
@@ -1646,6 +1646,27 @@ typedef int (semantic_func_t)(char *path, DIR *parent, DIR **d,
 
 #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);
@@ -1678,6 +1699,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));
+               find_param_fini(param);
                return -ENOMEM;
        }
 
@@ -1688,21 +1710,6 @@ static int common_param_init(struct find_param *param, char *path)
        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)
 {
@@ -3582,8 +3589,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;
-               free(dname);
-               return rc;
+               goto out_free;
        }
 
        strcpy((char *)lum, fname);
@@ -3593,6 +3599,7 @@ int llapi_file_get_stripe(const char *path, struct lov_user_md *lum)
        if (close(fd) == -1 && rc == 0)
                rc = -errno;
 
+out_free:
        free(dname);
        return rc;
 }
@@ -3970,7 +3977,7 @@ static int find_check_comp_options(struct find_param *param)
                break;
        }
 out:
-       if (forged_v1 != NULL)
+       if (forged_v1)
                free(forged_v1);
        return ret;
 }