All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	patches@lists.linux.dev, Liam Howlett <liam.howlett@oracle.com>,
	Suren Baghdasaryan <surenb@google.com>,
	Carlos Llamas <cmllamas@google.com>
Subject: [PATCH 5.15 38/42] Revert "android: binder: stop saving a pointer to the VMA"
Date: Thu,  1 Jun 2023 14:21:25 +0100	[thread overview]
Message-ID: <20230601131938.415893856@linuxfoundation.org> (raw)
In-Reply-To: <20230601131936.699199833@linuxfoundation.org>

From: Carlos Llamas <cmllamas@google.com>

commit c0fd2101781ef761b636769b2f445351f71c3626 upstream.

This reverts commit a43cfc87caaf46710c8027a8c23b8a55f1078f19.

This patch fixed an issue reported by syzkaller in [1]. However, this
turned out to be only a band-aid in binder. The root cause, as bisected
by syzkaller, was fixed by commit 5789151e48ac ("mm/mmap: undo ->mmap()
when mas_preallocate() fails"). We no longer need the patch for binder.

Reverting such patch allows us to have a lockless access to alloc->vma
in specific cases where the mmap_lock is not required. This approach
avoids the contention that caused a performance regression.

[1] https://lore.kernel.org/all/0000000000004a0dbe05e1d749e0@google.com

[cmllamas: resolved conflicts with rework of alloc->mm and removal of
 binder_alloc_set_vma() also fixed comment section]

Fixes: a43cfc87caaf ("android: binder: stop saving a pointer to the VMA")
Cc: Liam Howlett <liam.howlett@oracle.com>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20230502201220.1756319-2-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[cmllamas: fixed merge conflict in binder_alloc_set_vma()]
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/android/binder_alloc.c          |   27 +++++++++++++--------------
 drivers/android/binder_alloc.h          |    2 +-
 drivers/android/binder_alloc_selftest.c |    2 +-
 3 files changed, 15 insertions(+), 16 deletions(-)

--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -213,7 +213,7 @@ static int binder_update_page_range(stru
 
 	if (mm) {
 		mmap_read_lock(mm);
-		vma = vma_lookup(mm, alloc->vma_addr);
+		vma = alloc->vma;
 	}
 
 	if (!vma && need_mm) {
@@ -313,14 +313,12 @@ err_no_vma:
 static inline void binder_alloc_set_vma(struct binder_alloc *alloc,
 		struct vm_area_struct *vma)
 {
-	unsigned long vm_start = 0;
-
-	if (vma) {
-		vm_start = vma->vm_start;
-		mmap_assert_write_locked(alloc->vma_vm_mm);
-	}
-
-	alloc->vma_addr = vm_start;
+	/*
+	 * If we see alloc->vma is not NULL, buffer data structures set up
+	 * completely. Look at smp_rmb side binder_alloc_get_vma.
+	 */
+	smp_wmb();
+	alloc->vma = vma;
 }
 
 static inline struct vm_area_struct *binder_alloc_get_vma(
@@ -328,9 +326,11 @@ static inline struct vm_area_struct *bin
 {
 	struct vm_area_struct *vma = NULL;
 
-	if (alloc->vma_addr)
-		vma = vma_lookup(alloc->vma_vm_mm, alloc->vma_addr);
-
+	if (alloc->vma) {
+		/* Look at description in binder_alloc_set_vma */
+		smp_rmb();
+		vma = alloc->vma;
+	}
 	return vma;
 }
 
@@ -819,8 +819,7 @@ void binder_alloc_deferred_release(struc
 
 	buffers = 0;
 	mutex_lock(&alloc->mutex);
-	BUG_ON(alloc->vma_addr &&
-	       vma_lookup(alloc->vma_vm_mm, alloc->vma_addr));
+	BUG_ON(alloc->vma);
 
 	while ((n = rb_first(&alloc->allocated_buffers))) {
 		buffer = rb_entry(n, struct binder_buffer, rb_node);
--- a/drivers/android/binder_alloc.h
+++ b/drivers/android/binder_alloc.h
@@ -100,7 +100,7 @@ struct binder_lru_page {
  */
 struct binder_alloc {
 	struct mutex mutex;
-	unsigned long vma_addr;
+	struct vm_area_struct *vma;
 	struct mm_struct *vma_vm_mm;
 	void __user *buffer;
 	struct list_head buffers;
--- a/drivers/android/binder_alloc_selftest.c
+++ b/drivers/android/binder_alloc_selftest.c
@@ -287,7 +287,7 @@ void binder_selftest_alloc(struct binder
 	if (!binder_selftest_run)
 		return;
 	mutex_lock(&binder_selftest_lock);
-	if (!binder_selftest_run || !alloc->vma_addr)
+	if (!binder_selftest_run || !alloc->vma)
 		goto done;
 	pr_info("STARTED\n");
 	binder_selftest_alloc_offset(alloc, end_offset, 0);



  parent reply	other threads:[~2023-06-01 13:25 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-01 13:20 [PATCH 5.15 00/42] 5.15.115-rc1 review Greg Kroah-Hartman
2023-06-01 13:20 ` [PATCH 5.15 01/42] power: supply: bq27xxx: expose battery data when CI=1 Greg Kroah-Hartman
2023-06-01 13:20 ` [PATCH 5.15 02/42] power: supply: bq27xxx: Move bq27xxx_battery_update() down Greg Kroah-Hartman
2023-06-01 13:20 ` [PATCH 5.15 03/42] power: supply: bq27xxx: Ensure power_supply_changed() is called on current sign changes Greg Kroah-Hartman
2023-06-01 13:20 ` [PATCH 5.15 04/42] power: supply: bq27xxx: After charger plug in/out wait 0.5s for things to stabilize Greg Kroah-Hartman
2023-06-01 13:20 ` [PATCH 5.15 05/42] power: supply: core: Refactor power_supply_set_input_current_limit_from_supplier() Greg Kroah-Hartman
2023-06-01 13:20 ` [PATCH 5.15 06/42] power: supply: bq24190: Call power_supply_changed() after updating input current Greg Kroah-Hartman
2023-06-01 13:20 ` [PATCH 5.15 07/42] bpf: fix a memory leak in the LRU and LRU_PERCPU hash maps Greg Kroah-Hartman
2023-06-01 13:20 ` [PATCH 5.15 08/42] net/mlx5: devcom only supports 2 ports Greg Kroah-Hartman
2023-06-01 13:20 ` [PATCH 5.15 09/42] net/mlx5e: Fix deadlock in tc route query code Greg Kroah-Hartman
2023-06-01 13:20 ` [PATCH 5.15 10/42] net/mlx5: Devcom, serialize devcom registration Greg Kroah-Hartman
2023-06-01 13:20 ` [PATCH 5.15 11/42] platform/x86: ISST: PUNIT device mapping with Sub-NUMA clustering Greg Kroah-Hartman
2023-06-01 13:20 ` [PATCH 5.15 12/42] platform/x86: ISST: Remove 8 socket limit Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 13/42] net: phy: mscc: enable VSC8501/2 RGMII RX clock Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 14/42] dmaengine: at_xdmac: Move the free desc to the tail of the desc list Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 15/42] dmaengine: at_xdmac: Remove a level of indentation in at_xdmac_tasklet() Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 16/42] dmaengine: at_xdmac: disable/enable clock directly on suspend/resume Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 17/42] dmaengine: at_xdmac: do not resume channels paused by consumers Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 18/42] dmaengine: at_xdmac: restore the content of grws register Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 19/42] KVM: s390: pv: add export before import Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 20/42] KVM: s390: fix race in gmap_make_secure() Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 21/42] net: dsa: introduce helpers for iterating through ports using dp Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 22/42] net: dsa: mt7530: rework mt753[01]_setup Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 23/42] net: dsa: mt7530: split-off common parts from mt7531_setup Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 24/42] net: dsa: mt7530: fix network connectivity with multiple CPU ports Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 25/42] Bonding: add arp_missed_max option Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 26/42] bonding: fix send_peer_notif overflow Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 27/42] binder: fix UAF caused by faulty buffer cleanup Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 28/42] irqchip/mips-gic: Get rid of the reliance on irq_cpu_online() Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 29/42] irqchip/mips-gic: Use raw spinlock for gic_lock Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 30/42] net/mlx5e: Fix SQ wake logic in ptp napi_poll context Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 31/42] xdp: Allow registering memory model without rxq reference Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 32/42] net: page_pool: use in_softirq() instead Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 33/42] page_pool: fix inconsistency for page_pool_ring_[un]lock() Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 34/42] irqchip/mips-gic: Dont touch vl_map if a local interrupt is not routable Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 35/42] xdp: xdp_mem_allocator can be NULL in trace_mem_connect() Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 36/42] bluetooth: Add cmd validity checks at the start of hci_sock_ioctl() Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 37/42] Revert "binder_alloc: add missing mmap_lock calls when using the VMA" Greg Kroah-Hartman
2023-06-01 13:21 ` Greg Kroah-Hartman [this message]
2023-06-01 13:21 ` [PATCH 5.15 39/42] binder: add lockless binder_alloc_(set|get)_vma() Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 40/42] binder: fix UAF of alloc->vma in race with munmap() Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 41/42] ipv{4,6}/raw: fix output xfrm lookup wrt protocol Greg Kroah-Hartman
2023-06-01 13:21 ` [PATCH 5.15 42/42] netfilter: ctnetlink: Support offloaded conntrack entry deletion Greg Kroah-Hartman
2023-06-01 14:12 ` [PATCH 5.15 00/42] 5.15.115-rc1 review Naresh Kamboju

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=20230601131938.415893856@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=cmllamas@google.com \
    --cc=liam.howlett@oracle.com \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=surenb@google.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.