mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jin Yao <yao.jin@linux.intel.com>
To: acme@kernel.org, jolsa@kernel.org, peterz@infradead.org,
	mingo@redhat.com, alexander.shishkin@linux.intel.com
Cc: Linux-kernel@vger.kernel.org, ak@linux.intel.com,
	kan.liang@intel.com, yao.jin@intel.com,
	Jin Yao <yao.jin@linux.intel.com>
Subject: [PATCH] perf util: Fix wrong processing when closing evsel fd
Date: Wed, 18 Oct 2017 23:11:18 +0800	[thread overview]
Message-ID: <1508339478-26674-1-git-send-email-yao.jin@linux.intel.com> (raw)

In current xyarray code, xyarray__max_x() returns max_y, and
xyarray__max_y() returns max_x.

It's confusing and for code logic it looks not correct.
Error happens when closing evsel fd. Let's see this scenario:

1. Allocate an fd (pseudo-code)
-------------------------------

perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
{
	evsel->fd = xyarray__new(ncpus, nthreads, sizeof(int));
}

xyarray__new(int xlen, int ylen, size_t entry_size)
{
	size_t row_size = ylen * entry_size;
	struct xyarray *xy = zalloc(sizeof(*xy) + xlen * row_size);

	xy->entry_size = entry_size;
	xy->row_size   = row_size;
	xy->entries    = xlen * ylen;
	xy->max_x      = xlen;
	xy->max_y      = ylen;
	......
}

So max_x is ncpus, max_y is nthreads and row_size = nthreads * 4.

2. Use perf syscall and get the fd
----------------------------------

int perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
		     struct thread_map *threads)
{
	for (cpu = 0; cpu < cpus->nr; cpu++) {

		for (thread = 0; thread < nthreads; thread++) {
			int fd, group_fd;

			fd = sys_perf_event_open(&evsel->attr, pid, cpus->map[cpu],
						 group_fd, flags);

			FD(evsel, cpu, thread) = fd;
	}
}

static inline void *xyarray__entry(struct xyarray *xy, int x, int y)
{
	return &xy->contents[x * xy->row_size + y * xy->entry_size];
}

These codes don't have issues. The issue happens in the closing of fd.

3. Close fd.
-----------

void perf_evsel__close_fd(struct perf_evsel *evsel)
{
	int cpu, thread;

	for (cpu = 0; cpu < xyarray__max_x(evsel->fd); cpu++)
		for (thread = 0; thread < xyarray__max_y(evsel->fd); ++thread) {
			close(FD(evsel, cpu, thread));
			FD(evsel, cpu, thread) = -1;
		}
}

Since xyarray__max_x() returns max_y (nthreads) and xyarry__max_y()
returns max_x (ncpus), so above code is actually to be:

        for (cpu = 0; cpu < nthreads; cpu++)
                for (thread = 0; thread < ncpus; ++thread) {
                        close(FD(evsel, cpu, thread));
                        FD(evsel, cpu, thread) = -1;
                }

It's not correct!

This change is introduced by "475fb533fb7d"
("perf evsel: Fix buffer overflow while freeing events")

This fix is to let xyarray__max_x() return max_x (ncpus) and
let xyarry__max_y() return max_y (nthreads)

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/util/xyarray.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/xyarray.h b/tools/perf/util/xyarray.h
index 4ba726c..54af6046 100644
--- a/tools/perf/util/xyarray.h
+++ b/tools/perf/util/xyarray.h
@@ -23,12 +23,12 @@ static inline void *xyarray__entry(struct xyarray *xy, int x, int y)
 
 static inline int xyarray__max_y(struct xyarray *xy)
 {
-	return xy->max_x;
+	return xy->max_y;
 }
 
 static inline int xyarray__max_x(struct xyarray *xy)
 {
-	return xy->max_y;
+	return xy->max_x;
 }
 
 #endif /* _PERF_XYARRAY_H_ */
-- 
2.7.4

             reply	other threads:[~2017-10-18  7:15 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-18 15:11 Jin Yao [this message]
2017-10-18  8:12 ` Jiri Olsa
2017-10-18 12:07   ` Arnaldo Carvalho de Melo
2017-10-18 11:50 ` [PATCH] perf record: Fix tool crash with xyarray Ravi Bangoria
2017-10-18 12:07   ` Arnaldo Carvalho de Melo
2017-10-20  7:21   ` [tip:perf/urgent] perf xyarray: Fix wrong processing when closing evsel fd tip-bot for Jin Yao

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=1508339478-26674-1-git-send-email-yao.jin@linux.intel.com \
    --to=yao.jin@linux.intel.com \
    --cc=Linux-kernel@vger.kernel.org \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@intel.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=yao.jin@intel.com \
    /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

Powered by JetHome