From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org, Milos Vyletel <milos@redhat.com>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Don Zickus <dzickus@redhat.com>, Jiri Olsa <jolsa@kernel.org>,
Namhyung Kim <namhyung@kernel.org>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Steven Rostedt <rostedt@goodmis.org>,
Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: [PATCH 1/7] perf tools: Avoid possible race condition in copyfile()
Date: Thu, 11 Jun 2015 23:15:37 -0300 [thread overview]
Message-ID: <1434075343-4173-2-git-send-email-acme@kernel.org> (raw)
In-Reply-To: <1434075343-4173-1-git-send-email-acme@kernel.org>
From: Milos Vyletel <milos@redhat.com>
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@redhat.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/1433775018-19868-1-git-send-email-milos@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.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 0c264bc685ac..edc2d633b332 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;
}
--
2.1.0
next prev parent reply other threads:[~2015-06-12 2:16 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-12 2:15 [GIT PULL 0/7] perf/core improvements and fixes Arnaldo Carvalho de Melo
2015-06-12 2:15 ` Arnaldo Carvalho de Melo [this message]
2015-06-12 2:15 ` [PATCH 2/7] perf record: Amend option summaries Arnaldo Carvalho de Melo
2015-06-12 2:15 ` [PATCH 3/7] perf evsel: Display 0x for hex values when printing the attribute Arnaldo Carvalho de Melo
2015-06-12 2:15 ` [PATCH 4/7] perf stat: Error out unsupported group leader immediately Arnaldo Carvalho de Melo
2015-06-12 2:15 ` [PATCH 5/7] perf tools: Fix build failure on 32-bit arch Arnaldo Carvalho de Melo
2015-06-12 2:15 ` [PATCH 6/7] trace: Beautify perf_event_open syscall Arnaldo Carvalho de Melo
2015-06-12 2:15 ` [PATCH 7/7] perf tools: Update MANIFEST per files removed from kernel Arnaldo Carvalho de Melo
2015-06-12 8:09 ` [GIT PULL 0/7] perf/core improvements and fixes Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1434075343-4173-2-git-send-email-acme@kernel.org \
--to=acme@kernel.org \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@redhat.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=dzickus@redhat.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=milos@redhat.com \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).