Skip to content

Commit 9c5da09

Browse files
sqaziIngo Molnar
authored andcommitted
perf: Use css_tryget() to avoid propping up css refcount
An rmdir pushes css's ref count to zero. However, if the associated directory is open at the time, the dentry ref count is non-zero. If the fd for this directory is then passed into perf_event_open, it does a css_get(). This bounces the ref count back up from zero. This is a problem by itself. But what makes it turn into a crash is the fact that we end up doing an extra dput, since we perform a dput when css_put sees the ref count go down to zero. css_tryget() does not fall into that trap. So, we use that instead. Reproduction test-case for the bug: #include <unistd.h> #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> #include <linux/unistd.h> #include <linux/perf_event.h> #include <string.h> #include <errno.h> #include <stdio.h> #define PERF_FLAG_PID_CGROUP (1U << 2) int perf_event_open(struct perf_event_attr *hw_event_uptr, pid_t pid, int cpu, int group_fd, unsigned long flags) { return syscall(__NR_perf_event_open,hw_event_uptr, pid, cpu, group_fd, flags); } /* * Directly poke at the perf_event bug, since it's proving hard to repro * depending on where in the kernel tree. what moved? */ int main(int argc, char **argv) { int fd; struct perf_event_attr attr; memset(&attr, 0, sizeof(attr)); attr.exclude_kernel = 1; attr.size = sizeof(attr); mkdir("/dev/cgroup/perf_event/blah", 0777); fd = open("/dev/cgroup/perf_event/blah", O_RDONLY); perror("open"); rmdir("/dev/cgroup/perf_event/blah"); sleep(2); perf_event_open(&attr, fd, 0, -1, PERF_FLAG_PID_CGROUP); perror("perf_event_open"); close(fd); return 0; } Signed-off-by: Salman Qazi <[email protected]> Signed-off-by: Peter Zijlstra <[email protected]> Acked-by: Tejun Heo <[email protected]> Link: http://lkml.kernel.org/r/[email protected] Signed-off-by: Ingo Molnar <[email protected]>
1 parent 8ed9eac commit 9c5da09

File tree

1 file changed

+7
-3
lines changed

1 file changed

+7
-3
lines changed

kernel/events/core.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -253,9 +253,9 @@ perf_cgroup_match(struct perf_event *event)
253253
return !event->cgrp || event->cgrp == cpuctx->cgrp;
254254
}
255255

256-
static inline void perf_get_cgroup(struct perf_event *event)
256+
static inline bool perf_tryget_cgroup(struct perf_event *event)
257257
{
258-
css_get(&event->cgrp->css);
258+
return css_tryget(&event->cgrp->css);
259259
}
260260

261261
static inline void perf_put_cgroup(struct perf_event *event)
@@ -484,7 +484,11 @@ static inline int perf_cgroup_connect(int fd, struct perf_event *event,
484484
event->cgrp = cgrp;
485485

486486
/* must be done before we fput() the file */
487-
perf_get_cgroup(event);
487+
if (!perf_tryget_cgroup(event)) {
488+
event->cgrp = NULL;
489+
ret = -ENOENT;
490+
goto out;
491+
}
488492

489493
/*
490494
* all events in a group must monitor

0 commit comments

Comments
 (0)