From: Andreas Dilger <adilger@whamcloud.com>
Date: Thu, 18 Apr 2019 23:29:44 +0000 (-0600)
Subject: LU-11233 utils: fix double-free of params fields
X-Git-Tag: 2.12.54~101
X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=7c7c39d84c98df3c6fe33c04c9e391529a86db53

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
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -1573,6 +1573,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 c7a8281..c5098fd 100644
--- 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)
 
+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);
@@ -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));
+		find_param_fini(param);
 		return -ENOMEM;
 	}
 
@@ -1692,21 +1714,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)
 {
@@ -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;
-		free(dname);
-		return rc;
+		goto out_free;
 	}
 
 	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;
 
+out_free:
 	free(dname);
 	return rc;
 }
@@ -3974,7 +3981,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;
 }