From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org, Vince Weaver <vincent.weaver@maine.edu>,
Peter Zijlstra <peterz@infradead.org>,
Al Viro <viro@zeniv.linux.org.uk>,
Paul Mackerras <paulus@samba.org>,
Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
Ingo Molnar <mingo@kernel.org>, Zhouping Liu <zliu@redhat.com>
Subject: [ 12/13] perf: Fix perf mmap bugs
Date: Mon, 1 Jul 2013 13:10:31 -0700 [thread overview]
Message-ID: <20130701200524.527549872@linuxfoundation.org> (raw)
In-Reply-To: <20130701200523.096669485@linuxfoundation.org>
3.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Peter Zijlstra <peterz@infradead.org>
commit 26cb63ad11e04047a64309362674bcbbd6a6f246 upstream.
Vince reported a problem found by his perf specific trinity
fuzzer.
Al noticed 2 problems with perf's mmap():
- it has issues against fork() since we use vma->vm_mm for accounting.
- it has an rb refcount leak on double mmap().
We fix the issues against fork() by using VM_DONTCOPY; I don't
think there's code out there that uses this; we didn't hear
about weird accounting problems/crashes. If we do need this to
work, the previously proposed VM_PINNED could make this work.
Aside from the rb reference leak spotted by Al, Vince's example
prog was indeed doing a double mmap() through the use of
perf_event_set_output().
This exposes another problem, since we now have 2 events with
one buffer, the accounting gets screwy because we account per
event. Fix this by making the buffer responsible for its own
accounting.
[Backporting for 3.4-stable.
VM_RESERVED flag was replaced with pair 'VM_DONTEXPAND | VM_DONTDUMP' in
314e51b9 since 3.7.0-rc1, and 314e51b9 comes from a big patchset, we didn't
backport the patchset, so I restored 'VM_DNOTEXPAND | VM_DONTDUMP' as before:
- vma->vm_flags |= VM_DONTCOPY | VM_DONTEXPAND | VM_DONTDUMP;
+ vma->vm_flags |= VM_DONTCOPY | VM_RESERVED;
-- zliu]
Reported-by: Vince Weaver <vincent.weaver@maine.edu>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Link: http://lkml.kernel.org/r/20130528085548.GA12193@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Zhouping Liu <zliu@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
include/linux/perf_event.h | 3 +--
kernel/events/core.c | 37 ++++++++++++++++++++-----------------
kernel/events/internal.h | 3 +++
3 files changed, 24 insertions(+), 19 deletions(-)
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -950,8 +950,7 @@ struct perf_event {
/* mmap bits */
struct mutex mmap_mutex;
atomic_t mmap_count;
- int mmap_locked;
- struct user_struct *mmap_user;
+
struct ring_buffer *rb;
struct list_head rb_entry;
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2848,7 +2848,7 @@ static void free_event_rcu(struct rcu_he
kfree(event);
}
-static void ring_buffer_put(struct ring_buffer *rb);
+static bool ring_buffer_put(struct ring_buffer *rb);
static void free_event(struct perf_event *event)
{
@@ -3520,13 +3520,13 @@ static struct ring_buffer *ring_buffer_g
return rb;
}
-static void ring_buffer_put(struct ring_buffer *rb)
+static bool ring_buffer_put(struct ring_buffer *rb)
{
struct perf_event *event, *n;
unsigned long flags;
if (!atomic_dec_and_test(&rb->refcount))
- return;
+ return false;
spin_lock_irqsave(&rb->event_lock, flags);
list_for_each_entry_safe(event, n, &rb->event_list, rb_entry) {
@@ -3536,6 +3536,7 @@ static void ring_buffer_put(struct ring_
spin_unlock_irqrestore(&rb->event_lock, flags);
call_rcu(&rb->rcu_head, rb_free_rcu);
+ return true;
}
static void perf_mmap_open(struct vm_area_struct *vma)
@@ -3550,18 +3551,20 @@ static void perf_mmap_close(struct vm_ar
struct perf_event *event = vma->vm_file->private_data;
if (atomic_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex)) {
- unsigned long size = perf_data_size(event->rb);
- struct user_struct *user = event->mmap_user;
struct ring_buffer *rb = event->rb;
+ struct user_struct *mmap_user = rb->mmap_user;
+ int mmap_locked = rb->mmap_locked;
+ unsigned long size = perf_data_size(rb);
- atomic_long_sub((size >> PAGE_SHIFT) + 1, &user->locked_vm);
- vma->vm_mm->pinned_vm -= event->mmap_locked;
rcu_assign_pointer(event->rb, NULL);
ring_buffer_detach(event, rb);
mutex_unlock(&event->mmap_mutex);
- ring_buffer_put(rb);
- free_uid(user);
+ if (ring_buffer_put(rb)) {
+ atomic_long_sub((size >> PAGE_SHIFT) + 1, &mmap_user->locked_vm);
+ vma->vm_mm->pinned_vm -= mmap_locked;
+ free_uid(mmap_user);
+ }
}
}
@@ -3614,9 +3617,7 @@ static int perf_mmap(struct file *file,
WARN_ON_ONCE(event->ctx->parent_ctx);
mutex_lock(&event->mmap_mutex);
if (event->rb) {
- if (event->rb->nr_pages == nr_pages)
- atomic_inc(&event->rb->refcount);
- else
+ if (event->rb->nr_pages != nr_pages)
ret = -EINVAL;
goto unlock;
}
@@ -3658,12 +3659,14 @@ static int perf_mmap(struct file *file,
ret = -ENOMEM;
goto unlock;
}
- rcu_assign_pointer(event->rb, rb);
+
+ rb->mmap_locked = extra;
+ rb->mmap_user = get_current_user();
atomic_long_add(user_extra, &user->locked_vm);
- event->mmap_locked = extra;
- event->mmap_user = get_current_user();
- vma->vm_mm->pinned_vm += event->mmap_locked;
+ vma->vm_mm->pinned_vm += extra;
+
+ rcu_assign_pointer(event->rb, rb);
perf_event_update_userpage(event);
@@ -3672,7 +3675,7 @@ unlock:
atomic_inc(&event->mmap_count);
mutex_unlock(&event->mmap_mutex);
- vma->vm_flags |= VM_RESERVED;
+ vma->vm_flags |= VM_DONTCOPY | VM_RESERVED;
vma->vm_ops = &perf_mmap_vmops;
return ret;
--- a/kernel/events/internal.h
+++ b/kernel/events/internal.h
@@ -30,6 +30,9 @@ struct ring_buffer {
spinlock_t event_lock;
struct list_head event_list;
+ int mmap_locked;
+ struct user_struct *mmap_user;
+
struct perf_event_mmap_page *user_page;
void *data_pages[0];
};
next prev parent reply other threads:[~2013-07-01 20:14 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-01 20:10 [ 00/13] 3.4.52-stable review Greg Kroah-Hartman
2013-07-01 20:10 ` [ 01/13] ARM: 7755/1: handle user space mapped pages in flush_kernel_dcache_page Greg Kroah-Hartman
2013-07-01 20:10 ` [ 02/13] ARM: 7772/1: Fix missing flush_kernel_dcache_page() for noMMU Greg Kroah-Hartman
2013-07-01 20:10 ` [ 03/13] Bluetooth: Fix crash in l2cap_build_cmd() with small MTU Greg Kroah-Hartman
2013-07-01 20:10 ` [ 04/13] hw_breakpoint: Use cpu_possible_mask in {reserve,release}_bp_slot() Greg Kroah-Hartman
2013-07-01 20:10 ` [ 05/13] dlci: acquire rtnl_lock before calling __dev_get_by_name() Greg Kroah-Hartman
2013-07-01 20:10 ` [ 06/13] dlci: validate the net device in dlci_del() Greg Kroah-Hartman
2013-07-01 20:10 ` [ 07/13] net/tg3: Avoid delay during MMIO access Greg Kroah-Hartman
2013-07-02 9:06 ` Luis Henriques
2013-07-03 17:53 ` Greg Kroah-Hartman
2013-07-01 20:10 ` [ 08/13] perf: Disable monitoring on setuid processes for regular users Greg Kroah-Hartman
2013-07-01 20:10 ` [ 09/13] UBIFS: prepare to fix a horrid bug Greg Kroah-Hartman
2013-07-01 20:10 ` [ 10/13] UBIFS: " Greg Kroah-Hartman
2013-07-01 20:10 ` [ 11/13] pch_uart: fix a deadlock when pch_uart as console Greg Kroah-Hartman
2013-07-01 20:10 ` Greg Kroah-Hartman [this message]
2013-07-01 20:10 ` [ 13/13] perf: Fix mmap() accounting hole Greg Kroah-Hartman
2013-07-02 14:39 ` [ 00/13] 3.4.52-stable review Guenter Roeck
2013-07-02 18:55 ` Greg Kroah-Hartman
2013-07-02 21:25 ` Guenter Roeck
2013-07-02 18:47 ` Shuah Khan
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=20130701200524.527549872@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=acme@ghostprotocols.net \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=paulus@samba.org \
--cc=peterz@infradead.org \
--cc=stable@vger.kernel.org \
--cc=vincent.weaver@maine.edu \
--cc=viro@zeniv.linux.org.uk \
--cc=zliu@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.