From: Greg Kroah-Hartman Date: Thu, 1 Jun 2023 09:21:39 +0000 (+0100) Subject: 5.15-stable patches X-Git-Tag: v5.4.245~21 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=290f582d270ec996002b561d768a93ac7f927f6d;p=thirdparty%2Fkernel%2Fstable-queue.git 5.15-stable patches added patches: binder-add-lockless-binder_alloc_-set-get-_vma.patch binder-fix-uaf-of-alloc-vma-in-race-with-munmap.patch bluetooth-add-cmd-validity-checks-at-the-start-of-hci_sock_ioctl.patch revert-android-binder-stop-saving-a-pointer-to-the-vma.patch revert-binder_alloc-add-missing-mmap_lock-calls-when-using-the-vma.patch --- diff --git a/queue-5.15/binder-add-lockless-binder_alloc_-set-get-_vma.patch b/queue-5.15/binder-add-lockless-binder_alloc_-set-get-_vma.patch new file mode 100644 index 00000000000..de08a6b235b --- /dev/null +++ b/queue-5.15/binder-add-lockless-binder_alloc_-set-get-_vma.patch @@ -0,0 +1,88 @@ +From stable-owner@vger.kernel.org Tue May 30 20:44:04 2023 +From: Carlos Llamas +Date: Tue, 30 May 2023 19:43:37 +0000 +Subject: binder: add lockless binder_alloc_(set|get)_vma() +To: stable@vger.kernel.org +Cc: Carlos Llamas , Liam Howlett , Suren Baghdasaryan , Greg Kroah-Hartman +Message-ID: <20230530194338.1683009-4-cmllamas@google.com> + +From: Carlos Llamas + +commit 0fa53349c3acba0239369ba4cd133740a408d246 upstream. + +Bring back the original lockless design in binder_alloc to determine +whether the buffer setup has been completed by the ->mmap() handler. +However, this time use smp_load_acquire() and smp_store_release() to +wrap all the ordering in a single macro call. + +Also, add comments to make it evident that binder uses alloc->vma to +determine when the binder_alloc has been fully initialized. In these +scenarios acquiring the mmap_lock is not required. + +Fixes: a43cfc87caaf ("android: binder: stop saving a pointer to the VMA") +Cc: Liam Howlett +Cc: Suren Baghdasaryan +Cc: stable@vger.kernel.org +Signed-off-by: Carlos Llamas +Link: https://lore.kernel.org/r/20230502201220.1756319-3-cmllamas@google.com +Signed-off-by: Greg Kroah-Hartman +[cmllamas: fixed minor merge conflict in binder_alloc_set_vma()] +Signed-off-by: Carlos Llamas +Signed-off-by: Greg Kroah-Hartman +--- + drivers/android/binder_alloc.c | 22 +++++++--------------- + 1 file changed, 7 insertions(+), 15 deletions(-) + +--- a/drivers/android/binder_alloc.c ++++ b/drivers/android/binder_alloc.c +@@ -309,29 +309,18 @@ err_no_vma: + return vma ? -ENOMEM : -ESRCH; + } + +- + static inline void binder_alloc_set_vma(struct binder_alloc *alloc, + struct vm_area_struct *vma) + { +- /* +- * 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; ++ /* pairs with smp_load_acquire in binder_alloc_get_vma() */ ++ smp_store_release(&alloc->vma, vma); + } + + static inline struct vm_area_struct *binder_alloc_get_vma( + struct binder_alloc *alloc) + { +- struct vm_area_struct *vma = NULL; +- +- if (alloc->vma) { +- /* Look at description in binder_alloc_set_vma */ +- smp_rmb(); +- vma = alloc->vma; +- } +- return vma; ++ /* pairs with smp_store_release in binder_alloc_set_vma() */ ++ return smp_load_acquire(&alloc->vma); + } + + static bool debug_low_async_space_locked(struct binder_alloc *alloc, int pid) +@@ -394,6 +383,7 @@ static struct binder_buffer *binder_allo + size_t size, data_offsets_size; + int ret; + ++ /* Check binder_alloc is fully initialized */ + if (!binder_alloc_get_vma(alloc)) { + binder_alloc_debug(BINDER_DEBUG_USER_ERROR, + "%d: binder_alloc_buf, no vma\n", +@@ -789,6 +779,8 @@ int binder_alloc_mmap_handler(struct bin + buffer->free = 1; + binder_insert_free_buffer(alloc, buffer); + alloc->free_async_space = alloc->buffer_size / 2; ++ ++ /* Signal binder_alloc is fully initialized */ + binder_alloc_set_vma(alloc, vma); + + return 0; diff --git a/queue-5.15/binder-fix-uaf-of-alloc-vma-in-race-with-munmap.patch b/queue-5.15/binder-fix-uaf-of-alloc-vma-in-race-with-munmap.patch new file mode 100644 index 00000000000..ce9685fc5c1 --- /dev/null +++ b/queue-5.15/binder-fix-uaf-of-alloc-vma-in-race-with-munmap.patch @@ -0,0 +1,147 @@ +From stable-owner@vger.kernel.org Tue May 30 20:44:05 2023 +From: Carlos Llamas +Date: Tue, 30 May 2023 19:43:38 +0000 +Subject: binder: fix UAF of alloc->vma in race with munmap() +To: stable@vger.kernel.org +Cc: Carlos Llamas , Jann Horn , Minchan Kim , Yang Shi , Liam Howlett , Todd Kjos , Greg Kroah-Hartman +Message-ID: <20230530194338.1683009-5-cmllamas@google.com> + +From: Carlos Llamas + +commit d1d8875c8c13517f6fd1ff8d4d3e1ac366a17e07 upstream. + +[ cmllamas: clean forward port from commit 015ac18be7de ("binder: fix + UAF of alloc->vma in race with munmap()") in 5.10 stable. It is needed + in mainline after the revert of commit a43cfc87caaf ("android: binder: + stop saving a pointer to the VMA") as pointed out by Liam. The commit + log and tags have been tweaked to reflect this. ] + +In commit 720c24192404 ("ANDROID: binder: change down_write to +down_read") binder assumed the mmap read lock is sufficient to protect +alloc->vma inside binder_update_page_range(). This used to be accurate +until commit dd2283f2605e ("mm: mmap: zap pages with read mmap_sem in +munmap"), which now downgrades the mmap_lock after detaching the vma +from the rbtree in munmap(). Then it proceeds to teardown and free the +vma with only the read lock held. + +This means that accesses to alloc->vma in binder_update_page_range() now +will race with vm_area_free() in munmap() and can cause a UAF as shown +in the following KASAN trace: + + ================================================================== + BUG: KASAN: use-after-free in vm_insert_page+0x7c/0x1f0 + Read of size 8 at addr ffff16204ad00600 by task server/558 + + CPU: 3 PID: 558 Comm: server Not tainted 5.10.150-00001-gdc8dcf942daa #1 + Hardware name: linux,dummy-virt (DT) + Call trace: + dump_backtrace+0x0/0x2a0 + show_stack+0x18/0x2c + dump_stack+0xf8/0x164 + print_address_description.constprop.0+0x9c/0x538 + kasan_report+0x120/0x200 + __asan_load8+0xa0/0xc4 + vm_insert_page+0x7c/0x1f0 + binder_update_page_range+0x278/0x50c + binder_alloc_new_buf+0x3f0/0xba0 + binder_transaction+0x64c/0x3040 + binder_thread_write+0x924/0x2020 + binder_ioctl+0x1610/0x2e5c + __arm64_sys_ioctl+0xd4/0x120 + el0_svc_common.constprop.0+0xac/0x270 + do_el0_svc+0x38/0xa0 + el0_svc+0x1c/0x2c + el0_sync_handler+0xe8/0x114 + el0_sync+0x180/0x1c0 + + Allocated by task 559: + kasan_save_stack+0x38/0x6c + __kasan_kmalloc.constprop.0+0xe4/0xf0 + kasan_slab_alloc+0x18/0x2c + kmem_cache_alloc+0x1b0/0x2d0 + vm_area_alloc+0x28/0x94 + mmap_region+0x378/0x920 + do_mmap+0x3f0/0x600 + vm_mmap_pgoff+0x150/0x17c + ksys_mmap_pgoff+0x284/0x2dc + __arm64_sys_mmap+0x84/0xa4 + el0_svc_common.constprop.0+0xac/0x270 + do_el0_svc+0x38/0xa0 + el0_svc+0x1c/0x2c + el0_sync_handler+0xe8/0x114 + el0_sync+0x180/0x1c0 + + Freed by task 560: + kasan_save_stack+0x38/0x6c + kasan_set_track+0x28/0x40 + kasan_set_free_info+0x24/0x4c + __kasan_slab_free+0x100/0x164 + kasan_slab_free+0x14/0x20 + kmem_cache_free+0xc4/0x34c + vm_area_free+0x1c/0x2c + remove_vma+0x7c/0x94 + __do_munmap+0x358/0x710 + __vm_munmap+0xbc/0x130 + __arm64_sys_munmap+0x4c/0x64 + el0_svc_common.constprop.0+0xac/0x270 + do_el0_svc+0x38/0xa0 + el0_svc+0x1c/0x2c + el0_sync_handler+0xe8/0x114 + el0_sync+0x180/0x1c0 + + [...] + ================================================================== + +To prevent the race above, revert back to taking the mmap write lock +inside binder_update_page_range(). One might expect an increase of mmap +lock contention. However, binder already serializes these calls via top +level alloc->mutex. Also, there was no performance impact shown when +running the binder benchmark tests. + +Fixes: c0fd2101781e ("Revert "android: binder: stop saving a pointer to the VMA"") +Fixes: dd2283f2605e ("mm: mmap: zap pages with read mmap_sem in munmap") +Reported-by: Jann Horn +Closes: https://lore.kernel.org/all/20230518144052.xkj6vmddccq4v66b@revolver +Cc: +Cc: Minchan Kim +Cc: Yang Shi +Cc: Liam Howlett +Signed-off-by: Carlos Llamas +Acked-by: Todd Kjos +Link: https://lore.kernel.org/r/20230519195950.1775656-1-cmllamas@google.com +Signed-off-by: Greg Kroah-Hartman +Signed-off-by: Carlos Llamas +Signed-off-by: Greg Kroah-Hartman +--- + drivers/android/binder_alloc.c | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +--- a/drivers/android/binder_alloc.c ++++ b/drivers/android/binder_alloc.c +@@ -212,7 +212,7 @@ static int binder_update_page_range(stru + mm = alloc->vma_vm_mm; + + if (mm) { +- mmap_read_lock(mm); ++ mmap_write_lock(mm); + vma = alloc->vma; + } + +@@ -270,7 +270,7 @@ static int binder_update_page_range(stru + trace_binder_alloc_page_end(alloc, index); + } + if (mm) { +- mmap_read_unlock(mm); ++ mmap_write_unlock(mm); + mmput(mm); + } + return 0; +@@ -303,7 +303,7 @@ err_page_ptr_cleared: + } + err_no_vma: + if (mm) { +- mmap_read_unlock(mm); ++ mmap_write_unlock(mm); + mmput(mm); + } + return vma ? -ENOMEM : -ESRCH; diff --git a/queue-5.15/bluetooth-add-cmd-validity-checks-at-the-start-of-hci_sock_ioctl.patch b/queue-5.15/bluetooth-add-cmd-validity-checks-at-the-start-of-hci_sock_ioctl.patch new file mode 100644 index 00000000000..528db2a081f --- /dev/null +++ b/queue-5.15/bluetooth-add-cmd-validity-checks-at-the-start-of-hci_sock_ioctl.patch @@ -0,0 +1,67 @@ +From 000c2fa2c144c499c881a101819cf1936a1f7cf2 Mon Sep 17 00:00:00 2001 +From: Ruihan Li +Date: Sun, 16 Apr 2023 16:02:51 +0800 +Subject: bluetooth: Add cmd validity checks at the start of hci_sock_ioctl() + +From: Ruihan Li + +commit 000c2fa2c144c499c881a101819cf1936a1f7cf2 upstream. + +Previously, channel open messages were always sent to monitors on the first +ioctl() call for unbound HCI sockets, even if the command and arguments +were completely invalid. This can leave an exploitable hole with the abuse +of invalid ioctl calls. + +This commit hardens the ioctl processing logic by first checking if the +command is valid, and immediately returning with an ENOIOCTLCMD error code +if it is not. This ensures that ioctl calls with invalid commands are free +of side effects, and increases the difficulty of further exploitation by +forcing exploitation to find a way to pass a valid command first. + +Signed-off-by: Ruihan Li +Co-developed-by: Marcel Holtmann +Signed-off-by: Marcel Holtmann +Signed-off-by: Luiz Augusto von Dentz +Signed-off-by: Dragos-Marian Panait +Signed-off-by: Greg Kroah-Hartman +--- + net/bluetooth/hci_sock.c | 28 ++++++++++++++++++++++++++++ + 1 file changed, 28 insertions(+) + +--- a/net/bluetooth/hci_sock.c ++++ b/net/bluetooth/hci_sock.c +@@ -980,6 +980,34 @@ static int hci_sock_ioctl(struct socket + + BT_DBG("cmd %x arg %lx", cmd, arg); + ++ /* Make sure the cmd is valid before doing anything */ ++ switch (cmd) { ++ case HCIGETDEVLIST: ++ case HCIGETDEVINFO: ++ case HCIGETCONNLIST: ++ case HCIDEVUP: ++ case HCIDEVDOWN: ++ case HCIDEVRESET: ++ case HCIDEVRESTAT: ++ case HCISETSCAN: ++ case HCISETAUTH: ++ case HCISETENCRYPT: ++ case HCISETPTYPE: ++ case HCISETLINKPOL: ++ case HCISETLINKMODE: ++ case HCISETACLMTU: ++ case HCISETSCOMTU: ++ case HCIINQUIRY: ++ case HCISETRAW: ++ case HCIGETCONNINFO: ++ case HCIGETAUTHINFO: ++ case HCIBLOCKADDR: ++ case HCIUNBLOCKADDR: ++ break; ++ default: ++ return -ENOIOCTLCMD; ++ } ++ + lock_sock(sk); + + if (hci_pi(sk)->channel != HCI_CHANNEL_RAW) { diff --git a/queue-5.15/revert-android-binder-stop-saving-a-pointer-to-the-vma.patch b/queue-5.15/revert-android-binder-stop-saving-a-pointer-to-the-vma.patch new file mode 100644 index 00000000000..b8fb8fd6b82 --- /dev/null +++ b/queue-5.15/revert-android-binder-stop-saving-a-pointer-to-the-vma.patch @@ -0,0 +1,123 @@ +From stable-owner@vger.kernel.org Tue May 30 20:44:03 2023 +From: Carlos Llamas +Date: Tue, 30 May 2023 19:43:36 +0000 +Subject: Revert "android: binder: stop saving a pointer to the VMA" +To: stable@vger.kernel.org +Cc: Carlos Llamas , Liam Howlett , Suren Baghdasaryan , Greg Kroah-Hartman +Message-ID: <20230530194338.1683009-3-cmllamas@google.com> + +From: Carlos Llamas + +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 +Cc: Suren Baghdasaryan +Cc: stable@vger.kernel.org +Signed-off-by: Carlos Llamas +Link: https://lore.kernel.org/r/20230502201220.1756319-2-cmllamas@google.com +Signed-off-by: Greg Kroah-Hartman +[cmllamas: fixed merge conflict in binder_alloc_set_vma()] +Signed-off-by: Carlos Llamas +Signed-off-by: Greg Kroah-Hartman +--- + 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); diff --git a/queue-5.15/revert-binder_alloc-add-missing-mmap_lock-calls-when-using-the-vma.patch b/queue-5.15/revert-binder_alloc-add-missing-mmap_lock-calls-when-using-the-vma.patch new file mode 100644 index 00000000000..c4fc47c8f5b --- /dev/null +++ b/queue-5.15/revert-binder_alloc-add-missing-mmap_lock-calls-when-using-the-vma.patch @@ -0,0 +1,89 @@ +From stable-owner@vger.kernel.org Tue May 30 20:44:01 2023 +From: Carlos Llamas +Date: Tue, 30 May 2023 19:43:35 +0000 +Subject: Revert "binder_alloc: add missing mmap_lock calls when using the VMA" +To: stable@vger.kernel.org +Cc: Carlos Llamas , Liam Howlett , Suren Baghdasaryan , Greg Kroah-Hartman +Message-ID: <20230530194338.1683009-2-cmllamas@google.com> + +From: Carlos Llamas + +commit b15655b12ddca7ade09807f790bafb6fab61b50a upstream. + +This reverts commit 44e602b4e52f70f04620bbbf4fe46ecb40170bde. + +This caused a performance regression particularly when pages are getting +reclaimed. We don't need to acquire the mmap_lock to determine when the +binder buffer has been fully initialized. A subsequent patch will bring +back the lockless approach for this. + +[cmllamas: resolved trivial conflicts with renaming of alloc->mm] + +Fixes: 44e602b4e52f ("binder_alloc: add missing mmap_lock calls when using the VMA") +Cc: Liam Howlett +Cc: Suren Baghdasaryan +Cc: stable@vger.kernel.org +Signed-off-by: Carlos Llamas +Link: https://lore.kernel.org/r/20230502201220.1756319-1-cmllamas@google.com +Signed-off-by: Greg Kroah-Hartman +[cmllamas: revert of original commit 44e602b4e52f applied clean] +Signed-off-by: Carlos Llamas +Signed-off-by: Greg Kroah-Hartman +--- + drivers/android/binder_alloc.c | 31 ++++++++++--------------------- + 1 file changed, 10 insertions(+), 21 deletions(-) + +--- a/drivers/android/binder_alloc.c ++++ b/drivers/android/binder_alloc.c +@@ -394,15 +394,12 @@ static struct binder_buffer *binder_allo + size_t size, data_offsets_size; + int ret; + +- mmap_read_lock(alloc->vma_vm_mm); + if (!binder_alloc_get_vma(alloc)) { +- mmap_read_unlock(alloc->vma_vm_mm); + binder_alloc_debug(BINDER_DEBUG_USER_ERROR, + "%d: binder_alloc_buf, no vma\n", + alloc->pid); + return ERR_PTR(-ESRCH); + } +- mmap_read_unlock(alloc->vma_vm_mm); + + data_offsets_size = ALIGN(data_size, sizeof(void *)) + + ALIGN(offsets_size, sizeof(void *)); +@@ -930,25 +927,17 @@ void binder_alloc_print_pages(struct seq + * Make sure the binder_alloc is fully initialized, otherwise we might + * read inconsistent state. + */ +- +- mmap_read_lock(alloc->vma_vm_mm); +- if (binder_alloc_get_vma(alloc) == NULL) { +- mmap_read_unlock(alloc->vma_vm_mm); +- goto uninitialized; +- } +- +- mmap_read_unlock(alloc->vma_vm_mm); +- for (i = 0; i < alloc->buffer_size / PAGE_SIZE; i++) { +- page = &alloc->pages[i]; +- if (!page->page_ptr) +- free++; +- else if (list_empty(&page->lru)) +- active++; +- else +- lru++; ++ if (binder_alloc_get_vma(alloc) != NULL) { ++ for (i = 0; i < alloc->buffer_size / PAGE_SIZE; i++) { ++ page = &alloc->pages[i]; ++ if (!page->page_ptr) ++ free++; ++ else if (list_empty(&page->lru)) ++ active++; ++ else ++ lru++; ++ } + } +- +-uninitialized: + mutex_unlock(&alloc->mutex); + seq_printf(m, " pages: %d:%d:%d\n", active, lru, free); + seq_printf(m, " pages high watermark: %zu\n", alloc->pages_high); diff --git a/queue-5.15/series b/queue-5.15/series index 011c12ec102..092499c63ca 100644 --- a/queue-5.15/series +++ b/queue-5.15/series @@ -33,3 +33,8 @@ net-page_pool-use-in_softirq-instead.patch page_pool-fix-inconsistency-for-page_pool_ring_-un-l.patch irqchip-mips-gic-don-t-touch-vl_map-if-a-local-inter.patch xdp-xdp_mem_allocator-can-be-null-in-trace_mem_conne.patch +bluetooth-add-cmd-validity-checks-at-the-start-of-hci_sock_ioctl.patch +revert-binder_alloc-add-missing-mmap_lock-calls-when-using-the-vma.patch +revert-android-binder-stop-saving-a-pointer-to-the-vma.patch +binder-add-lockless-binder_alloc_-set-get-_vma.patch +binder-fix-uaf-of-alloc-vma-in-race-with-munmap.patch