Skip to content

Commit

Permalink
perf: Use css_tryget() to avoid propping up css refcount
Browse files Browse the repository at this point in the history
commit 9c5da09d266ca9b32eb16cf940f8161d949c2fe5 upstream.

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;
 }

Change-Id: I7d1c30bcc23d50cfde200acb94ad6e229cb3e32d
Original-Change-Id: Id8681b3277e4631b26eda9da1175d0b4ec36dfed
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]>
Cc: Li Zefan <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
  • Loading branch information
sqazi authored and nasty007 committed Aug 18, 2017
1 parent a378295 commit 0553537
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions kernel/events/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,9 @@ perf_cgroup_match(struct perf_event *event)
return !event->cgrp || event->cgrp == cpuctx->cgrp;
}

static inline void perf_get_cgroup(struct perf_event *event)
static inline bool perf_tryget_cgroup(struct perf_event *event)
{
css_get(&event->cgrp->css);
return css_tryget(&event->cgrp->css);
}

static inline void perf_put_cgroup(struct perf_event *event)
Expand Down Expand Up @@ -493,7 +493,11 @@ static inline int perf_cgroup_connect(int fd, struct perf_event *event,
event->cgrp = cgrp;

/* must be done before we fput() the file */
perf_get_cgroup(event);
if (!perf_tryget_cgroup(event)) {
event->cgrp = NULL;
ret = -ENOENT;
goto out;
}

/*
* all events in a group must monitor
Expand Down

0 comments on commit 0553537

Please sign in to comment.