user/sven/linux.git/drivers/android/binder_alloc.c, branch v5.4.3

binder: Handle start==NULL in binder_update_page_range()

2019-12-13T07:43:25Z

commit 2a9edd056ed4fbf9d2e797c3fc06335af35bccc4 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: 457b9a6f09f0 ("Staging: android: add binder driver") Signed-off-by: Jann Horn Acked-by: Christian Brauner Link: https://lore.kernel.org/r/20191018205631.248274-3-jannh@google.com Signed-off-by: Greg Kroah-Hartman

binder: Prevent repeated use of ->mmap() via NULL mapping

2019-12-13T07:43:25Z

commit a7a74d7ff55a0c657bc46238b050460b9eacea95 upstream. binder_alloc_mmap_handler() attempts to detect the use of ->mmap() on a binder_proc whose binder_alloc has already been initialized by checking whether alloc->buffer is non-zero. Before commit 880211667b20 ("binder: remove kernel vm_area for buffer space"), alloc->buffer was a kernel mapping address, which is always non-zero, but since that commit, it is a userspace mapping address. A sufficiently privileged user can map /dev/binder at NULL, tricking binder_alloc_mmap_handler() into assuming that the binder_proc has not been mapped yet. This leads to memory unsafety. Luckily, no context on Android has such privileges, and on a typical Linux desktop system, you need to be root to do that. Fix it by using the mapping size instead of the mapping address to distinguish the mapped case. A valid VMA can't have size zero. Fixes: 880211667b20 ("binder: remove kernel vm_area for buffer space") Cc: stable@vger.kernel.org Signed-off-by: Jann Horn Acked-by: Christian Brauner Link: https://lore.kernel.org/r/20191018205631.248274-2-jannh@google.com Signed-off-by: Greg Kroah-Hartman

binder: Fix race between mmap() and binder_alloc_print_pages()

2019-12-13T07:43:23Z

commit 8eb52a1ee37aafd9b796713aa0b3ab9cbc455be3 upstream. binder_alloc_print_pages() iterates over alloc->pages[0..alloc->buffer_size-1] under alloc->mutex. binder_alloc_mmap_handler() writes alloc->pages and alloc->buffer_size without holding that lock, and even writes them before the last bailout point. Unfortunately we can't take the alloc->mutex in the ->mmap() handler because mmap_sem can be taken while alloc->mutex is held. So instead, we have to locklessly check whether the binder_alloc has been fully initialized with binder_alloc_get_vma(), like in binder_alloc_new_buf_locked(). Fixes: 8ef4665aa129 ("android: binder: Add page usage in binder stats") Cc: stable@vger.kernel.org Signed-off-by: Jann Horn Acked-by: Christian Brauner Link: https://lore.kernel.org/r/20191018205631.248274-1-jannh@google.com Signed-off-by: Greg Kroah-Hartman

binder: Don't modify VMA bounds in ->mmap handler

2019-10-17T12:58:44Z

binder_mmap() tries to prevent the creation of overly big binder mappings by silently truncating the size of the VMA to 4MiB. However, this violates the API contract of mmap(). If userspace attempts to create a large binder VMA, and later attempts to unmap that VMA, it will call munmap() on a range beyond the end of the VMA, which may have been allocated to another VMA in the meantime. This can lead to userspace memory corruption. The following sequence of calls leads to a segfault without this commit: int main(void) { int binder_fd = open("/dev/binder", O_RDWR); if (binder_fd == -1) err(1, "open binder"); void *binder_mapping = mmap(NULL, 0x800000UL, PROT_READ, MAP_SHARED, binder_fd, 0); if (binder_mapping == MAP_FAILED) err(1, "mmap binder"); void *data_mapping = mmap(NULL, 0x400000UL, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0); if (data_mapping == MAP_FAILED) err(1, "mmap data"); munmap(binder_mapping, 0x800000UL); *(char*)data_mapping = 1; return 0; } Cc: stable@vger.kernel.org Signed-off-by: Jann Horn Acked-by: Todd Kjos Acked-by: Christian Brauner Link: https://lore.kernel.org/r/20191016150119.154756-1-jannh@google.com Signed-off-by: Greg Kroah-Hartman

binder: Fix comment headers on binder_alloc_prepare_to_free()

2019-10-10T12:39:23Z

binder_alloc_buffer_lookup() doesn't exist and is named "binder_alloc_prepare_to_free()". Correct the code comments to reflect this. Signed-off-by: Joel Fernandes (Google) Reviewed-by: Christian Brauner Link: https://lore.kernel.org/r/20190930201250.139554-1-joel@joelfernandes.org Signed-off-by: Greg Kroah-Hartman

binder: return errors from buffer copy functions

2019-07-01T06:42:47Z

The buffer copy functions assumed the caller would ensure correct alignment and that the memory to be copied was completely within the binder buffer. There have been a few cases discovered by syzkallar where a malformed transaction created by a user could violated the assumptions and resulted in a BUG_ON. The fix is to remove the BUG_ON and always return the error to be handled appropriately by the caller. Acked-by: Martijn Coenen Reported-by: syzbot+3ae18325f96190606754@syzkaller.appspotmail.com Fixes: bde4a19fc04f ("binder: use userspace pointer as base of buffer space") Suggested-by: Dan Carpenter Signed-off-by: Todd Kjos Cc: stable Signed-off-by: Greg Kroah-Hartman

treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 282

2019-06-05T15:36:37Z

Based on 1 normalized pattern(s): this software is licensed under the terms of the gnu general public license version 2 as published by the free software foundation and may be copied distributed and modified under those terms this program is distributed in the hope that it will be useful but without any warranty without even the implied warranty of merchantability or fitness for a particular purpose see the gnu general public license for more details extracted by the scancode license scanner the SPDX license identifier GPL-2.0-only has been chosen to replace the boilerplate/reference in 285 file(s). Signed-off-by: Thomas Gleixner Reviewed-by: Alexios Zavras Reviewed-by: Allison Randal Cc: linux-spdx@vger.kernel.org Link: https://lkml.kernel.org/r/20190529141900.642774971@linutronix.de Signed-off-by: Greg Kroah-Hartman

binder: take read mode of mmap_sem in binder_alloc_free_page()

2019-04-25T09:53:43Z

Restore the behavior of locking mmap_sem for reading in binder_alloc_free_page(), as was first done in commit 3013bf62b67a ("binder: reduce mmap_sem write-side lock"). That change was inadvertently reverted by commit 5cec2d2e5839 ("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: 5cec2d2e5839 ("binder: fix race between munmap() and direct reclaim") Signed-off-by: Tyler Hicks Acked-by: Todd Kjos Signed-off-by: Greg Kroah-Hartman

binder: fix race between munmap() and direct reclaim

2019-03-21T05:51:32Z

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. Signed-off-by: Todd Kjos Reviewed-by: Joel Fernandes (Google) Cc: stable Signed-off-by: Greg Kroah-Hartman

binder: reduce mmap_sem write-side lock

2019-02-19T13:50:11Z

binder has used write-side mmap_sem semaphore to release memory mapped at address space of the process. However, right lock to release pages is down_read, not down_write because page table lock already protects the race for parallel freeing. Please do not use mmap_sem write-side lock which is well known contented lock. Cc: Todd Kjos Cc: Martijn Coenen Cc: Arve Hjønnevåg Signed-off-by: Minchan Kim Signed-off-by: Greg Kroah-Hartman