- Jul 29, 2020
-
-
Tetsuo Handa authored
commit f867c771 upstream. syzbot is reporting that mmput() from shrinker function has a risk of deadlock [1], for delayed_uprobe_add() from update_ref_ctr() calls kzalloc(GFP_KERNEL) with delayed_uprobe_lock held, and uprobe_clear_state() from __mmput() also holds delayed_uprobe_lock. Commit a1b2289c ("android: binder: drop lru lock in isolate callback") replaced mmput() with mmput_async() in order to avoid sleeping with spinlock held. But this patch replaces mmput() with mmput_async() in order not to start __mmput() from shrinker context. [1] https://syzkaller.appspot.com/bug?id=bc9e7303f537c41b2b0cc2dfcea3fc42964c2d45 Reported-by:
syzbot <syzbot+1068f09c44d151250c33@syzkaller.appspotmail.com> Reported-by:
syzbot <syzbot+e5344baa319c9a96edec@syzkaller.appspotmail.com> Signed-off-by:
Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Reviewed-by:
Michal Hocko <mhocko@suse.com> Acked-by:
Todd Kjos <tkjos@google.com> Acked-by:
Christian Brauner <christian.brauner@ubuntu.com> Cc: stable <stable@vger.kernel.org> Link: https://lore.kernel.org/r/4ba9adb2-43f5-2de0-22de-f6075c1fab50@i-love.sakura.ne.jp Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- May 02, 2020
-
-
Tyler Hicks authored
commit 60d48857 upstream. Restore the behavior of locking mmap_sem for reading in binder_alloc_free_page(), as was first done in commit 3013bf62 ("binder: reduce mmap_sem write-side lock"). That change was inadvertently reverted by commit 5cec2d2e ("binder: fix race between munmap() and direct reclaim"). In addition, change the name of the label for the error path to accurately reflect that we're taking the lock for reading. Backporting note: This fix is only needed when *both* of the commits mentioned above are applied. That's an unlikely situation since they both landed during the development of v5.1 but only one of them is targeted for stable. Fixes: 5cec2d2e ("binder: fix race between munmap() and direct reclaim") Signed-off-by:
Tyler Hicks <tyhicks@canonical.com> Acked-by:
Todd Kjos <tkjos@android.com> Cc: Guenter Roeck <linux@roeck-us.net> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- Dec 17, 2019
-
-
Jann Horn authored
commit 2a9edd05 upstream. The old loop wouldn't stop when reaching `start` if `start==NULL`, instead continuing backwards to index -1 and crashing. Luckily you need to be highly privileged to map things at NULL, so it's not a big problem. Fix it by adjusting the loop so that the loop variable is always in bounds. This patch is deliberately minimal to simplify backporting, but IMO this function could use a refactor. The jump labels in the second loop body are horrible (the error gotos should be jumping to free_range instead), and both loops would look nicer if they just iterated upwards through indices. And the up_read()+mmput() shouldn't be duplicated like that. Cc: stable@vger.kernel.org Fixes: 457b9a6f ("Staging: android: add binder driver") Signed-off-by:
Jann Horn <jannh@google.com> Acked-by:
Christian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20191018205631.248274-3-jannh@google.com Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- Jun 09, 2019
-
-
Todd Kjos authored
commit 5cec2d2e upstream. An munmap() on a binder device causes binder_vma_close() to be called which clears the alloc->vma pointer. If direct reclaim causes binder_alloc_free_page() to be called, there is a race where alloc->vma is read into a local vma pointer and then used later after the mm->mmap_sem is acquired. This can result in calling zap_page_range() with an invalid vma which manifests as a use-after-free in zap_page_range(). The fix is to check alloc->vma after acquiring the mmap_sem (which we were acquiring anyway) and skip zap_page_range() if it has changed to NULL. Cc: Ben Hutchings <ben.hutchings@codethink.co.uk> Signed-off-by:
Todd Kjos <tkjos@google.com> Reviewed-by:
Joel Fernandes (Google) <joel@joelfernandes.org> Cc: stable <stable@vger.kernel.org> # 4.14 Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Todd Kjos authored
This reverts commit 33c6b9ca. The commit message is for a different patch. Reverting and then adding the same patch back with the correct commit message. Reported-by:
Ben Hutchings <ben.hutchings@codethink.co.uk> Cc: stable <stable@vger.kernel.org> # 4.14 Signed-off-by:
Todd Kjos <tkjos@google.com> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- May 02, 2019
-
-
Todd Kjos authored
commit 26528be6 upstream. Fixes crash found by syzbot: kernel BUG at drivers/android/binder_alloc.c:LINE! (2) Reported-and-tested-by:
<syzbot+55de1eb4975dec156d8f@syzkaller.appspotmail.com> Signed-off-by:
Todd Kjos <tkjos@google.com> Reviewed-by:
Joel Fernandes (Google) <joel@joelfernandes.org> Cc: stable <stable@vger.kernel.org> # 5.0, 4.19, 4.14 Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- Dec 05, 2018
-
-
Todd Kjos authored
commit 7bada55a upstream. Malicious code can attempt to free buffers using the BC_FREE_BUFFER ioctl to binder. There are protections against a user freeing a buffer while in use by the kernel, however there was a window where BC_FREE_BUFFER could be used to free a recently allocated buffer that was not completely initialized. This resulted in a use-after-free detected by KASAN with a malicious test program. This window is closed by setting the buffer's allow_user_free attribute to 0 when the buffer is allocated or when the user has previously freed it instead of waiting for the caller to set it. The problem was that when the struct buffer was recycled, allow_user_free was stale and set to 1 allowing a free to go through. Signed-off-by:
Todd Kjos <tkjos@google.com> Acked-by:
Arve Hjønnevåg <arve@android.com> Cc: stable <stable@vger.kernel.org> # 4.14 Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- Sep 19, 2018
-
-
Minchan Kim authored
commit da1b9564 upstream. There is RaceFuzzer report like below because we have no lock to close below the race between binder_mmap and binder_alloc_new_buf_locked. To close the race, let's use memory barrier so that if someone see alloc->vma is not NULL, alloc->vma_vm_mm should be never NULL. (I didn't add stable mark intentionallybecause standard android userspace libraries that interact with binder (libbinder & libhwbinder) prevent the mmap/ioctl race. - from Todd) " Thread interleaving: CPU0 (binder_alloc_mmap_handler) CPU1 (binder_alloc_new_buf_locked) ===== ===== // drivers/android/binder_alloc.c // #L718 (v4.18-rc3) alloc->vma = vma; // drivers/android/binder_alloc.c // #L346 (v4.18-rc3) if (alloc->vma == NULL) { ... // alloc->vma is not NULL at this point return ERR_PTR(-ESRCH); } ... // #L438 binder_update_page_range(alloc, 0, (void *)PAGE_ALIGN((uintptr_t)buffer->data), end_page_addr); // In binder_update_page_range() #L218 // But still alloc->vma_vm_mm is NULL here if (need_mm && mmget_not_zero(alloc->vma_vm_mm)) alloc->vma_vm_mm = vma->vm_mm; Crash Log: ================================================================== BUG: KASAN: null-ptr-deref in __atomic_add_unless include/asm-generic/atomic-instrumented.h:89 [inline] BUG: KASAN: null-ptr-deref in atomic_add_unless include/linux/atomic.h:533 [inline] BUG: KASAN: null-ptr-deref in mmget_not_zero include/linux/sched/mm.h:75 [inline] BUG: KASAN: null-ptr-deref in binder_update_page_range+0xece/0x18e0 drivers/android/binder_alloc.c:218 Write of size 4 at addr 0000000000000058 by task syz-executor0/11184 CPU: 1 PID: 11184 Comm: syz-executor0 Not tainted 4.18.0-rc3 #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x16e/0x22c lib/dump_stack.c:113 kasan_report_error mm/kasan/report.c:352 [inline] kasan_report+0x163/0x380 mm/kasan/report.c:412 check_memory_region_inline mm/kasan/kasan.c:260 [inline] check_memory_region+0x140/0x1a0 mm/kasan/kasan.c:267 kasan_check_write+0x14/0x20 mm/kasan/kasan.c:278 __atomic_add_unless include/asm-generic/atomic-instrumented.h:89 [inline] atomic_add_unless include/linux/atomic.h:533 [inline] mmget_not_zero include/linux/sched/mm.h:75 [inline] binder_update_page_range+0xece/0x18e0 drivers/android/binder_alloc.c:218 binder_alloc_new_buf_locked drivers/android/binder_alloc.c:443 [inline] binder_alloc_new_buf+0x467/0xc30 drivers/android/binder_alloc.c:513 binder_transaction+0x125b/0x4fb0 drivers/android/binder.c:2957 binder_thread_write+0xc08/0x2770 drivers/android/binder.c:3528 binder_ioctl_write_read.isra.39+0x24f/0x8e0 drivers/android/binder.c:4456 binder_ioctl+0xa86/0xf34 drivers/android/binder.c:4596 vfs_ioctl fs/ioctl.c:46 [inline] do_vfs_ioctl+0x154/0xd40 fs/ioctl.c:686 ksys_ioctl+0x94/0xb0 fs/ioctl.c:701 __do_sys_ioctl fs/ioctl.c:708 [inline] __se_sys_ioctl fs/ioctl.c:706 [inline] __x64_sys_ioctl+0x43/0x50 fs/ioctl.c:706 do_syscall_64+0x167/0x4b0 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe " Signed-off-by:
Todd Kjos <tkjos@google.com> Signed-off-by:
Minchan Kim <minchan@kernel.org> Reviewed-by:
Martijn Coenen <maco@android.com> Cc: stable <stable@vger.kernel.org> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- Feb 03, 2018
-
-
Ganesh Mahendran authored
commit aac6830e upstream. VM_IOREMAP is used to access hardware through a mechanism called I/O mapped memory. Android binder is a IPC machanism which will not access I/O memory. And VM_IOREMAP has alignment requiement which may not needed in binder. __get_vm_area_node() { ... if (flags & VM_IOREMAP) align = 1ul << clamp_t(int, fls_long(size), PAGE_SHIFT, IOREMAP_MAX_ORDER); ... } This patch will save some kernel vm area, especially for 32bit os. In 32bit OS, kernel vm area is only 240MB. We may got below error when launching a app: <3>[ 4482.440053] binder_alloc: binder_alloc_mmap_handler: 15728 8ce67000-8cf65000 get_vm_area failed -12 <3>[ 4483.218817] binder_alloc: binder_alloc_mmap_handler: 15745 8ce67000-8cf65000 get_vm_area failed -12 Signed-off-by:
Ganesh Mahendran <opensource.ganesh@gmail.com> Acked-by:
Martijn Coenen <maco@android.com> Acked-by:
Todd Kjos <tkjos@google.com> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- Oct 21, 2017
-
-
Sherry Yang authored
Don't access next->data in kernel debug message when the next buffer is null. Acked-by:
Arve Hjønnevåg <arve@android.com> Signed-off-by:
Sherry Yang <sherryy@android.com> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Sherry Yang authored
Use binder_alloc struct's mm_struct rather than getting a reference to the mm struct through get_task_mm to avoid a potential deadlock between lru lock, task lock and dentry lock, since a thread can be holding the task lock and the dentry lock while trying to acquire the lru lock. Acked-by:
Arve Hjønnevåg <arve@android.com> Signed-off-by:
Sherry Yang <sherryy@android.com> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- Oct 04, 2017
-
-
Sherry Yang authored
Drop the global lru lock in isolate callback before calling zap_page_range which calls cond_resched, and re-acquire the global lru lock before returning. Also change return code to LRU_REMOVED_RETRY. Use mmput_async when fail to acquire mmap sem in an atomic context. Fix "BUG: sleeping function called from invalid context" errors when CONFIG_DEBUG_ATOMIC_SLEEP is enabled. Also restore mmput_async, which was initially introduced in commit ec8d7c14 ("mm, oom_reaper: do not mmput synchronously from the oom reaper context"), and was removed in commit 21292580 ("mm: oom: let oom_reap_task and exit_mmap run concurrently"). Link: http://lkml.kernel.org/r/20170914182231.90908-1-sherryy@android.com Fixes: f2517eb7 ("android: binder: Add global lru shrinker to binder") Signed-off-by:
Sherry Yang <sherryy@android.com> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org> Reported-by:
Kyle Yan <kyan@codeaurora.org> Acked-by:
Arve Hjønnevåg <arve@android.com> Acked-by:
Michal Hocko <mhocko@suse.com> Cc: Martijn Coenen <maco@google.com> Cc: Todd Kjos <tkjos@google.com> Cc: Riley Andrews <riandrews@android.com> Cc: Ingo Molnar <mingo@kernel.org> Cc: Vlastimil Babka <vbabka@suse.cz> Cc: Hillf Danton <hdanton@sina.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Andrea Arcangeli <aarcange@redhat.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Andy Lutomirski <luto@kernel.org> Cc: Oleg Nesterov <oleg@redhat.com> Cc: Hoeun Ryu <hoeun.ryu@gmail.com> Cc: Christopher Lameter <cl@linux.com> Cc: Vegard Nossum <vegard.nossum@oracle.com> Cc: Frederic Weisbecker <fweisbec@gmail.com> Signed-off-by:
Andrew Morton <akpm@linux-foundation.org> Signed-off-by:
Linus Torvalds <torvalds@linux-foundation.org>
-
- Sep 01, 2017
-
-
Sherry Yang authored
Add the number of active, lru, and free pages for each binder process in binder stats Signed-off-by:
Sherry Yang <sherryy@android.com> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Sherry Yang authored
Fix crash introduced by 74310e06 (android: binder: Move buffer out of area shared with user space) when close is called after open without mmap in between. Reported-by:
kernel test robot <fengguang.wu@intel.com> Fixes: 74310e06 ("android: binder: Move buffer out of area shared with user space") Signed-off-by:
Sherry Yang <sherryy@android.com> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- Aug 28, 2017
-
-
Sherry Yang authored
Add tracepoints in binder transaction allocator to record lru hits and alloc/free page. Signed-off-by:
Sherry Yang <sherryy@android.com> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Sherry Yang authored
Hold on to the pages allocated and mapped for transaction buffers until the system is under memory pressure. When that happens, use linux shrinker to free pages. Without using shrinker, patch "android: binder: Move buffer out of area shared with user space" will cause a significant slow down for small transactions that fit into the first page because free list buffer header used to be inlined with buffer data. In addition to prevent the performance regression for small transactions, this patch improves the performance for transactions that take up more than one page. Modify alloc selftest to work with the shrinker change. Test: Run memory intensive applications (Chrome and Camera) to trigger shrinker callbacks. Binder frees memory as expected. Test: Run binderThroughputTest with high memory pressure option enabled. Signed-off-by:
Sherry Yang <sherryy@android.com> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Sherry Yang authored
Binder driver allocates buffer meta data in a region that is mapped in user space. These meta data contain pointers in the kernel. This patch allocates buffer meta data on the kernel heap that is not mapped in user space, and uses a pointer to refer to the data mapped. Signed-off-by:
Sherry Yang <sherryy@android.com> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Sherry Yang authored
Use helper functions buffer_next and buffer_prev instead of list_entry to get the next and previous buffers. Signed-off-by:
Sherry Yang <sherryy@android.com> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- Jul 17, 2017
-
-
Martijn Coenen authored
Display information about allocated/free space whenever binder buffer allocation fails on synchronous transactions. Signed-off-by:
Martijn Coenen <maco@android.com> Signed-off-by:
Siqi Lin <siqilin@google.com> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Todd Kjos authored
Adds protection against malicious user code freeing the same buffer at the same time which could cause a crash. Cannot happen under normal use. Signed-off-by:
Todd Kjos <tkjos@google.com> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Todd Kjos authored
Add additional information to determine the cause of binder failures. Adds the following to failed transaction log and kernel messages: return_error : value returned for transaction return_error_param : errno returned by binder allocator return_error_line : line number where error detected Also, return BR_DEAD_REPLY if an allocation error indicates a dead proc (-ESRCH) Signed-off-by:
Todd Kjos <tkjos@google.com> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
Todd Kjos authored
Move the binder allocator functionality to its own file Continuation of splitting the binder allocator from the binder driver. Split binder_alloc functions from normal binder functions. Add kernel doc comments to functions declared extern in binder_alloc.h Signed-off-by:
Todd Kjos <tkjos@google.com> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-