All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Yang Jihong <yangjihong1@huawei.com>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Sasha Levin <sashal@kernel.org>
Subject: [PATCH 4.9 08/26] perf/core: Fix data race between perf_event_set_output() and perf_mmap_close()
Date: Wed, 27 Jul 2022 18:10:37 +0200	[thread overview]
Message-ID: <20220727160959.472445849@linuxfoundation.org> (raw)
In-Reply-To: <20220727160959.122591422@linuxfoundation.org>

From: Peter Zijlstra <peterz@infradead.org>

[ Upstream commit 68e3c69803dada336893640110cb87221bb01dcf ]

Yang Jihing reported a race between perf_event_set_output() and
perf_mmap_close():

	CPU1					CPU2

	perf_mmap_close(e2)
	  if (atomic_dec_and_test(&e2->rb->mmap_count)) // 1 - > 0
	    detach_rest = true

						ioctl(e1, IOC_SET_OUTPUT, e2)
						  perf_event_set_output(e1, e2)

	  ...
	  list_for_each_entry_rcu(e, &e2->rb->event_list, rb_entry)
	    ring_buffer_attach(e, NULL);
	    // e1 isn't yet added and
	    // therefore not detached

						    ring_buffer_attach(e1, e2->rb)
						      list_add_rcu(&e1->rb_entry,
								   &e2->rb->event_list)

After this; e1 is attached to an unmapped rb and a subsequent
perf_mmap() will loop forever more:

	again:
		mutex_lock(&e->mmap_mutex);
		if (event->rb) {
			...
			if (!atomic_inc_not_zero(&e->rb->mmap_count)) {
				...
				mutex_unlock(&e->mmap_mutex);
				goto again;
			}
		}

The loop in perf_mmap_close() holds e2->mmap_mutex, while the attach
in perf_event_set_output() holds e1->mmap_mutex. As such there is no
serialization to avoid this race.

Change perf_event_set_output() to take both e1->mmap_mutex and
e2->mmap_mutex to alleviate that problem. Additionally, have the loop
in perf_mmap() detach the rb directly, this avoids having to wait for
the concurrent perf_mmap_close() to get around to doing it to make
progress.

Fixes: 9bb5d40cd93c ("perf: Fix mmap() accounting hole")
Reported-by: Yang Jihong <yangjihong1@huawei.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Tested-by: Yang Jihong <yangjihong1@huawei.com>
Link: https://lkml.kernel.org/r/YsQ3jm2GR38SW7uD@worktop.programming.kicks-ass.net
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 kernel/events/core.c | 45 ++++++++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 2466e2ae54dc..58ef731d52c7 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5291,10 +5291,10 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 
 		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.
+			 * Raced against perf_mmap_close(); remove the
+			 * event and try again.
 			 */
+			ring_buffer_attach(event, NULL);
 			mutex_unlock(&event->mmap_mutex);
 			goto again;
 		}
@@ -9542,14 +9542,25 @@ static int perf_copy_attr(struct perf_event_attr __user *uattr,
 	goto out;
 }
 
+static void mutex_lock_double(struct mutex *a, struct mutex *b)
+{
+	if (b < a)
+		swap(a, b);
+
+	mutex_lock(a);
+	mutex_lock_nested(b, SINGLE_DEPTH_NESTING);
+}
+
 static int
 perf_event_set_output(struct perf_event *event, struct perf_event *output_event)
 {
 	struct ring_buffer *rb = NULL;
 	int ret = -EINVAL;
 
-	if (!output_event)
+	if (!output_event) {
+		mutex_lock(&event->mmap_mutex);
 		goto set;
+	}
 
 	/* don't allow circular references */
 	if (event == output_event)
@@ -9587,8 +9598,15 @@ perf_event_set_output(struct perf_event *event, struct perf_event *output_event)
 	    event->pmu != output_event->pmu)
 		goto out;
 
+	/*
+	 * Hold both mmap_mutex to serialize against perf_mmap_close().  Since
+	 * output_event is already on rb->event_list, and the list iteration
+	 * restarts after every removal, it is guaranteed this new event is
+	 * observed *OR* if output_event is already removed, it's guaranteed we
+	 * observe !rb->mmap_count.
+	 */
+	mutex_lock_double(&event->mmap_mutex, &output_event->mmap_mutex);
 set:
-	mutex_lock(&event->mmap_mutex);
 	/* Can't redirect output if we've got an active mmap() */
 	if (atomic_read(&event->mmap_count))
 		goto unlock;
@@ -9598,6 +9616,12 @@ perf_event_set_output(struct perf_event *event, struct perf_event *output_event)
 		rb = ring_buffer_get(output_event);
 		if (!rb)
 			goto unlock;
+
+		/* did we race against perf_mmap_close() */
+		if (!atomic_read(&rb->mmap_count)) {
+			ring_buffer_put(rb);
+			goto unlock;
+		}
 	}
 
 	ring_buffer_attach(event, rb);
@@ -9605,20 +9629,13 @@ perf_event_set_output(struct perf_event *event, struct perf_event *output_event)
 	ret = 0;
 unlock:
 	mutex_unlock(&event->mmap_mutex);
+	if (output_event)
+		mutex_unlock(&output_event->mmap_mutex);
 
 out:
 	return ret;
 }
 
-static void mutex_lock_double(struct mutex *a, struct mutex *b)
-{
-	if (b < a)
-		swap(a, b);
-
-	mutex_lock(a);
-	mutex_lock_nested(b, SINGLE_DEPTH_NESTING);
-}
-
 static int perf_event_set_clock(struct perf_event *event, clockid_t clk_id)
 {
 	bool nmi_safe = false;
-- 
2.35.1




  parent reply	other threads:[~2022-07-27 16:23 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-27 16:10 [PATCH 4.9 00/26] 4.9.325-rc1 review Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 01/26] security,selinux,smack: kill security_task_wait hook Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 02/26] xen/gntdev: Ignore failure to unmap INVALID_GRANT_HANDLE Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 03/26] misc: rtsx_usb: fix use of dma mapped buffer for usb bulk transfer Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 04/26] misc: rtsx_usb: use separate command and response buffers Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 05/26] misc: rtsx_usb: set return value in rsp_buf alloc err path Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 06/26] xfrm: xfrm_policy: fix a possible double xfrm_pols_put() in xfrm_bundle_lookup() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 07/26] power/reset: arm-versatile: Fix refcount leak in versatile_reboot_probe Greg Kroah-Hartman
2022-07-27 16:10 ` Greg Kroah-Hartman [this message]
2022-07-27 16:10 ` [PATCH 4.9 09/26] ip: Fix a data-race around sysctl_fwmark_reflect Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 10/26] tcp/dccp: Fix a data-race around sysctl_tcp_fwmark_accept Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 11/26] tcp: Fix a data-race around sysctl_tcp_probe_threshold Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 12/26] i2c: cadence: Change large transfer count reset logic to be unconditional Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 13/26] igmp: Fix data-races around sysctl_igmp_llm_reports Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 14/26] igmp: Fix a data-race around sysctl_igmp_max_memberships Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 15/26] tcp: Fix a data-race around sysctl_tcp_notsent_lowat Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 16/26] be2net: Fix buffer overflow in be_get_module_eeprom Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 17/26] Revert "Revert "char/random: silence a lockdep splat with printk()"" Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 18/26] mm/mempolicy: fix uninit-value in mpol_rebind_policy() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 19/26] bpf: Make sure mac_header was set before using it Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 20/26] ALSA: memalloc: Align buffer allocations in page size Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 21/26] tty: drivers/tty/, stop using tty_schedule_flip() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 22/26] tty: the rest, " Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 23/26] tty: drop tty_schedule_flip() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 24/26] tty: extract tty_flip_buffer_commit() from tty_flip_buffer_push() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 25/26] tty: use new tty_insert_flip_string_and_push_buffer() in pty_write() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 26/26] net: usb: ax88179_178a needs FLAG_SEND_ZLP Greg Kroah-Hartman
2022-07-27 22:18 ` [PATCH 4.9 00/26] 4.9.325-rc1 review Florian Fainelli
2022-07-28  9:42 ` Pavel Machek
2022-07-28 10:08 ` Naresh Kamboju
2022-07-28 14:32 ` Jon Hunter
2022-07-28 14:52 ` Shuah Khan
2022-07-28 22:57 ` Guenter Roeck

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=20220727160959.472445849@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=yangjihong1@huawei.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.