Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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@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>

authored by

Milos Vyletel and committed by
Arnaldo Carvalho de Melo
d7c72606 028c63b5

+31 -15
+31 -15
tools/perf/util/util.c
··· 115 115 return rmdir(path); 116 116 } 117 117 118 - static int slow_copyfile(const char *from, const char *to, mode_t mode) 118 + static int slow_copyfile(const char *from, const char *to) 119 119 { 120 120 int err = -1; 121 121 char *line = NULL; 122 122 size_t n; 123 123 FILE *from_fp = fopen(from, "r"), *to_fp; 124 - mode_t old_umask; 125 124 126 125 if (from_fp == NULL) 127 126 goto out; 128 127 129 - old_umask = umask(mode ^ 0777); 130 128 to_fp = fopen(to, "w"); 131 - umask(old_umask); 132 129 if (to_fp == NULL) 133 130 goto out_fclose_from; 134 131 ··· 175 178 int fromfd, tofd; 176 179 struct stat st; 177 180 int err = -1; 181 + char *tmp = NULL, *ptr = NULL; 178 182 179 183 if (stat(from, &st)) 180 184 goto out; 181 185 182 - if (st.st_size == 0) /* /proc? do it slowly... */ 183 - return slow_copyfile(from, to, mode); 186 + /* extra 'x' at the end is to reserve space for '.' */ 187 + if (asprintf(&tmp, "%s.XXXXXXx", to) < 0) { 188 + tmp = NULL; 189 + goto out; 190 + } 191 + ptr = strrchr(tmp, '/'); 192 + if (!ptr) 193 + goto out; 194 + ptr = memmove(ptr + 1, ptr, strlen(ptr) - 1); 195 + *ptr = '.'; 196 + 197 + tofd = mkstemp(tmp); 198 + if (tofd < 0) 199 + goto out; 200 + 201 + if (fchmod(tofd, mode)) 202 + goto out_close_to; 203 + 204 + if (st.st_size == 0) { /* /proc? do it slowly... */ 205 + err = slow_copyfile(from, tmp); 206 + goto out_close_to; 207 + } 184 208 185 209 fromfd = open(from, O_RDONLY); 186 210 if (fromfd < 0) 187 - goto out; 188 - 189 - tofd = creat(to, mode); 190 - if (tofd < 0) 191 - goto out_close_from; 211 + goto out_close_to; 192 212 193 213 err = copyfile_offset(fromfd, 0, tofd, 0, st.st_size); 194 214 195 - close(tofd); 196 - if (err) 197 - unlink(to); 198 - out_close_from: 199 215 close(fromfd); 216 + out_close_to: 217 + close(tofd); 218 + if (!err) 219 + err = link(tmp, to); 220 + unlink(tmp); 200 221 out: 222 + free(tmp); 201 223 return err; 202 224 } 203 225