linux-kernel - [tip:perf/core] perf tools: Avoid possible race condition in copyfile()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <tip-d7c72606d97e6f462a99b79e55b39808147d4c8b@git.kernel.org>
Date:	Fri, 12 Jun 2015 01:49:17 -0700
From:	tip-bot for Milos Vyletel <tipbot@...or.com>
To:	linux-tip-commits@...r.kernel.org
Cc:	dzickus@...hat.com, linux-kernel@...r.kernel.org, acme@...hat.com,
	andriy.shevchenko@...ux.intel.com, tglx@...utronix.de,
	namhyung@...nel.org, rostedt@...dmis.org, hpa@...or.com,
	milos@...hat.com, jolsa@...nel.org, a.p.zijlstra@...llo.nl,
	mingo@...nel.org
Subject: [tip:perf/core] perf tools:
  Avoid possible race condition in copyfile()

Commit-ID:  d7c72606d97e6f462a99b79e55b39808147d4c8b
Gitweb:     http://git.kernel.org/tip/d7c72606d97e6f462a99b79e55b39808147d4c8b
Author:     Milos Vyletel <milos@...hat.com>
AuthorDate: Mon, 8 Jun 2015 16:50:16 +0200
Committer:  Arnaldo Carvalho de Melo <acme@...hat.com>
CommitDate: Wed, 10 Jun 2015 11:51:24 -0300

perf tools: Avoid possible race condition in copyfile()

Use unique temporary files when copying to buildid dir to prevent races
in case multiple instances are trying to copy same file. This is done by

- creating template in form <path>/.<filename>.XXXXXX where the suffix is
  used by mkstemp() to create unique file
- change file mode
- copy content
- if successful link temp file to target file
- unlink temp file

At this point the only file left at target path should be the desired
one either created by us or other instance if we raced. This should also
prevent not yet fully copied files to be visible to to other perf
instances that could try to parse them.

On top of that slow_copyfile no longer needs to deal with file mode when
creating file since temporary file is already created and mode is set.

Succesfully tested by myself by running perf record, archive and reading
the data on other system and by running perf buildid-cache on perf
binary itself. I also did revert fix from 0635b0f that to exposes
previously fixed race with EEXIST and recreator test passed sucessfully.

Signed-off-by: Milos Vyletel <milos@...hat.com>
Acked-by: Ingo Molnar <mingo@...nel.org>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Don Zickus <dzickus@...hat.com>
Cc: Jiri Olsa <jolsa@...nel.org>
Cc: Namhyung Kim <namhyung@...nel.org>
Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc: Steven Rostedt <rostedt@...dmis.org>
Link: http://lkml.kernel.org/r/1433775018-19868-1-git-send-email-milos@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
---
 tools/perf/util/util.c | 46 +++++++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 0c264bc..edc2d63 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -115,20 +115,17 @@ int rm_rf(char *path)
 	return rmdir(path);
 }
 
-static int slow_copyfile(const char *from, const char *to, mode_t mode)
+static int slow_copyfile(const char *from, const char *to)
 {
 	int err = -1;
 	char *line = NULL;
 	size_t n;
 	FILE *from_fp = fopen(from, "r"), *to_fp;
-	mode_t old_umask;
 
 	if (from_fp == NULL)
 		goto out;
 
-	old_umask = umask(mode ^ 0777);
 	to_fp = fopen(to, "w");
-	umask(old_umask);
 	if (to_fp == NULL)
 		goto out_fclose_from;
 
@@ -178,29 +175,48 @@ int copyfile_mode(const char *from, const char *to, mode_t mode)
 	int fromfd, tofd;
 	struct stat st;
 	int err = -1;
+	char *tmp = NULL, *ptr = NULL;
 
 	if (stat(from, &st))
 		goto out;
 
-	if (st.st_size == 0) /* /proc? do it slowly... */
-		return slow_copyfile(from, to, mode);
-
-	fromfd = open(from, O_RDONLY);
-	if (fromfd < 0)
+	/* extra 'x' at the end is to reserve space for '.' */
+	if (asprintf(&tmp, "%s.XXXXXXx", to) < 0) {
+		tmp = NULL;
 		goto out;
+	}
+	ptr = strrchr(tmp, '/');
+	if (!ptr)
+		goto out;
+	ptr = memmove(ptr + 1, ptr, strlen(ptr) - 1);
+	*ptr = '.';
 
-	tofd = creat(to, mode);
+	tofd = mkstemp(tmp);
 	if (tofd < 0)
-		goto out_close_from;
+		goto out;
+
+	if (fchmod(tofd, mode))
+		goto out_close_to;
+
+	if (st.st_size == 0) { /* /proc? do it slowly... */
+		err = slow_copyfile(from, tmp);
+		goto out_close_to;
+	}
+
+	fromfd = open(from, O_RDONLY);
+	if (fromfd < 0)
+		goto out_close_to;
 
 	err = copyfile_offset(fromfd, 0, tofd, 0, st.st_size);
 
-	close(tofd);
-	if (err)
-		unlink(to);
-out_close_from:
 	close(fromfd);
+out_close_to:
+	close(tofd);
+	if (!err)
+		err = link(tmp, to);
+	unlink(tmp);
 out:
+	free(tmp);
 	return err;
 }
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/