lkml.org 
[lkml]   [2018]   [May]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[tip:perf/core] perf/core: Fix bad use of igrab()
    Commit-ID:  9511bce9fe8e5e6c0f923c09243a713eba560141
    Gitweb: https://git.kernel.org/tip/9511bce9fe8e5e6c0f923c09243a713eba560141
    Author: Song Liu <songliubraving@fb.com>
    AuthorDate: Tue, 17 Apr 2018 23:29:07 -0700
    Committer: Ingo Molnar <mingo@kernel.org>
    CommitDate: Fri, 25 May 2018 08:11:10 +0200

    perf/core: Fix bad use of igrab()

    As Miklos reported and suggested:

    "This pattern repeats two times in trace_uprobe.c and in
    kernel/events/core.c as well:

    ret = kern_path(filename, LOOKUP_FOLLOW, &path);
    if (ret)
    goto fail_address_parse;

    inode = igrab(d_inode(path.dentry));
    path_put(&path);

    And it's wrong. You can only hold a reference to the inode if you
    have an active ref to the superblock as well (which is normally
    through path.mnt) or holding s_umount.

    This way unmounting the containing filesystem while the tracepoint is
    active will give you the "VFS: Busy inodes after unmount..." message
    and a crash when the inode is finally put.

    Solution: store path instead of inode."

    This patch fixes the issue in kernel/event/core.c.

    Reviewed-and-tested-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
    Reported-by: Miklos Szeredi <miklos@szeredi.hu>
    Signed-off-by: Song Liu <songliubraving@fb.com>
    Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
    Cc: <kernel-team@fb.com>
    Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
    Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
    Cc: Jiri Olsa <jolsa@redhat.com>
    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Stephane Eranian <eranian@google.com>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    Cc: Vince Weaver <vincent.weaver@maine.edu>
    Fixes: 375637bc5249 ("perf/core: Introduce address range filtering")
    Link: http://lkml.kernel.org/r/20180418062907.3210386-2-songliubraving@fb.com
    Signed-off-by: Ingo Molnar <mingo@kernel.org>
    ---
    arch/x86/events/intel/pt.c | 4 ++--
    include/linux/perf_event.h | 2 +-
    kernel/events/core.c | 21 +++++++++------------
    3 files changed, 12 insertions(+), 15 deletions(-)

    diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
    index 3b993942a0e4..8d016ce5b80d 100644
    --- a/arch/x86/events/intel/pt.c
    +++ b/arch/x86/events/intel/pt.c
    @@ -1194,7 +1194,7 @@ static int pt_event_addr_filters_validate(struct list_head *filters)
    filter->action == PERF_ADDR_FILTER_ACTION_START)
    return -EOPNOTSUPP;

    - if (!filter->inode) {
    + if (!filter->path.dentry) {
    if (!valid_kernel_ip(filter->offset))
    return -EINVAL;

    @@ -1221,7 +1221,7 @@ static void pt_event_addr_filters_sync(struct perf_event *event)
    return;

    list_for_each_entry(filter, &head->list, entry) {
    - if (filter->inode && !offs[range]) {
    + if (filter->path.dentry && !offs[range]) {
    msr_a = msr_b = 0;
    } else {
    /* apply the offset */
    diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
    index def866f7269b..bea0b0cd4bf7 100644
    --- a/include/linux/perf_event.h
    +++ b/include/linux/perf_event.h
    @@ -467,7 +467,7 @@ enum perf_addr_filter_action_t {
    */
    struct perf_addr_filter {
    struct list_head entry;
    - struct inode *inode;
    + struct path path;
    unsigned long offset;
    unsigned long size;
    enum perf_addr_filter_action_t action;
    diff --git a/kernel/events/core.c b/kernel/events/core.c
    index ce6aa5ff3c96..24dea13a27ed 100644
    --- a/kernel/events/core.c
    +++ b/kernel/events/core.c
    @@ -6668,7 +6668,7 @@ static void perf_event_addr_filters_exec(struct perf_event *event, void *data)

    raw_spin_lock_irqsave(&ifh->lock, flags);
    list_for_each_entry(filter, &ifh->list, entry) {
    - if (filter->inode) {
    + if (filter->path.dentry) {
    event->addr_filters_offs[count] = 0;
    restart++;
    }
    @@ -7333,7 +7333,7 @@ static bool perf_addr_filter_match(struct perf_addr_filter *filter,
    struct file *file, unsigned long offset,
    unsigned long size)
    {
    - if (filter->inode != file_inode(file))
    + if (d_inode(filter->path.dentry) != file_inode(file))
    return false;

    if (filter->offset > offset + size)
    @@ -8686,8 +8686,7 @@ static void free_filters_list(struct list_head *filters)
    struct perf_addr_filter *filter, *iter;

    list_for_each_entry_safe(filter, iter, filters, entry) {
    - if (filter->inode)
    - iput(filter->inode);
    + path_put(&filter->path);
    list_del(&filter->entry);
    kfree(filter);
    }
    @@ -8784,7 +8783,7 @@ static void perf_event_addr_filters_apply(struct perf_event *event)
    * Adjust base offset if the filter is associated to a binary
    * that needs to be mapped:
    */
    - if (filter->inode)
    + if (filter->path.dentry)
    event->addr_filters_offs[count] =
    perf_addr_filter_apply(filter, mm);

    @@ -8858,7 +8857,6 @@ perf_event_parse_addr_filter(struct perf_event *event, char *fstr,
    {
    struct perf_addr_filter *filter = NULL;
    char *start, *orig, *filename = NULL;
    - struct path path;
    substring_t args[MAX_OPT_ARGS];
    int state = IF_STATE_ACTION, token;
    unsigned int kernel = 0;
    @@ -8971,19 +8969,18 @@ perf_event_parse_addr_filter(struct perf_event *event, char *fstr,
    goto fail_free_name;

    /* look up the path and grab its inode */
    - ret = kern_path(filename, LOOKUP_FOLLOW, &path);
    + ret = kern_path(filename, LOOKUP_FOLLOW,
    + &filter->path);
    if (ret)
    goto fail_free_name;

    - filter->inode = igrab(d_inode(path.dentry));
    - path_put(&path);
    kfree(filename);
    filename = NULL;

    ret = -EINVAL;
    - if (!filter->inode ||
    - !S_ISREG(filter->inode->i_mode))
    - /* free_filters_list() will iput() */
    + if (!filter->path.dentry ||
    + !S_ISREG(d_inode(filter->path.dentry)
    + ->i_mode))
    goto fail;

    event->addr_filters.nr_file_filters++;
    \
     
     \ /
      Last update: 2018-05-25 11:57    [W:3.354 / U:0.384 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site