summaryrefslogtreecommitdiffstats
path: root/drivers/android
AgeCommit message (Collapse)Author
2020-08-11binder: Prevent context manager from incrementing ref 0Jann Horn
commit 4b836a1426cb0f1ef2a6e211d7e553221594f8fc upstream. 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: 457b9a6f09f0 ("Staging: android: add binder driver") Acked-by: Todd Kjos <tkjos@google.com> Signed-off-by: Jann Horn <jannh@google.com> Reviewed-by: Martijn Coenen <maco@android.com> Link: https://lore.kernel.org/r/20200727120424.1627555-1-jannh@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-07-23binder: Don't use mmput() from shrinker function.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 a1b2289cef92ef0e ("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>
2020-06-23binder: fix null deref of proc->contextTodd 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 f0fe2c0f050d ("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: f0fe2c0f050d ("binder: prevent UAF for binderfs devices II") Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Todd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Link: https://lore.kernel.org/r/20200622200715.114382-1-tkjos@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-06-14treewide: replace '---help---' in Kconfig files with 'help'Masahiro Yamada
Since commit 84af7a6194e4 ("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: Masahiro Yamada <masahiroy@kernel.org>
2020-06-09mmap locking API: convert mmap_sem API commentsMichel Lespinasse
Convert comments that reference old mmap_sem APIs to reference corresponding new mmap locking APIs instead. Signed-off-by: Michel Lespinasse <walken@google.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Reviewed-by: Vlastimil Babka <vbabka@suse.cz> Reviewed-by: Davidlohr Bueso <dbueso@suse.de> Reviewed-by: Daniel 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.com Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2020-06-09mmap locking API: use coccinelle to convert mmap_sem rwsem call sitesMichel 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: Michel Lespinasse <walken@google.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Reviewed-by: Daniel Jordan <daniel.m.jordan@oracle.com> Reviewed-by: Laurent Dufour <ldufour@linux.ibm.com> Reviewed-by: Vlastimil 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.com Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2020-04-23binderfs: remove redundant assignment to pointer ctxColin 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: Colin Ian King <colin.king@canonical.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20200402105000.506296-1-colin.king@canonical.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-04-23binderfs: Fix binderfs.c selftest compilation warningTang 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: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com> Link: https://lore.kernel.org/r/20200411145151.5576-1-tangbin@cmss.chinamobile.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-03-23Merge 5.6-rc7 into char-misc-nextGreg Kroah-Hartman
We need the char/misc driver fixes in here as well. Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-03-19binderfs: port to new mount apiChristian 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: Christian Brauner <christian.brauner@ubuntu.com> Reviewed-by: Kees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20200313153427.141789-1-christian.brauner@ubuntu.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-03-11binderfs: use refcount for binder control devices tooChristian 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: f0fe2c0f050d ("binder: prevent UAF for binderfs devices II") Link: https://lore.kernel.org/r/CA+G9fYusdfg7PMfC9Xce-xLT7NiyKSbgojpK35GOm=Pf9jXXrA@mail.gmail.com Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> Cc: stable@vger.kernel.org Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Acked-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20200311105309.1742827-1-christian.brauner@ubuntu.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-03-03binder: prevent UAF for binderfs devices IIChristian Brauner
This is a necessary follow up to the first fix I proposed and we merged in 2669b8b0c79 ("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 51d8a7eca677 ("binder: prevent UAF read in print_binder_transaction_log_entry()"). Fixes: 3ad20fe393b3 ("binder: implement binderfs") Fixes: 2669b8b0c798 ("binder: prevent UAF for binderfs devices") Fixes: 03e2e07e3814 ("binder: Make transaction_log available in binderfs") Related : 51d8a7eca677 ("binder: prevent UAF read in print_binder_transaction_log_entry()") Cc: stable@vger.kernel.org Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Acked-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20200303164340.670054-1-christian.brauner@ubuntu.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-03-03binder: prevent UAF for binderfs devicesChristian 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 51d8a7eca677 ("binder: prevent UAF read in print_binder_transaction_log_entry()"). Fixes: 3ad20fe393b3 ("binder: implement binderfs") Fixes: 03e2e07e3814 ("binder: Make transaction_log available in binderfs") Related : 51d8a7eca677 ("binder: prevent UAF read in print_binder_transaction_log_entry()") Cc: stable@vger.kernel.org Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Acked-by: Todd Kjos <tkjos@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-01-29Merge tag 'for-5.6/io_uring-vfs-2020-01-29' of git://git.kernel.dk/linux-blockLinus Torvalds
Pull io_uring updates from Jens Axboe: - Support for various new opcodes (fallocate, openat, close, statx, fadvise, madvise, openat2, non-vectored read/write, send/recv, and epoll_ctl) - Faster ring quiesce for fileset updates - Optimizations for overflow condition checking - Support for max-sized clamping - Support for probing what opcodes are supported - Support for io-wq backend sharing between "sibling" rings - Support for registering personalities - Lots of little fixes and improvements * tag 'for-5.6/io_uring-vfs-2020-01-29' of git://git.kernel.dk/linux-block: (64 commits) io_uring: add support for epoll_ctl(2) eventpoll: support non-blocking do_epoll_ctl() calls eventpoll: abstract out epoll_ctl() handler io_uring: fix linked command file table usage io_uring: support using a registered personality for commands io_uring: allow registering credentials io_uring: add io-wq workqueue sharing io-wq: allow grabbing existing io-wq io_uring/io-wq: don't use static creds/mm assignments io-wq: make the io_wq ref counted io_uring: fix refcounting with batched allocations at OOM io_uring: add comment for drain_next io_uring: don't attempt to copy iovec for READ/WRITE io_uring: honor IOSQE_ASYNC for linked reqs io_uring: prep req when do IOSQE_ASYNC io_uring: use labeled array init in io_op_defs io_uring: optimise sqe-to-req flags translation io_uring: remove REQ_F_IO_DRAINED io_uring: file switch work needs to get flushed on exit io_uring: hide uring_fd in ctx ...
2020-01-22binder: fix log spam for existing debugfs file creation.Martin Fuzzey
Since commit 43e23b6c0b01 ("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: Martin Fuzzey <martin.fuzzey@flowbird.group> Acked-by: Todd Kjos <tkjos@google.com> Fixes: 43e23b6c0b01 ("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.group Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-01-20fs: move filp_close() outside of __close_fd_get_file()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: Jens Axboe <axboe@kernel.dk>
2019-12-14binder: fix incorrect calculation for num_validTodd 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: bde4a19fc04f ("binder: use userspace pointer as base of buffer space") Signed-off-by: Todd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Link: https://lore.kernel.org/r/20191213202531.55010-1-tkjos@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-12-01Merge tag 'compat-ioctl-5.5' of ↵Linus Torvalds
git://git.kernel.org:/pub/scm/linux/kernel/git/arnd/playground Pull removal of most of fs/compat_ioctl.c from Arnd Bergmann: "As part of the cleanup of some remaining y2038 issues, I came to fs/compat_ioctl.c, which still has a couple of commands that need support for time64_t. In completely unrelated work, I spent time on cleaning up parts of this file in the past, moving things out into drivers instead. After Al Viro reviewed an earlier version of this series and did a lot more of that cleanup, I decided to try to completely eliminate the rest of it and move it all into drivers. This series incorporates some of Al's work and many patches of my own, but in the end stops short of actually removing the last part, which is the scsi ioctl handlers. I have patches for those as well, but they need more testing or possibly a rewrite" * tag 'compat-ioctl-5.5' of git://git.kernel.org:/pub/scm/linux/kernel/git/arnd/playground: (42 commits) scsi: sd: enable compat ioctls for sed-opal pktcdvd: add compat_ioctl handler compat_ioctl: move SG_GET_REQUEST_TABLE handling compat_ioctl: ppp: move simple commands into ppp_generic.c compat_ioctl: handle PPPIOCGIDLE for 64-bit time_t compat_ioctl: move PPPIOCSCOMPRESS to ppp_generic compat_ioctl: unify copy-in of ppp filters tty: handle compat PPP ioctls compat_ioctl: move SIOCOUTQ out of compat_ioctl.c compat_ioctl: handle SIOCOUTQNSD af_unix: add compat_ioctl support compat_ioctl: reimplement SG_IO handling compat_ioctl: move WDIOC handling into wdt drivers fs: compat_ioctl: move FITRIM emulation into file systems gfs2: add compat_ioctl support compat_ioctl: remove unused convert_in_user macro compat_ioctl: remove last RAID handling code compat_ioctl: remove /dev/raw ioctl translation compat_ioctl: remove PCI ioctl translation compat_ioctl: remove joystick ioctl translation ...
2019-11-14binder: Handle start==NULL in binder_update_page_range()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: 457b9a6f09f0 ("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>
2019-11-14binder: Prevent repeated use of ->mmap() via NULL mappingJann 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 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 <jannh@google.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20191018205631.248274-2-jannh@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-11-14binder: Fix race between mmap() and binder_alloc_print_pages()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: 8ef4665aa129 ("android: binder: Add page usage in binder stats") Cc: stable@vger.kernel.org Signed-off-by: Jann Horn <jannh@google.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20191018205631.248274-1-jannh@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-10-27Merge 5.4-rc5 into char-misc-nextGreg Kroah-Hartman
We want the binder fix in here as well for testing and to work on top of. Also handles a merge issue in binder.c to help linux-next out Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-10-23compat_ioctl: move more drivers to compat_ptr_ioctlArnd 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: Jason Gunthorpe <jgg@mellanox.com> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Acked-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Acked-by: David Sterba <dsterba@suse.com> Acked-by: Darren Hart (VMware) <dvhart@infradead.org> Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org> Acked-by: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
2019-10-21binder: Remove incorrect comment about vm_insert_page() behaviorJann 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: a145dd411eb2 ("VM: add "vm_insert_page()" function") Signed-off-by: Jann Horn <jannh@google.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20191018153946.128584-1-jannh@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-10-17binder: Use common definition of SZ_1KJann Horn
SZ_1K has been defined in include/linux/sizes.h since v3.6. Get rid of the duplicate definition. Signed-off-by: Jann Horn <jannh@google.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20191016150119.154756-2-jannh@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-10-17binder: Don't modify VMA bounds in ->mmap handlerJann 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: Jann Horn <jannh@google.com> Acked-by: Todd Kjos <tkjos@google.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20191016150119.154756-1-jannh@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-10-10binder: Fix comment headers on binder_alloc_prepare_to_free()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: Joel Fernandes (Google) <joel@joelfernandes.org> Reviewed-by: Christian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20190930201250.139554-1-joel@joelfernandes.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-10-10binder: prevent UAF read in print_binder_transaction_log_entry()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: Jann Horn <jannh@google.com> Fixes: 03e2e07e3814 ("binder: Make transaction_log available in binderfs") Link: https://lore.kernel.org/r/CAG48ez14Q0-F8LqsvcNbyR2o6gPW8SHXsm4u5jmD9MpsteM2Tw@mail.gmail.com Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org> Acked-by: Todd Kjos <tkjos@google.com> Reviewed-by: Hridya Valsaraju <hridya@google.com> Link: https://lore.kernel.org/r/20191008130159.10161-1-christian.brauner@ubuntu.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-09-04binder: Add binder_proc logging to binderfsHridya 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: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Hridya Valsaraju <hridya@google.com> Link: https://lore.kernel.org/r/20190903161655.107408-5-hridya@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-09-04binder: Make transaction_log available in binderfsHridya 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: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Hridya Valsaraju <hridya@google.com> Link: https://lore.kernel.org/r/20190903161655.107408-4-hridya@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-09-04binder: Add stats, state and transactions filesHridya 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: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Hridya Valsaraju <hridya@google.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20190903161655.107408-3-hridya@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-09-04binder: add a mount option to show global statsHridya 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: Hridya Valsaraju <hridya@google.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20190903161655.107408-2-hridya@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-09-04binder: Add default binder devices through binderfs when configuredHridya 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: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Hridya Valsaraju <hridya@google.com> Reviewed-by: Joel 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.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-09-04binder: Validate the default binderfs device names.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: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Hridya Valsaraju <hridya@google.com> Reviewed-by: Joel 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.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-07-24binder: prevent transactions to context manager from its own process.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: Hridya Valsaraju <hridya@google.com> Acked-by: Todd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Link: https://lore.kernel.org/r/20190715191804.112933-1-hridya@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-07-24binder: Set end of SG buffer area properly.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: Todd Kjos <tkjos@google.com> Fixes: ec74136ded79 ("binder: create node flag to request sender's security context") Signed-off-by: Martijn Coenen <maco@android.com> Cc: stable@vger.kernel.org # 5.1+ Link: https://lore.kernel.org/r/20190709110923.220736-1-maco@android.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-07-01binder: return errors from buffer copy functionsTodd 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: Martijn Coenen <maco@android.com> Reported-by: syzbot+3ae18325f96190606754@syzkaller.appspotmail.com Fixes: bde4a19fc04f ("binder: use userspace pointer as base of buffer space") Suggested-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Todd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-06-23Merge 5.2-rc6 into char-misc-nextGreg Kroah-Hartman
We need the char-misc fixes in here as well. Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-06-22binder: fix memory leak in error pathTodd 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: Todd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-06-13binder: fix possible UAF when freeing bufferTodd 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: Todd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-06-05treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 282Thomas 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: Thomas Gleixner <tglx@linutronix.de> Reviewed-by: Alexios Zavras <alexios.zavras@intel.com> Reviewed-by: Allison Randal <allison@lohutok.net> Cc: linux-spdx@vger.kernel.org Link: https://lkml.kernel.org/r/20190529141900.642774971@linutronix.de Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-05-21treewide: Add SPDX license identifier - Makefile/KconfigThomas 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: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-05-07Merge tag 'char-misc-5.2-rc1-part2' of ↵Linus Torvalds
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc Pull char/misc update part 2 from Greg KH: "Here is the "real" big set of char/misc driver patches for 5.2-rc1 Loads of different driver subsystem stuff in here, all over the places: - thunderbolt driver updates - habanalabs driver updates - nvmem driver updates - extcon driver updates - intel_th driver updates - mei driver updates - coresight driver updates - soundwire driver cleanups and updates - fastrpc driver updates - other minor driver updates - chardev minor fixups Feels like this tree is getting to be a dumping ground of "small driver subsystems" these days. Which is fine with me, if it makes things easier for those subsystem maintainers. All of these have been in linux-next for a while with no reported issues" * tag 'char-misc-5.2-rc1-part2' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc: (255 commits) intel_th: msu: Add current window tracking intel_th: msu: Add a sysfs attribute to trigger window switch intel_th: msu: Correct the block wrap detection intel_th: Add switch triggering support intel_th: gth: Factor out trace start/stop intel_th: msu: Factor out pipeline draining intel_th: msu: Switch over to scatterlist intel_th: msu: Replace open-coded list_{first,last,next}_entry variants intel_th: Only report useful IRQs to subdevices intel_th: msu: Start handling IRQs intel_th: pci: Use MSI interrupt signalling intel_th: Communicate IRQ via resource intel_th: Add "rtit" source device intel_th: Skip subdevices if their MMIO is missing intel_th: Rework resource passing between glue layers and core intel_th: SPDX-ify the documentation intel_th: msu: Fix single mode with IOMMU coresight: funnel: Support static funnel dt-bindings: arm: coresight: Unify funnel DT binding coresight: replicator: Add new device id for static replicator ...
2019-04-25binder: check for overflow when alloc for security contextTodd 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: Todd Kjos <tkjos@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-04-25binder: take read mode of mmap_sem in binder_alloc_free_page()Tyler Hicks
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 <tyhicks@canonical.com> Acked-by: Todd Kjos <tkjos@android.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-03-21binder: fix race between munmap() and direct reclaimTodd 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: Todd Kjos <tkjos@google.com> Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org> Cc: stable <stable@vger.kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-03-21binder: fix BUG_ON found by selinux-testsuiteTodd Kjos
The selinux-testsuite found an issue resulting in a BUG_ON() where a conditional relied on a size_t going negative when checking the validity of a buffer offset. Fixes: 7a67a39320df ("binder: add function to copy binder object from buffer") Reported-by: Paul Moore <paul@paul-moore.com> Tested-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Todd Kjos <tkjos@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-02-19binder: reduce mmap_sem write-side lockMinchan Kim
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 <tkjos@google.com> Cc: Martijn Coenen <maco@android.com> Cc: Arve Hjønnevåg <arve@android.com> Signed-off-by: Minchan Kim <minchan@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-02-15binder: fix handling of misaligned binder objectTodd Kjos
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> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-02-14binder: fix sparse issue in binder_alloc_selftest.cTodd Kjos
Fixes sparse issues reported by the kbuild test robot running on https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-testing: bde4a19fc04f5 ("binder: use userspace pointer as base of buffer space") Error output (drivers/android/binder_alloc_selftest.c): sparse: warning: incorrect type in assignment (different address spaces) sparse: expected void *page_addr sparse: got void [noderef] <asn:1> *user_data sparse: error: subtraction of different types can't work Fixed by adding necessary "__user" tags. Reported-by: kbuild test robot <lkp@intel.com> Signed-off-by: Todd Kjos <tkjos@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>