From: Luis Henriques <luis.henriques@canonical.com>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org,
kernel-team@lists.ubuntu.com
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@kernel.org>, Zhouping Liu <zliu@redhat.com>,
Luis Henriques <luis.henriques@canonical.com>
Subject: [PATCH 50/50] perf: Fix mmap() accounting hole
Date: Tue, 2 Jul 2013 12:07:38 +0100 [thread overview]
Message-ID: <1372763258-20664-51-git-send-email-luis.henriques@canonical.com> (raw)
In-Reply-To: <1372763258-20664-1-git-send-email-luis.henriques@canonical.com>
3.5.7.16 -stable review patch. If anyone has any objections, please let me know.
------------------
From: Peter Zijlstra <peterz@infradead.org>
commit 9bb5d40cd93c9dd4be74834b1dcb1ba03629716b upstream.
Vince's fuzzer once again found holes. This time it spotted a leak in
the locked page accounting.
When an event had redirected output and its close() was the last
reference to the buffer we didn't have a vm context to undo accounting.
Change the code to destroy the buffer on the last munmap() and detach
all redirected events at that time. This provides us the right context
to undo the vm accounting.
Reported-and-tested-by: Vince Weaver <vincent.weaver@maine.edu>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20130604084421.GI8923@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Zhouping Liu <zliu@redhat.com>
[ luis: backported to 3.5: adjusted context ]
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
---
kernel/events/core.c | 228 ++++++++++++++++++++++++++++++++---------------
kernel/events/internal.h | 3 +-
2 files changed, 159 insertions(+), 72 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index e758be0..f5505f7 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -193,9 +193,6 @@ static void cpu_ctx_sched_in(struct perf_cpu_context *cpuctx,
static void update_context_time(struct perf_event_context *ctx);
static u64 perf_event_time(struct perf_event *event);
-static void ring_buffer_attach(struct perf_event *event,
- struct ring_buffer *rb);
-
void __weak perf_event_print_debug(void) { }
extern __weak const char *perf_pmu_name(void)
@@ -2852,7 +2849,8 @@ static void free_event_rcu(struct rcu_head *head)
kfree(event);
}
-static bool ring_buffer_put(struct ring_buffer *rb);
+static void ring_buffer_put(struct ring_buffer *rb);
+static void ring_buffer_detach(struct perf_event *event, struct ring_buffer *rb);
static void free_event(struct perf_event *event)
{
@@ -2877,15 +2875,30 @@ static void free_event(struct perf_event *event)
if (has_branch_stack(event)) {
static_key_slow_dec_deferred(&perf_sched_events);
/* is system-wide event */
- if (!(event->attach_state & PERF_ATTACH_TASK))
+ if (!(event->attach_state & PERF_ATTACH_TASK)) {
atomic_dec(&per_cpu(perf_branch_stack_events,
event->cpu));
+ }
}
}
if (event->rb) {
- ring_buffer_put(event->rb);
- event->rb = NULL;
+ struct ring_buffer *rb;
+
+ /*
+ * Can happen when we close an event with re-directed output.
+ *
+ * Since we have a 0 refcount, perf_mmap_close() will skip
+ * over us; possibly making our ring_buffer_put() the last.
+ */
+ mutex_lock(&event->mmap_mutex);
+ rb = event->rb;
+ if (rb) {
+ rcu_assign_pointer(event->rb, NULL);
+ ring_buffer_detach(event, rb);
+ ring_buffer_put(rb); /* could be last */
+ }
+ mutex_unlock(&event->mmap_mutex);
}
if (is_cgroup_event(event))
@@ -3123,30 +3136,13 @@ static unsigned int perf_poll(struct file *file, poll_table *wait)
unsigned int events = POLL_HUP;
/*
- * Race between perf_event_set_output() and perf_poll(): perf_poll()
- * grabs the rb reference but perf_event_set_output() overrides it.
- * Here is the timeline for two threads T1, T2:
- * t0: T1, rb = rcu_dereference(event->rb)
- * t1: T2, old_rb = event->rb
- * t2: T2, event->rb = new rb
- * t3: T2, ring_buffer_detach(old_rb)
- * t4: T1, ring_buffer_attach(rb1)
- * t5: T1, poll_wait(event->waitq)
- *
- * To avoid this problem, we grab mmap_mutex in perf_poll()
- * thereby ensuring that the assignment of the new ring buffer
- * and the detachment of the old buffer appear atomic to perf_poll()
+ * Pin the event->rb by taking event->mmap_mutex; otherwise
+ * perf_event_set_output() can swizzle our rb and make us miss wakeups.
*/
mutex_lock(&event->mmap_mutex);
-
- rcu_read_lock();
- rb = rcu_dereference(event->rb);
- if (rb) {
- ring_buffer_attach(event, rb);
+ rb = event->rb;
+ if (rb)
events = atomic_xchg(&rb->poll, 0);
- }
- rcu_read_unlock();
-
mutex_unlock(&event->mmap_mutex);
poll_wait(file, &event->waitq, wait);
@@ -3462,16 +3458,12 @@ static void ring_buffer_attach(struct perf_event *event,
return;
spin_lock_irqsave(&rb->event_lock, flags);
- if (!list_empty(&event->rb_entry))
- goto unlock;
-
- list_add(&event->rb_entry, &rb->event_list);
-unlock:
+ if (list_empty(&event->rb_entry))
+ list_add(&event->rb_entry, &rb->event_list);
spin_unlock_irqrestore(&rb->event_lock, flags);
}
-static void ring_buffer_detach(struct perf_event *event,
- struct ring_buffer *rb)
+static void ring_buffer_detach(struct perf_event *event, struct ring_buffer *rb)
{
unsigned long flags;
@@ -3490,13 +3482,10 @@ static void ring_buffer_wakeup(struct perf_event *event)
rcu_read_lock();
rb = rcu_dereference(event->rb);
- if (!rb)
- goto unlock;
-
- list_for_each_entry_rcu(event, &rb->event_list, rb_entry)
- wake_up_all(&event->waitq);
-
-unlock:
+ if (rb) {
+ list_for_each_entry_rcu(event, &rb->event_list, rb_entry)
+ wake_up_all(&event->waitq);
+ }
rcu_read_unlock();
}
@@ -3523,23 +3512,14 @@ static struct ring_buffer *ring_buffer_get(struct perf_event *event)
return rb;
}
-static bool ring_buffer_put(struct ring_buffer *rb)
+static void ring_buffer_put(struct ring_buffer *rb)
{
- struct perf_event *event, *n;
- unsigned long flags;
-
if (!atomic_dec_and_test(&rb->refcount))
- return false;
+ return;
- spin_lock_irqsave(&rb->event_lock, flags);
- list_for_each_entry_safe(event, n, &rb->event_list, rb_entry) {
- list_del_init(&event->rb_entry);
- wake_up_all(&event->waitq);
- }
- spin_unlock_irqrestore(&rb->event_lock, flags);
+ WARN_ON_ONCE(!list_empty(&rb->event_list));
call_rcu(&rb->rcu_head, rb_free_rcu);
- return true;
}
static void perf_mmap_open(struct vm_area_struct *vma)
@@ -3547,28 +3527,100 @@ static void perf_mmap_open(struct vm_area_struct *vma)
struct perf_event *event = vma->vm_file->private_data;
atomic_inc(&event->mmap_count);
+ atomic_inc(&event->rb->mmap_count);
}
+/*
+ * A buffer can be mmap()ed multiple times; either directly through the same
+ * event, or through other events by use of perf_event_set_output().
+ *
+ * In order to undo the VM accounting done by perf_mmap() we need to destroy
+ * the buffer here, where we still have a VM context. This means we need
+ * to detach all events redirecting to us.
+ */
static void perf_mmap_close(struct vm_area_struct *vma)
{
struct perf_event *event = vma->vm_file->private_data;
- if (atomic_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex)) {
- 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);
+ 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);
- rcu_assign_pointer(event->rb, NULL);
- ring_buffer_detach(event, rb);
- mutex_unlock(&event->mmap_mutex);
+ atomic_dec(&rb->mmap_count);
+
+ if (!atomic_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
+ return;
+
+ /* Detach current event from the buffer. */
+ rcu_assign_pointer(event->rb, NULL);
+ ring_buffer_detach(event, rb);
+ mutex_unlock(&event->mmap_mutex);
+
+ /* If there's still other mmap()s of this buffer, we're done. */
+ if (atomic_read(&rb->mmap_count)) {
+ ring_buffer_put(rb); /* can't be last */
+ return;
+ }
- 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);
+ /*
+ * No other mmap()s, detach from all other events that might redirect
+ * into the now unreachable buffer. Somewhat complicated by the
+ * fact that rb::event_lock otherwise nests inside mmap_mutex.
+ */
+again:
+ rcu_read_lock();
+ list_for_each_entry_rcu(event, &rb->event_list, rb_entry) {
+ if (!atomic_long_inc_not_zero(&event->refcount)) {
+ /*
+ * This event is en-route to free_event() which will
+ * detach it and remove it from the list.
+ */
+ continue;
}
+ rcu_read_unlock();
+
+ mutex_lock(&event->mmap_mutex);
+ /*
+ * Check we didn't race with perf_event_set_output() which can
+ * swizzle the rb from under us while we were waiting to
+ * acquire mmap_mutex.
+ *
+ * If we find a different rb; ignore this event, a next
+ * iteration will no longer find it on the list. We have to
+ * still restart the iteration to make sure we're not now
+ * iterating the wrong list.
+ */
+ if (event->rb == rb) {
+ rcu_assign_pointer(event->rb, NULL);
+ ring_buffer_detach(event, rb);
+ ring_buffer_put(rb); /* can't be last, we still have one */
+ }
+ mutex_unlock(&event->mmap_mutex);
+ put_event(event);
+
+ /*
+ * Restart the iteration; either we're on the wrong list or
+ * destroyed its integrity by doing a deletion.
+ */
+ goto again;
}
+ rcu_read_unlock();
+
+ /*
+ * It could be there's still a few 0-ref events on the list; they'll
+ * get cleaned up by free_event() -- they'll also still have their
+ * ref on the rb and will free it whenever they are done with it.
+ *
+ * Aside from that, this buffer is 'fully' detached and unmapped,
+ * undo the VM accounting.
+ */
+
+ atomic_long_sub((size >> PAGE_SHIFT) + 1, &mmap_user->locked_vm);
+ vma->vm_mm->pinned_vm -= mmap_locked;
+ free_uid(mmap_user);
+
+ ring_buffer_put(rb); /* could be last */
}
static const struct vm_operations_struct perf_mmap_vmops = {
@@ -3618,10 +3670,24 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
return -EINVAL;
WARN_ON_ONCE(event->ctx->parent_ctx);
+again:
mutex_lock(&event->mmap_mutex);
if (event->rb) {
- if (event->rb->nr_pages != nr_pages)
+ if (event->rb->nr_pages != nr_pages) {
ret = -EINVAL;
+ goto unlock;
+ }
+
+ if (!atomic_inc_not_zero(&event->rb->mmap_count)) {
+ /*
+ * Raced against perf_mmap_close() through
+ * perf_event_set_output(). Try again, hope for better
+ * luck.
+ */
+ mutex_unlock(&event->mmap_mutex);
+ goto again;
+ }
+
goto unlock;
}
@@ -3663,12 +3729,14 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
goto unlock;
}
+ atomic_set(&rb->mmap_count, 1);
rb->mmap_locked = extra;
rb->mmap_user = get_current_user();
atomic_long_add(user_extra, &user->locked_vm);
vma->vm_mm->pinned_vm += extra;
+ ring_buffer_attach(event, rb);
rcu_assign_pointer(event->rb, rb);
perf_event_update_userpage(event);
@@ -3678,6 +3746,10 @@ unlock:
atomic_inc(&event->mmap_count);
mutex_unlock(&event->mmap_mutex);
+ /*
+ * Since pinned accounting is per vm we cannot allow fork() to copy our
+ * vma.
+ */
vma->vm_flags |= VM_DONTCOPY | VM_RESERVED;
vma->vm_ops = &perf_mmap_vmops;
@@ -6167,6 +6239,8 @@ set:
if (atomic_read(&event->mmap_count))
goto unlock;
+ old_rb = event->rb;
+
if (output_event) {
/* get the rb we want to redirect to */
rb = ring_buffer_get(output_event);
@@ -6174,16 +6248,28 @@ set:
goto unlock;
}
- old_rb = event->rb;
- rcu_assign_pointer(event->rb, rb);
if (old_rb)
ring_buffer_detach(event, old_rb);
+
+ if (rb)
+ ring_buffer_attach(event, rb);
+
+ rcu_assign_pointer(event->rb, rb);
+
+ if (old_rb) {
+ ring_buffer_put(old_rb);
+ /*
+ * Since we detached before setting the new rb, so that we
+ * could attach the new rb, we could have missed a wakeup.
+ * Provide it now.
+ */
+ wake_up_all(&event->waitq);
+ }
+
ret = 0;
unlock:
mutex_unlock(&event->mmap_mutex);
- if (old_rb)
- ring_buffer_put(old_rb);
out:
return ret;
}
diff --git a/kernel/events/internal.h b/kernel/events/internal.h
index 04a14cc..b400e64 100644
--- a/kernel/events/internal.h
+++ b/kernel/events/internal.h
@@ -30,7 +30,8 @@ struct ring_buffer {
spinlock_t event_lock;
struct list_head event_list;
- int mmap_locked;
+ atomic_t mmap_count;
+ unsigned long mmap_locked;
struct user_struct *mmap_user;
struct perf_event_mmap_page *user_page;
--
1.8.1.2
prev parent reply other threads:[~2013-07-02 11:08 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-02 11:06 [ 3.5.y.z extended stable ] Linux 3.5.7.16 stable review Luis Henriques
2013-07-02 11:06 ` [PATCH 01/50] clk: remove notifier from list before freeing it Luis Henriques
2013-07-02 11:06 ` [PATCH 02/50] ALSA: usb-audio: work around Android accessory firmware bug Luis Henriques
2013-07-02 11:06 ` [PATCH 03/50] ALSA: usb-audio: Fix invalid volume resolution for Logitech HD Webcam c310 Luis Henriques
2013-07-02 11:06 ` [PATCH 04/50] ncpfs: fix rmdir returns Device or resource busy Luis Henriques
2013-07-02 11:06 ` [PATCH 05/50] tcp: fix tcp_md5_hash_skb_data() Luis Henriques
2013-07-02 11:06 ` [PATCH 06/50] gianfar: add missing iounmap() on error in gianfar_ptp_probe() Luis Henriques
2013-07-02 11:06 ` [PATCH 07/50] ipv6: fix possible crashes in ip6_cork_release() Luis Henriques
2013-07-02 11:06 ` [PATCH 08/50] netlabel: improve domain mapping validation Luis Henriques
2013-07-02 11:06 ` [PATCH 09/50] r8169: fix 8168evl frame padding Luis Henriques
2013-07-02 11:06 ` [PATCH 10/50] r8169: fix offloaded tx checksum for small packets Luis Henriques
2013-07-02 11:06 ` [PATCH 11/50] 8139cp: reset BQL when ring tx ring cleared Luis Henriques
2013-07-02 11:07 ` [PATCH 12/50] tcp: bug fix in proportional rate reduction Luis Henriques
2013-07-02 11:07 ` [PATCH 13/50] net: Block MSG_CMSG_COMPAT in send(m)msg and recv(m)msg Luis Henriques
2013-07-02 11:07 ` [PATCH 14/50] net: Unbreak compat_sys_{send,recv}msg Luis Henriques
2013-07-02 11:07 ` [PATCH 15/50] tcp: xps: fix reordering issues Luis Henriques
2013-07-02 11:07 ` [PATCH 16/50] ip_tunnel: fix kernel panic with icmp_dest_unreach Luis Henriques
2013-07-02 11:07 ` [PATCH 17/50] net: force a reload of first item in hlist_nulls_for_each_entry_rcu Luis Henriques
2013-07-02 11:07 ` [PATCH 18/50] ipv6: assign rt6_info to inet6_ifaddr in init_loopback Luis Henriques
2013-07-02 11:07 ` [PATCH 19/50] net: sctp: fix NULL pointer dereference in socket destruction Luis Henriques
2013-07-02 11:07 ` [PATCH 20/50] team: check return value of team_get_port_by_index_rcu() for NULL Luis Henriques
2013-07-02 11:07 ` [PATCH 21/50] packet: packet_getname_spkt: make sure string is always 0-terminated Luis Henriques
2013-07-02 11:07 ` [PATCH 22/50] l2tp: Fix PPP header erasure and memory leak Luis Henriques
2013-07-02 11:07 ` [PATCH 23/50] l2tp: Fix sendmsg() return value Luis Henriques
2013-07-02 11:07 ` [PATCH 24/50] bonding: rlb mode of bond should not alter ARP originating via bridge Luis Henriques
2013-07-02 11:07 ` [PATCH 25/50] tilepro: work around module link error with gcc 4.7 Luis Henriques
2013-07-02 11:07 ` [PATCH 26/50] Input: cyttsp - fix memcpy size param Luis Henriques
2013-07-02 11:07 ` [PATCH 27/50] target/iscsi: don't corrupt bh_count in iscsit_stop_time2retain_timer() Luis Henriques
2013-07-02 11:07 ` [PATCH 28/50] tcm_qla2xxx: Fix residual for underrun commands that fail Luis Henriques
2013-07-02 11:07 ` [PATCH 29/50] KVM: x86: remove vcpu's CPL check in host-invoked XCR set Luis Henriques
2013-07-02 11:07 ` [PATCH 30/50] x86: fix build error and kconfig for ia32_emulation and binfmt Luis Henriques
2013-07-02 11:07 ` [PATCH 31/50] USB: serial: ti_usb_3410_5052: new device id for Abbot strip port cable Luis Henriques
2013-07-02 11:07 ` [PATCH 32/50] drm/radeon: update lockup tracking when scheduling in empty ring Luis Henriques
2013-07-02 11:07 ` [PATCH 33/50] team: move add to port list before port enablement Luis Henriques
2013-07-02 11:07 ` [PATCH 34/50] net: sh_eth: fix incorrect RX length error if R8A7740 Luis Henriques
2013-07-02 11:07 ` [PATCH 35/50] sctp: fully initialize sctp_outq in sctp_outq_init Luis Henriques
2013-07-02 11:07 ` [PATCH 36/50] ARM: 7755/1: handle user space mapped pages in flush_kernel_dcache_page Luis Henriques
2013-07-02 11:07 ` [PATCH 37/50] iwlwifi: dvm: fix chain noise calibration Luis Henriques
2013-07-02 11:07 ` [PATCH 38/50] Bluetooth: Fix crash in l2cap_build_cmd() with small MTU Luis Henriques
2013-07-02 11:07 ` [PATCH 39/50] hw_breakpoint: Use cpu_possible_mask in {reserve,release}_bp_slot() Luis Henriques
2013-07-02 11:07 ` [PATCH 40/50] ARM: 7772/1: Fix missing flush_kernel_dcache_page() for noMMU Luis Henriques
2013-07-02 11:07 ` [PATCH 41/50] net/tg3: Avoid delay during MMIO access Luis Henriques
2013-07-02 11:07 ` [PATCH 42/50] dlci: acquire rtnl_lock before calling __dev_get_by_name() Luis Henriques
2013-07-02 11:07 ` [PATCH 43/50] dlci: validate the net device in dlci_del() Luis Henriques
2013-07-02 11:07 ` [PATCH 44/50] Bluetooth: btmrvl: fix thread stopping race Luis Henriques
2013-07-02 11:07 ` [PATCH 45/50] pch_uart: fix a deadlock when pch_uart as console Luis Henriques
2013-07-02 11:07 ` [PATCH 46/50] perf: Disable monitoring on setuid processes for regular users Luis Henriques
2013-07-02 11:07 ` [PATCH 47/50] UBIFS: prepare to fix a horrid bug Luis Henriques
2013-07-02 11:07 ` [PATCH 48/50] UBIFS: " Luis Henriques
2013-07-02 11:07 ` [PATCH 49/50] perf: Fix perf mmap bugs Luis Henriques
2013-07-02 11:07 ` Luis Henriques [this message]
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=1372763258-20664-51-git-send-email-luis.henriques@canonical.com \
--to=luis.henriques@canonical.com \
--cc=kernel-team@lists.ubuntu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=stable@vger.kernel.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®