This is a note to let you know that I've just added the patch titled
Revert "android: binder: stop saving a pointer to the VMA"
to the 5.15-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
revert-android-binder-stop-saving-a-pointer-to-the-vma.patch
and it can be found in the queue-5.15 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From stable-owner@xxxxxxxxxxxxxxx Tue May 30 20:44:03 2023
From: Carlos Llamas <cmllamas@xxxxxxxxxx>
Date: Tue, 30 May 2023 19:43:36 +0000
Subject: Revert "android: binder: stop saving a pointer to the VMA"
To: stable@xxxxxxxxxxxxxxx
Cc: Carlos Llamas <cmllamas@xxxxxxxxxx>, Liam Howlett <liam.howlett@xxxxxxxxxx>, Suren Baghdasaryan <surenb@xxxxxxxxxx>, Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Message-ID: <20230530194338.1683009-3-cmllamas@xxxxxxxxxx>
From: Carlos Llamas <cmllamas@xxxxxxxxxx>
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@xxxxxxxxxx
[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@xxxxxxxxxx>
Cc: Suren Baghdasaryan <surenb@xxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Carlos Llamas <cmllamas@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20230502201220.1756319-2-cmllamas@xxxxxxxxxx
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
[cmllamas: fixed merge conflict in binder_alloc_set_vma()]
Signed-off-by: Carlos Llamas <cmllamas@xxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
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);
Patches currently in stable-queue which might be from stable-owner@xxxxxxxxxxxxxxx are
queue-5.15/revert-android-binder-stop-saving-a-pointer-to-the-vma.patch
queue-5.15/binder-fix-uaf-of-alloc-vma-in-race-with-munmap.patch
queue-5.15/revert-binder_alloc-add-missing-mmap_lock-calls-when-using-the-vma.patch
queue-5.15/binder-add-lockless-binder_alloc_-set-get-_vma.patch
