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 <[email protected]>
---
 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 via email to