Commit 304f0a2f authored by Ian Rogers's avatar Ian Rogers Committed by Arnaldo Carvalho de Melo
Browse files

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: default avatar <gthelen@google.com>
Reviewed-by: default avatarLeo Yan <leo.yan@linaro.org>
Signed-off-by: default avatarIan Rogers <irogers@google.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Anand K Mistry <amistry@google.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: https://lore.kernel.org/r/20221024011024.462518-1-irogers@google.com


Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent f1bdebbb

tools/perf/builtin-record.c

+25 −16
Original line number Diff line number Diff line
@@ -649,7 +649,7 @@ static int record__pushfn(struct mmap *map, void *to, void *bf, size_t size) 
static volatile int signr = -1;

static volatile int child_finished;

#ifdef HAVE_EVENTFD_SUPPORT

static int done_fd = -1;

static volatile int done_fd = -1;

#endif



static void sig_handler(int sig)
@@ -661,18 +661,23 @@ static void sig_handler(int sig) 


	done = 1;

#ifdef HAVE_EVENTFD_SUPPORT

{

	if (done_fd >= 0) {

		u64 tmp = 1;

		int orig_errno = errno;



		/*

	 * It is possible for this signal handler to run after done is checked

	 * in the main loop, but before the perf counter fds are polled. If this

	 * happens, the poll() will continue to wait even though done is set,

	 * and will only break out if either another signal is received, or the

	 * counters are ready for read. To ensure the poll() doesn't sleep when

	 * done is set, use an eventfd (done_fd) to wake up the poll().

		 * It is possible for this signal handler to run after done is

		 * checked in the main loop, but before the perf counter fds are

		 * polled. If this happens, the poll() will continue to wait

		 * even though done is set, and will only break out if either

		 * another signal is received, or the counters are ready for

		 * read. To ensure the poll() doesn't sleep when done is set,

		 * use an eventfd (done_fd) to wake up the poll().

		 */

		if (write(done_fd, &tmp, sizeof(tmp)) < 0)

			pr_err("failed to signal wakeup fd, error: %m\n");



		errno = orig_errno;

	}

#endif // HAVE_EVENTFD_SUPPORT

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


out_delete_session:

#ifdef HAVE_EVENTFD_SUPPORT

	if (done_fd >= 0)

		close(done_fd);

	if (done_fd >= 0) {

		fd = done_fd;

		done_fd = -1;



		close(fd);

	}

#endif

	zstd_fini(&session->zstd_data);

	perf_session__delete(session);