Skip to content

Commit 304f0a2

Browse files
captain5050acmel
authored andcommitted
perf record: Fix event fd races
The write call may set errno which is problematic if occurring in a function also setting errno. Save and restore errno around the write call. done_fd may be used after close, clear it as part of the close and check its validity in the signal handler. Suggested-by: <[email protected]> Reviewed-by: Leo Yan <[email protected]> Signed-off-by: Ian Rogers <[email protected]> Cc: Alexander Shishkin <[email protected]> Cc: Anand K Mistry <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Jiri Olsa <[email protected]> Cc: Mark Rutland <[email protected]> Cc: Namhyung Kim <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Stephane Eranian <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
1 parent f1bdebb commit 304f0a2

File tree

1 file changed

+25
-16
lines changed

1 file changed

+25
-16
lines changed

tools/perf/builtin-record.c

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -649,7 +649,7 @@ static int record__pushfn(struct mmap *map, void *to, void *bf, size_t size)
649649
static volatile int signr = -1;
650650
static volatile int child_finished;
651651
#ifdef HAVE_EVENTFD_SUPPORT
652-
static int done_fd = -1;
652+
static volatile int done_fd = -1;
653653
#endif
654654

655655
static void sig_handler(int sig)
@@ -661,19 +661,24 @@ static void sig_handler(int sig)
661661

662662
done = 1;
663663
#ifdef HAVE_EVENTFD_SUPPORT
664-
{
665-
u64 tmp = 1;
666-
/*
667-
* It is possible for this signal handler to run after done is checked
668-
* in the main loop, but before the perf counter fds are polled. If this
669-
* happens, the poll() will continue to wait even though done is set,
670-
* and will only break out if either another signal is received, or the
671-
* counters are ready for read. To ensure the poll() doesn't sleep when
672-
* done is set, use an eventfd (done_fd) to wake up the poll().
673-
*/
674-
if (write(done_fd, &tmp, sizeof(tmp)) < 0)
675-
pr_err("failed to signal wakeup fd, error: %m\n");
676-
}
664+
if (done_fd >= 0) {
665+
u64 tmp = 1;
666+
int orig_errno = errno;
667+
668+
/*
669+
* It is possible for this signal handler to run after done is
670+
* checked in the main loop, but before the perf counter fds are
671+
* polled. If this happens, the poll() will continue to wait
672+
* even though done is set, and will only break out if either
673+
* another signal is received, or the counters are ready for
674+
* read. To ensure the poll() doesn't sleep when done is set,
675+
* use an eventfd (done_fd) to wake up the poll().
676+
*/
677+
if (write(done_fd, &tmp, sizeof(tmp)) < 0)
678+
pr_err("failed to signal wakeup fd, error: %m\n");
679+
680+
errno = orig_errno;
681+
}
677682
#endif // HAVE_EVENTFD_SUPPORT
678683
}
679684

@@ -2834,8 +2839,12 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
28342839

28352840
out_delete_session:
28362841
#ifdef HAVE_EVENTFD_SUPPORT
2837-
if (done_fd >= 0)
2838-
close(done_fd);
2842+
if (done_fd >= 0) {
2843+
fd = done_fd;
2844+
done_fd = -1;
2845+
2846+
close(fd);
2847+
}
28392848
#endif
28402849
zstd_fini(&session->zstd_data);
28412850
perf_session__delete(session);

0 commit comments

Comments
 (0)