- 29 7月, 2020 1 次提交
-
-
由 Jann Horn 提交于
Binder is designed such that a binder_proc never has references to itself. If this rule is violated, memory corruption can occur when a process sends a transaction to itself; see e.g. <https://syzkaller.appspot.com/bug?extid=09e05aba06723a94d43d>. There is a remaining edgecase through which such a transaction-to-self can still occur from the context of a task with BINDER_SET_CONTEXT_MGR access: - task A opens /dev/binder twice, creating binder_proc instances P1 and P2 - P1 becomes context manager - P2 calls ACQUIRE on the magic handle 0, allocating index 0 in its handle table - P1 dies (by closing the /dev/binder fd and waiting a bit) - P2 becomes context manager - P2 calls ACQUIRE on the magic handle 0, allocating index 1 in its handle table [this triggers a warning: "binder: 1974:1974 tried to acquire reference to desc 0, got 1 instead"] - task B opens /dev/binder once, creating binder_proc instance P3 - P3 calls P2 (via magic handle 0) with (void*)1 as argument (two-way transaction) - P2 receives the handle and uses it to call P3 (two-way transaction) - P3 calls P2 (via magic handle 0) (two-way transaction) - P2 calls P2 (via handle 1) (two-way transaction) And then, if P2 does *NOT* accept the incoming transaction work, but instead closes the binder fd, we get a crash. Solve it by preventing the context manager from using ACQUIRE on ref 0. There shouldn't be any legitimate reason for the context manager to do that. Additionally, print a warning if someone manages to find another way to trigger a transaction-to-self bug in the future. Cc: stable@vger.kernel.org Fixes: 457b9a6f ("Staging: android: add binder driver") Acked-by: NTodd Kjos <tkjos@google.com> Signed-off-by: NJann Horn <jannh@google.com> Reviewed-by: NMartijn Coenen <maco@android.com> Link: https://lore.kernel.org/r/20200727120424.1627555-1-jannh@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 23 7月, 2020 1 次提交
-
-
由 Tetsuo Handa 提交于
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=bc9e7303f537c41b2b0cc2dfcea3fc42964c2d45Reported-by: Nsyzbot <syzbot+1068f09c44d151250c33@syzkaller.appspotmail.com> Reported-by: Nsyzbot <syzbot+e5344baa319c9a96edec@syzkaller.appspotmail.com> Signed-off-by: NTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Reviewed-by: NMichal Hocko <mhocko@suse.com> Acked-by: NTodd Kjos <tkjos@google.com> Acked-by: NChristian 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.jpSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 23 6月, 2020 1 次提交
-
-
由 Todd Kjos 提交于
The binder driver makes the assumption proc->context pointer is invariant after initialization (as documented in the kerneldoc header for struct proc). However, in commit f0fe2c0f ("binder: prevent UAF for binderfs devices II") proc->context is set to NULL during binder_deferred_release(). Another proc was in the middle of setting up a transaction to the dying process and crashed on a NULL pointer deref on "context" which is a local set to &proc->context: new_ref->data.desc = (node == context->binder_context_mgr_node) ? 0 : 1; Here's the stack: [ 5237.855435] Call trace: [ 5237.855441] binder_get_ref_for_node_olocked+0x100/0x2ec [ 5237.855446] binder_inc_ref_for_node+0x140/0x280 [ 5237.855451] binder_translate_binder+0x1d0/0x388 [ 5237.855456] binder_transaction+0x2228/0x3730 [ 5237.855461] binder_thread_write+0x640/0x25bc [ 5237.855466] binder_ioctl_write_read+0xb0/0x464 [ 5237.855471] binder_ioctl+0x30c/0x96c [ 5237.855477] do_vfs_ioctl+0x3e0/0x700 [ 5237.855482] __arm64_sys_ioctl+0x78/0xa4 [ 5237.855488] el0_svc_common+0xb4/0x194 [ 5237.855493] el0_svc_handler+0x74/0x98 [ 5237.855497] el0_svc+0x8/0xc The fix is to move the kfree of the binder_device to binder_free_proc() so the binder_device is freed when we know there are no references remaining on the binder_proc. Fixes: f0fe2c0f ("binder: prevent UAF for binderfs devices II") Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NTodd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Link: https://lore.kernel.org/r/20200622200715.114382-1-tkjos@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 14 6月, 2020 1 次提交
-
-
由 Masahiro Yamada 提交于
Since commit 84af7a61 ("checkpatch: kconfig: prefer 'help' over '---help---'"), the number of '---help---' has been gradually decreasing, but there are still more than 2400 instances. This commit finishes the conversion. While I touched the lines, I also fixed the indentation. There are a variety of indentation styles found. a) 4 spaces + '---help---' b) 7 spaces + '---help---' c) 8 spaces + '---help---' d) 1 space + 1 tab + '---help---' e) 1 tab + '---help---' (correct indentation) f) 1 tab + 1 space + '---help---' g) 1 tab + 2 spaces + '---help---' In order to convert all of them to 1 tab + 'help', I ran the following commend: $ find . -name 'Kconfig*' | xargs sed -i 's/^[[:space:]]*---help---/\thelp/' Signed-off-by: NMasahiro Yamada <masahiroy@kernel.org>
-
- 10 6月, 2020 2 次提交
-
-
由 Michel Lespinasse 提交于
Convert comments that reference old mmap_sem APIs to reference corresponding new mmap locking APIs instead. Signed-off-by: NMichel Lespinasse <walken@google.com> Signed-off-by: NAndrew Morton <akpm@linux-foundation.org> Reviewed-by: NVlastimil Babka <vbabka@suse.cz> Reviewed-by: NDavidlohr Bueso <dbueso@suse.de> Reviewed-by: NDaniel Jordan <daniel.m.jordan@oracle.com> Cc: David Rientjes <rientjes@google.com> Cc: Hugh Dickins <hughd@google.com> Cc: Jason Gunthorpe <jgg@ziepe.ca> Cc: Jerome Glisse <jglisse@redhat.com> Cc: John Hubbard <jhubbard@nvidia.com> Cc: Laurent Dufour <ldufour@linux.ibm.com> Cc: Liam Howlett <Liam.Howlett@oracle.com> Cc: Matthew Wilcox <willy@infradead.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ying Han <yinghan@google.com> Link: http://lkml.kernel.org/r/20200520052908.204642-12-walken@google.comSigned-off-by: NLinus Torvalds <torvalds@linux-foundation.org>
-
由 Michel Lespinasse 提交于
This change converts the existing mmap_sem rwsem calls to use the new mmap locking API instead. The change is generated using coccinelle with the following rule: // spatch --sp-file mmap_lock_api.cocci --in-place --include-headers --dir . @@ expression mm; @@ ( -init_rwsem +mmap_init_lock | -down_write +mmap_write_lock | -down_write_killable +mmap_write_lock_killable | -down_write_trylock +mmap_write_trylock | -up_write +mmap_write_unlock | -downgrade_write +mmap_write_downgrade | -down_read +mmap_read_lock | -down_read_killable +mmap_read_lock_killable | -down_read_trylock +mmap_read_trylock | -up_read +mmap_read_unlock ) -(&mm->mmap_sem) +(mm) Signed-off-by: NMichel Lespinasse <walken@google.com> Signed-off-by: NAndrew Morton <akpm@linux-foundation.org> Reviewed-by: NDaniel Jordan <daniel.m.jordan@oracle.com> Reviewed-by: NLaurent Dufour <ldufour@linux.ibm.com> Reviewed-by: NVlastimil Babka <vbabka@suse.cz> Cc: Davidlohr Bueso <dbueso@suse.de> Cc: David Rientjes <rientjes@google.com> Cc: Hugh Dickins <hughd@google.com> Cc: Jason Gunthorpe <jgg@ziepe.ca> Cc: Jerome Glisse <jglisse@redhat.com> Cc: John Hubbard <jhubbard@nvidia.com> Cc: Liam Howlett <Liam.Howlett@oracle.com> Cc: Matthew Wilcox <willy@infradead.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ying Han <yinghan@google.com> Link: http://lkml.kernel.org/r/20200520052908.204642-5-walken@google.comSigned-off-by: NLinus Torvalds <torvalds@linux-foundation.org>
-
- 23 4月, 2020 2 次提交
-
-
由 Colin Ian King 提交于
The pointer ctx is being initialized with a value that is never read and it is being updated later with a new value. The initialization is redundant and can be removed. Addresses-Coverity: ("Unused value") Signed-off-by: NColin Ian King <colin.king@canonical.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20200402105000.506296-1-colin.king@canonical.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Tang Bin 提交于
Fix missing braces compilation warning in the ARM compiler environment: drivers/android/binderfs.c: In function 'binderfs_fill_super': drivers/android/binderfs.c:650:9: warning: missing braces around initializer [-Wmissing-braces] struct binderfs_device device_info = { 0 }; drivers/android/binderfs.c:650:9: warning: (near initialization for ‘device_info.name’) [-Wmissing-braces] Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NTang Bin <tangbin@cmss.chinamobile.com> Link: https://lore.kernel.org/r/20200411145151.5576-1-tangbin@cmss.chinamobile.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 19 3月, 2020 1 次提交
-
-
由 Christian Brauner 提交于
When I first wrote binderfs the new mount api had not yet landed. Now that it has been around for a little while and a bunch of filesystems have already been ported we should do so too. When Al sent his mount-api-conversion pr he requested that binderfs (and a few others) be ported separately. It's time we port binderfs. We can make use of the new option parser, get nicer infrastructure and it will be easier if we ever add any new mount options. This survives testing with the binderfs selftests: for i in `seq 1 1000`; do ./binderfs_test; done including the new stress tests I sent out for review today: TAP version 13 1..1 # selftests: filesystems/binderfs: binderfs_test # [==========] Running 3 tests from 1 test cases. # [ RUN ] global.binderfs_stress # [ XFAIL! ] Tests are not run as root. Skipping privileged tests # [==========] Running 3 tests from 1 test cases. # [ RUN ] global.binderfs_stress # [ OK ] global.binderfs_stress # [ RUN ] global.binderfs_test_privileged # [ OK ] global.binderfs_test_privileged # [ RUN ] global.binderfs_test_unprivileged # # Allocated new binder device with major 243, minor 4, and name my-binder # # Detected binder version: 8 # [==========] Running 3 tests from 1 test cases. # [ RUN ] global.binderfs_stress # [ OK ] global.binderfs_stress # [ RUN ] global.binderfs_test_privileged # [ OK ] global.binderfs_test_privileged # [ RUN ] global.binderfs_test_unprivileged # [ OK ] global.binderfs_test_unprivileged # [==========] 3 / 3 tests passed. # [ PASSED ] ok 1 selftests: filesystems/binderfs: binderfs_test Cc: Todd Kjos <tkjos@google.com> Signed-off-by: NChristian Brauner <christian.brauner@ubuntu.com> Reviewed-by: NKees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20200313153427.141789-1-christian.brauner@ubuntu.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 12 3月, 2020 1 次提交
-
-
由 Christian Brauner 提交于
Binderfs binder-control devices are cleaned up via binderfs_evict_inode too() which will use refcount_dec_and_test(). However, we missed to set the refcount for binderfs binder-control devices and so we underflowed when the binderfs instance got unmounted. Pretty obvious oversight and should have been part of the more general UAF fix. The good news is that having test cases (suprisingly) helps. Technically, we could detect that we're about to cleanup the binder-control dentry in binderfs_evict_inode() and then simply clean it up. But that makes the assumption that the binder driver itself will never make use of a binderfs binder-control device after the binderfs instance it belongs to has been unmounted and the superblock for it been destroyed. While it is unlikely to ever come to this let's be on the safe side. Performance-wise this also really doesn't matter since the binder-control device is only every really when creating the binderfs filesystem or creating additional binder devices. Both operations are pretty rare. Fixes: f0fe2c0f ("binder: prevent UAF for binderfs devices II") Link: https://lore.kernel.org/r/CA+G9fYusdfg7PMfC9Xce-xLT7NiyKSbgojpK35GOm=Pf9jXXrA@mail.gmail.comReported-by: NNaresh Kamboju <naresh.kamboju@linaro.org> Cc: stable@vger.kernel.org Signed-off-by: NChristian Brauner <christian.brauner@ubuntu.com> Acked-by: NTodd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20200311105309.1742827-1-christian.brauner@ubuntu.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 04 3月, 2020 1 次提交
-
-
由 Christian Brauner 提交于
This is a necessary follow up to the first fix I proposed and we merged in 2669b8b0 ("binder: prevent UAF for binderfs devices"). I have been overly optimistic that the simple fix I proposed would work. But alas, ihold() + iput() won't work since the inodes won't survive the destruction of the superblock. So all we get with my prior fix is a different race with a tinier race-window but it doesn't solve the issue. Fwiw, the problem lies with generic_shutdown_super(). It even has this cozy Al-style comment: if (!list_empty(&sb->s_inodes)) { printk("VFS: Busy inodes after unmount of %s. " "Self-destruct in 5 seconds. Have a nice day...\n", sb->s_id); } On binder_release(), binder_defer_work(proc, BINDER_DEFERRED_RELEASE) is called which punts the actual cleanup operation to a workqueue. At some point, binder_deferred_func() will be called which will end up calling binder_deferred_release() which will retrieve and cleanup the binder_context attach to this struct binder_proc. If we trace back where this binder_context is attached to binder_proc we see that it is set in binder_open() and is taken from the struct binder_device it is associated with. This obviously assumes that the struct binder_device that context is attached to is _never_ freed. While that might be true for devtmpfs binder devices it is most certainly wrong for binderfs binder devices. So, assume binder_open() is called on a binderfs binder devices. We now stash away the struct binder_context associated with that struct binder_devices: proc->context = &binder_dev->context; /* binderfs stashes devices in i_private */ if (is_binderfs_device(nodp)) { binder_dev = nodp->i_private; info = nodp->i_sb->s_fs_info; binder_binderfs_dir_entry_proc = info->proc_log_dir; } else { . . . proc->context = &binder_dev->context; Now let's assume that the binderfs instance for that binder devices is shutdown via umount() and/or the mount namespace associated with it goes away. As long as there is still an fd open for that binderfs binder device things are fine. But let's assume we now close the last fd for that binderfs binder device. Now binder_release() is called and punts to the workqueue. Assume that the workqueue has quite a bit of stuff to do and doesn't get to cleaning up the struct binder_proc and the associated struct binder_context with it for that binderfs binder device right away. In the meantime, the VFS is killing the super block and is ultimately calling sb->evict_inode() which means it will call binderfs_evict_inode() which does: static void binderfs_evict_inode(struct inode *inode) { struct binder_device *device = inode->i_private; struct binderfs_info *info = BINDERFS_I(inode); clear_inode(inode); if (!S_ISCHR(inode->i_mode) || !device) return; mutex_lock(&binderfs_minors_mutex); --info->device_count; ida_free(&binderfs_minors, device->miscdev.minor); mutex_unlock(&binderfs_minors_mutex); kfree(device->context.name); kfree(device); } thereby freeing the struct binder_device including struct binder_context. Now the workqueue finally has time to get around to cleaning up struct binder_proc and is now trying to access the associate struct binder_context. Since it's already freed it will OOPs. Fix this by introducing a refounct on binder devices. This is an alternative fix to 51d8a7ec ("binder: prevent UAF read in print_binder_transaction_log_entry()"). Fixes: 3ad20fe3 ("binder: implement binderfs") Fixes: 2669b8b0 ("binder: prevent UAF for binderfs devices") Fixes: 03e2e07e ("binder: Make transaction_log available in binderfs") Related : 51d8a7ec ("binder: prevent UAF read in print_binder_transaction_log_entry()") Cc: stable@vger.kernel.org Signed-off-by: NChristian Brauner <christian.brauner@ubuntu.com> Acked-by: NTodd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20200303164340.670054-1-christian.brauner@ubuntu.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 03 3月, 2020 1 次提交
-
-
由 Christian Brauner 提交于
On binder_release(), binder_defer_work(proc, BINDER_DEFERRED_RELEASE) is called which punts the actual cleanup operation to a workqueue. At some point, binder_deferred_func() will be called which will end up calling binder_deferred_release() which will retrieve and cleanup the binder_context attach to this struct binder_proc. If we trace back where this binder_context is attached to binder_proc we see that it is set in binder_open() and is taken from the struct binder_device it is associated with. This obviously assumes that the struct binder_device that context is attached to is _never_ freed. While that might be true for devtmpfs binder devices it is most certainly wrong for binderfs binder devices. So, assume binder_open() is called on a binderfs binder devices. We now stash away the struct binder_context associated with that struct binder_devices: proc->context = &binder_dev->context; /* binderfs stashes devices in i_private */ if (is_binderfs_device(nodp)) { binder_dev = nodp->i_private; info = nodp->i_sb->s_fs_info; binder_binderfs_dir_entry_proc = info->proc_log_dir; } else { . . . proc->context = &binder_dev->context; Now let's assume that the binderfs instance for that binder devices is shutdown via umount() and/or the mount namespace associated with it goes away. As long as there is still an fd open for that binderfs binder device things are fine. But let's assume we now close the last fd for that binderfs binder device. Now binder_release() is called and punts to the workqueue. Assume that the workqueue has quite a bit of stuff to do and doesn't get to cleaning up the struct binder_proc and the associated struct binder_context with it for that binderfs binder device right away. In the meantime, the VFS is killing the super block and is ultimately calling sb->evict_inode() which means it will call binderfs_evict_inode() which does: static void binderfs_evict_inode(struct inode *inode) { struct binder_device *device = inode->i_private; struct binderfs_info *info = BINDERFS_I(inode); clear_inode(inode); if (!S_ISCHR(inode->i_mode) || !device) return; mutex_lock(&binderfs_minors_mutex); --info->device_count; ida_free(&binderfs_minors, device->miscdev.minor); mutex_unlock(&binderfs_minors_mutex); kfree(device->context.name); kfree(device); } thereby freeing the struct binder_device including struct binder_context. Now the workqueue finally has time to get around to cleaning up struct binder_proc and is now trying to access the associate struct binder_context. Since it's already freed it will OOPs. Fix this by holding an additional reference to the inode that is only released once the workqueue is done cleaning up struct binder_proc. This is an easy alternative to introducing separate refcounting on struct binder_device which we can always do later if it becomes necessary. This is an alternative fix to 51d8a7ec ("binder: prevent UAF read in print_binder_transaction_log_entry()"). Fixes: 3ad20fe3 ("binder: implement binderfs") Fixes: 03e2e07e ("binder: Make transaction_log available in binderfs") Related : 51d8a7ec ("binder: prevent UAF read in print_binder_transaction_log_entry()") Cc: stable@vger.kernel.org Signed-off-by: NChristian Brauner <christian.brauner@ubuntu.com> Acked-by: NTodd Kjos <tkjos@google.com> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 22 1月, 2020 1 次提交
-
-
由 Martin Fuzzey 提交于
Since commit 43e23b6c ("debugfs: log errors when something goes wrong") debugfs logs attempts to create existing files. However binder attempts to create multiple debugfs files with the same name when a single PID has multiple contexts, this leads to log spamming during an Android boot (17 such messages during boot on my system). Fix this by checking if we already know the PID and only create the debugfs entry for the first context per PID. Do the same thing for binderfs for symmetry. Signed-off-by: NMartin Fuzzey <martin.fuzzey@flowbird.group> Acked-by: NTodd Kjos <tkjos@google.com> Fixes: 43e23b6c ("debugfs: log errors when something goes wrong") Cc: stable <stable@vger.kernel.org> Link: https://lore.kernel.org/r/1578671054-5982-1-git-send-email-martin.fuzzey@flowbird.groupSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 21 1月, 2020 1 次提交
-
-
由 Jens Axboe 提交于
Just one caller of this, and just use filp_close() there manually. This is important to allow async close/removal of the fd. Signed-off-by: NJens Axboe <axboe@kernel.dk>
-
- 14 12月, 2019 1 次提交
-
-
由 Todd Kjos 提交于
For BINDER_TYPE_PTR and BINDER_TYPE_FDA transactions, the num_valid local was calculated incorrectly causing the range check in binder_validate_ptr() to miss out-of-bounds offsets. Fixes: bde4a19f ("binder: use userspace pointer as base of buffer space") Signed-off-by: NTodd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Link: https://lore.kernel.org/r/20191213202531.55010-1-tkjos@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 14 11月, 2019 3 次提交
-
-
由 Jann Horn 提交于
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: NJann Horn <jannh@google.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20191018205631.248274-3-jannh@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Jann Horn 提交于
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 88021166 ("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: 88021166 ("binder: remove kernel vm_area for buffer space") Cc: stable@vger.kernel.org Signed-off-by: NJann Horn <jannh@google.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20191018205631.248274-2-jannh@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Jann Horn 提交于
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: 8ef4665a ("android: binder: Add page usage in binder stats") Cc: stable@vger.kernel.org Signed-off-by: NJann Horn <jannh@google.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20191018205631.248274-1-jannh@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 23 10月, 2019 1 次提交
-
-
由 Arnd Bergmann 提交于
The .ioctl and .compat_ioctl file operations have the same prototype so they can both point to the same function, which works great almost all the time when all the commands are compatible. One exception is the s390 architecture, where a compat pointer is only 31 bit wide, and converting it into a 64-bit pointer requires calling compat_ptr(). Most drivers here will never run in s390, but since we now have a generic helper for it, it's easy enough to use it consistently. I double-checked all these drivers to ensure that all ioctl arguments are used as pointers or are ignored, but are not interpreted as integer values. Acked-by: NJason Gunthorpe <jgg@mellanox.com> Acked-by: NDaniel Vetter <daniel.vetter@ffwll.ch> Acked-by: NMauro Carvalho Chehab <mchehab+samsung@kernel.org> Acked-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> Acked-by: NDavid Sterba <dsterba@suse.com> Acked-by: NDarren Hart (VMware) <dvhart@infradead.org> Acked-by: NJonathan Cameron <Jonathan.Cameron@huawei.com> Acked-by: NBjorn Andersson <bjorn.andersson@linaro.org> Acked-by: NDan Williams <dan.j.williams@intel.com> Signed-off-by: NArnd Bergmann <arnd@arndb.de>
-
- 22 10月, 2019 1 次提交
-
-
由 Jann Horn 提交于
vm_insert_page() does increment the page refcount, and just to be sure, I've confirmed it by printing page_count(page[0].page_ptr) before and after vm_insert_page(). It's 1 before, 2 afterwards, as expected. Fixes: a145dd41 ("VM: add "vm_insert_page()" function") Signed-off-by: NJann Horn <jannh@google.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20191018153946.128584-1-jannh@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 17 10月, 2019 2 次提交
-
-
由 Jann Horn 提交于
SZ_1K has been defined in include/linux/sizes.h since v3.6. Get rid of the duplicate definition. Signed-off-by: NJann Horn <jannh@google.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20191016150119.154756-2-jannh@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Jann Horn 提交于
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: NJann Horn <jannh@google.com> Acked-by: NTodd Kjos <tkjos@google.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20191016150119.154756-1-jannh@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 10 10月, 2019 2 次提交
-
-
由 Joel Fernandes (Google) 提交于
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: NJoel Fernandes (Google) <joel@joelfernandes.org> Reviewed-by: NChristian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20190930201250.139554-1-joel@joelfernandes.orgSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Christian Brauner 提交于
When a binder transaction is initiated on a binder device coming from a binderfs instance, a pointer to the name of the binder device is stashed in the binder_transaction_log_entry's context_name member. Later on it is used to print the name in print_binder_transaction_log_entry(). By the time print_binder_transaction_log_entry() accesses context_name binderfs_evict_inode() might have already freed the associated memory thereby causing a UAF. Do the simple thing and prevent this by copying the name of the binder device instead of stashing a pointer to it. Reported-by: NJann Horn <jannh@google.com> Fixes: 03e2e07e ("binder: Make transaction_log available in binderfs") Link: https://lore.kernel.org/r/CAG48ez14Q0-F8LqsvcNbyR2o6gPW8SHXsm4u5jmD9MpsteM2Tw@mail.gmail.comSigned-off-by: NChristian Brauner <christian.brauner@ubuntu.com> Reviewed-by: NJoel Fernandes (Google) <joel@joelfernandes.org> Acked-by: NTodd Kjos <tkjos@google.com> Reviewed-by: NHridya Valsaraju <hridya@google.com> Link: https://lore.kernel.org/r/20191008130159.10161-1-christian.brauner@ubuntu.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 04 9月, 2019 6 次提交
-
-
由 Hridya Valsaraju 提交于
Currently /sys/kernel/debug/binder/proc contains the debug data for every binder_proc instance. This patch makes this information also available in a binderfs instance mounted with a mount option "stats=global" in addition to debugfs. The patch does not affect the presence of the file in debugfs. If a binderfs instance is mounted at path /dev/binderfs, this file would be present at /dev/binderfs/binder_logs/proc. This change provides an alternate way to access this file when debugfs is not mounted. Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NHridya Valsaraju <hridya@google.com> Link: https://lore.kernel.org/r/20190903161655.107408-5-hridya@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Hridya Valsaraju 提交于
Currently, the binder transaction log files 'transaction_log' and 'failed_transaction_log' live in debugfs at the following locations: /sys/kernel/debug/binder/failed_transaction_log /sys/kernel/debug/binder/transaction_log This patch makes these files also available in a binderfs instance mounted with the mount option "stats=global". It does not affect the presence of these files in debugfs. If a binderfs instance is mounted at path /dev/binderfs, the location of these files will be as follows: /dev/binderfs/binder_logs/failed_transaction_log /dev/binderfs/binder_logs/transaction_log This change provides an alternate option to access these files when debugfs is not mounted. Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NHridya Valsaraju <hridya@google.com> Link: https://lore.kernel.org/r/20190903161655.107408-4-hridya@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Hridya Valsaraju 提交于
The following binder stat files currently live in debugfs. /sys/kernel/debug/binder/state /sys/kernel/debug/binder/stats /sys/kernel/debug/binder/transactions This patch makes these files available in a binderfs instance mounted with the mount option 'stats=global'. For example, if a binderfs instance is mounted at path /dev/binderfs, the above files will be available at the following locations: /dev/binderfs/binder_logs/state /dev/binderfs/binder_logs/stats /dev/binderfs/binder_logs/transactions This provides a way to access them even when debugfs is not mounted. Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NHridya Valsaraju <hridya@google.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20190903161655.107408-3-hridya@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Hridya Valsaraju 提交于
Currently, all binder state and statistics live in debugfs. We need this information even when debugfs is not mounted. This patch adds the mount option 'stats' to enable a binderfs instance to have binder debug information present in the same. 'stats=global' will enable the global binder statistics. In the future, 'stats=local' will enable binder statistics local to the binderfs instance. The two modes 'global' and 'local' will be mutually exclusive. 'stats=global' option is only available for a binderfs instance mounted in the initial user namespace. An attempt to use the option to mount a binderfs instance in another user namespace will return an EPERM error. Signed-off-by: NHridya Valsaraju <hridya@google.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20190903161655.107408-2-hridya@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Hridya Valsaraju 提交于
Currently, since each binderfs instance needs its own private binder devices, every time a binderfs instance is mounted, all the default binder devices need to be created via the BINDER_CTL_ADD IOCTL. This patch aims to add a solution to automatically create the default binder devices for each binderfs instance that gets mounted. To achieve this goal, when CONFIG_ANDROID_BINDERFS is set, the default binder devices specified by CONFIG_ANDROID_BINDER_DEVICES are created in each binderfs instance instead of global devices being created by the binder driver. Co-developed-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NHridya Valsaraju <hridya@google.com> Reviewed-by: NJoel Fernandes (Google) <joel@joelfernandes.org> Link: https://lore.kernel.org/r/20190808222727.132744-2-hridya@google.com Link: https://lore.kernel.org/r/20190904110704.8606-2-christian.brauner@ubuntu.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Hridya Valsaraju 提交于
Length of a binderfs device name cannot exceed BINDERFS_MAX_NAME. This patch adds a check in binderfs_init() to ensure the same for the default binder devices that will be created in every binderfs instance. Co-developed-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NHridya Valsaraju <hridya@google.com> Reviewed-by: NJoel Fernandes (Google) <joel@joelfernandes.org> Link: https://lore.kernel.org/r/20190808222727.132744-3-hridya@google.com Link: https://lore.kernel.org/r/20190904110704.8606-3-christian.brauner@ubuntu.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 24 7月, 2019 2 次提交
-
-
由 Hridya Valsaraju 提交于
Currently, a transaction to context manager from its own process is prevented by checking if its binder_proc struct is the same as that of the sender. However, this would not catch cases where the process opens the binder device again and uses the new fd to send a transaction to the context manager. Reported-by: syzbot+8b3c354d33c4ac78bfad@syzkaller.appspotmail.com Signed-off-by: NHridya Valsaraju <hridya@google.com> Acked-by: NTodd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Link: https://lore.kernel.org/r/20190715191804.112933-1-hridya@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Martijn Coenen 提交于
In case the target node requests a security context, the extra_buffers_size is increased with the size of the security context. But, that size is not available for use by regular scatter-gather buffers; make sure the ending of that buffer is marked correctly. Acked-by: NTodd Kjos <tkjos@google.com> Fixes: ec74136d ("binder: create node flag to request sender's security context") Signed-off-by: NMartijn Coenen <maco@android.com> Cc: stable@vger.kernel.org # 5.1+ Link: https://lore.kernel.org/r/20190709110923.220736-1-maco@android.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 01 7月, 2019 1 次提交
-
-
由 Todd Kjos 提交于
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: NMartijn Coenen <maco@android.com> Reported-by: syzbot+3ae18325f96190606754@syzkaller.appspotmail.com Fixes: bde4a19f ("binder: use userspace pointer as base of buffer space") Suggested-by: NDan Carpenter <dan.carpenter@oracle.com> Signed-off-by: NTodd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 22 6月, 2019 1 次提交
-
-
由 Todd Kjos 提交于
syzkallar found a 32-byte memory leak in a rarely executed error case. The transaction complete work item was not freed if put_user() failed when writing the BR_TRANSACTION_COMPLETE to the user command buffer. Fixed by freeing it before put_user() is called. Reported-by: syzbot+182ce46596c3f2e1eb24@syzkaller.appspotmail.com Signed-off-by: NTodd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 13 6月, 2019 1 次提交
-
-
由 Todd Kjos 提交于
There is a race between the binder driver cleaning up a completed transaction via binder_free_transaction() and a user calling binder_ioctl(BC_FREE_BUFFER) to release a buffer. It doesn't matter which is first but they need to be protected against running concurrently which can result in a UAF. Signed-off-by: NTodd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 05 6月, 2019 1 次提交
-
-
由 Thomas Gleixner 提交于
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: NThomas Gleixner <tglx@linutronix.de> Reviewed-by: NAlexios Zavras <alexios.zavras@intel.com> Reviewed-by: NAllison Randal <allison@lohutok.net> Cc: linux-spdx@vger.kernel.org Link: https://lkml.kernel.org/r/20190529141900.642774971@linutronix.deSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 21 5月, 2019 1 次提交
-
-
由 Thomas Gleixner 提交于
Add SPDX license identifiers to all Make/Kconfig files which: - Have no license information of any form These files fall under the project license, GPL v2 only. The resulting SPDX license identifier is: GPL-2.0-only Signed-off-by: NThomas Gleixner <tglx@linutronix.de> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 26 4月, 2019 1 次提交
-
-
由 Todd Kjos 提交于
When allocating space in the target buffer for the security context, make sure the extra_buffers_size doesn't overflow. This can only happen if the given size is invalid, but an overflow can turn it into a valid size. Fail the transaction if an overflow is detected. Signed-off-by: NTodd Kjos <tkjos@google.com> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 25 4月, 2019 1 次提交
-
-
由 Tyler Hicks 提交于
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: NTyler Hicks <tyhicks@canonical.com> Acked-by: NTodd Kjos <tkjos@android.com> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 21 3月, 2019 1 次提交
-
-
由 Todd Kjos 提交于
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: NTodd Kjos <tkjos@google.com> Reviewed-by: NJoel Fernandes (Google) <joel@joelfernandes.org> Cc: stable <stable@vger.kernel.org> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-