From: John L. Hammond <john.hammond@intel.com>
Date: Tue, 1 Sep 2015 21:50:44 +0000 (-0500)
Subject: LU-7057 utils: use stronger flags when opening volatile files
X-Git-Tag: 2.7.60~31
X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=63b37d321fbe27d72055e3895ad94305d56927d7;p=fs%2Flustre-release.git

LU-7057 utils: use stronger flags when opening volatile files

Existing files with volatile-ish names may be accidentally opened by
lfs_migrate(). This is not the intent so disallow this behavior by
adding O_EXCL and O_NOFOLLOW to the flags to open(). Use the original
file's MDT index and a random suffix when constructing the volatile
file name (as is done in llapi_create_volatile_idx()) and reduce the
mode of the file created.

To handle pre-existing files use a do while lookup around the volatile
opens in llapi_create_volatile_idx() and lfs_migrate().

Signed-off-by: John L. Hammond <john.hammond@intel.com>
Change-Id: I399d30104d841df13fe1051aaa8264514911714d
Reviewed-on: http://review.whamcloud.com/16166
Tested-by: Jenkins
Reviewed-by: Henri Doreau <henri.doreau@cea.fr>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Tested-by: Andreas Dilger <andreas.dilger@intel.com>
---

diff --git a/lustre/utils/lfs.c b/lustre/utils/lfs.c
index c58113d..c221e14 100644
--- a/lustre/utils/lfs.c
+++ b/lustre/utils/lfs.c
@@ -623,9 +623,13 @@ static int lfs_migrate(char *name, __u64 migration_flags,
 {
 	int			 fd = -1;
 	int			 fdv = -1;
-	char			 volatile_file[PATH_MAX +
-						LUSTRE_VOLATILE_HDR_LEN + 4];
 	char			 parent[PATH_MAX];
+	int			 mdt_index;
+	int                      random_value;
+	char			 volatile_file[sizeof(parent) +
+					       LUSTRE_VOLATILE_HDR_LEN +
+					       2 * sizeof(mdt_index) +
+					       2 * sizeof(random_value) + 4];
 	char			*ptr;
 	int			 rc;
 	struct lov_user_md	*lum = NULL;
@@ -699,18 +703,29 @@ static int lfs_migrate(char *name, __u64 migration_flags,
 			*ptr = '\0';
 	}
 
-	rc = snprintf(volatile_file, sizeof(volatile_file), "%s/%s::", parent,
-		      LUSTRE_VOLATILE_HDR);
-	if (rc >= sizeof(volatile_file)) {
-		rc = -E2BIG;
+	rc = llapi_file_fget_mdtidx(fd, &mdt_index);
+	if (rc < 0) {
+		fprintf(stderr, "%s: %s: cannot get MDT index: %s\n",
+			progname, name, strerror(-rc));
 		goto error;
 	}
 
-	/* create, open a volatile file, use caching (ie no directio) */
-	/* exclusive create is not needed because volatile files cannot
-	 * conflict on name by construction */
-	fdv = llapi_file_open_param(volatile_file, O_CREAT | O_WRONLY, 0644,
-				    param);
+	do {
+		random_value = random();
+		rc = snprintf(volatile_file, sizeof(volatile_file),
+			      "%s/%s:%.4X:%.4X", parent, LUSTRE_VOLATILE_HDR,
+			      mdt_index, random_value);
+		if (rc >= sizeof(volatile_file)) {
+			rc = -E2BIG;
+			goto error;
+		}
+
+		/* create, open a volatile file, use caching (ie no directio) */
+		fdv = llapi_file_open_param(volatile_file,
+				O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW,
+					    S_IRUSR | S_IWUSR, param);
+	} while (fdv == -EEXIST);
+
 	if (fdv < 0) {
 		rc = fdv;
 		fprintf(stderr, "%s: %s: cannot create volatile file in"
diff --git a/lustre/utils/liblustreapi.c b/lustre/utils/liblustreapi.c
index 5cab621..ff8e58d 100644
--- a/lustre/utils/liblustreapi.c
+++ b/lustre/utils/liblustreapi.c
@@ -4628,26 +4628,30 @@ int llapi_create_volatile_idx(char *directory, int idx, int open_flags)
 {
 	char	file_path[PATH_MAX];
 	char	filename[PATH_MAX];
+	int	saved_errno = errno;
 	int	fd;
 	int	rnumber;
 	int	rc;
 
-	rnumber = random();
-	if (idx == -1)
-		snprintf(filename, sizeof(filename),
-			 LUSTRE_VOLATILE_HDR"::%.4X", rnumber);
-	else
-		snprintf(filename, sizeof(filename),
-			 LUSTRE_VOLATILE_HDR":%.4X:%.4X", idx, rnumber);
+	do {
+		rnumber = random();
+		if (idx == -1)
+			snprintf(filename, sizeof(filename),
+				 LUSTRE_VOLATILE_HDR"::%.4X", rnumber);
+		else
+			snprintf(filename, sizeof(filename),
+				 LUSTRE_VOLATILE_HDR":%.4X:%.4X", idx, rnumber);
+
+		rc = snprintf(file_path, sizeof(file_path),
+			      "%s/%s", directory, filename);
+		if (rc >= sizeof(file_path))
+			return -E2BIG;
 
-	rc = snprintf(file_path, sizeof(file_path),
-		      "%s/%s", directory, filename);
-	if (rc >= sizeof(file_path))
-		return -E2BIG;
+		fd = open(file_path,
+			  O_RDWR | O_CREAT | O_EXCL | O_NOFOLLOW | open_flags,
+			  S_IRUSR | S_IWUSR);
+	} while (fd < 0 && errno == EEXIST);
 
-	fd = open(file_path,
-		  O_RDWR | O_CREAT | O_EXCL | O_NOFOLLOW | open_flags,
-		  S_IRUSR | S_IWUSR);
 	if (fd < 0) {
 		llapi_error(LLAPI_MSG_ERROR, errno,
 			    "Cannot create volatile file '%s' in '%s'",
@@ -4655,10 +4659,17 @@ int llapi_create_volatile_idx(char *directory, int idx, int open_flags)
 			    directory);
 		return -errno;
 	}
-	/* unlink file in case this wasn't a Lustre filesystem, and the
-	 * magic volatile filename wasn't handled as intended.  The effect
-	 * is the same. */
-	unlink(file_path);
+
+	/* Unlink file in case this wasn't a Lustre filesystem and the
+	 * magic volatile filename wasn't handled as intended. The
+	 * effect is the same. If volatile open was supported then we
+	 * expect unlink() to return -ENOENT. */
+	(void)unlink(file_path);
+
+	/* Since we are returning successfully we restore errno (and
+	 * mask out possible EEXIST from open() and ENOENT from
+	 * unlink(). */
+	errno = saved_errno;
 
 	return fd;
 }