summaryrefslogtreecommitdiff
path: root/drivers/android
AgeCommit message (Collapse)AuthorFilesLines
2022-01-27binder: avoid potential data leakage when copying txnTodd Kjos1-24/+70
[ Upstream commit 6d98eb95b450a75adb4516a1d33652dc78d2b20c ] Transactions are copied from the sender to the target first and objects like BINDER_TYPE_PTR and BINDER_TYPE_FDA are then fixed up. This means there is a short period where the sender's version of these objects are visible to the target prior to the fixups. Instead of copying all of the data first, copy data only after any needed fixups have been applied. Fixes: 457b9a6f09f0 ("Staging: android: add binder driver") Reviewed-by: Martijn Coenen <maco@android.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20211130185152.437403-3-tkjos@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-01-27binder: fix handling of error during copyTodd Kjos1-2/+2
[ Upstream commit fe6b1869243f23a485a106c214bcfdc7aa0ed593 ] If a memory copy function fails to copy the whole buffer, a positive integar with the remaining bytes is returned. In binder_translate_fd_array() this can result in an fd being skipped due to the failed copy, but the loop continues processing fds since the early return condition expects a negative integer on error. Fix by returning "ret > 0 ? -EINVAL : ret" to handle this case. Fixes: bb4a2e48d510 ("binder: return errors from buffer copy functions") Suggested-by: Dan Carpenter <dan.carpenter@oracle.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20211130185152.437403-2-tkjos@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-01-05binder: fix async_free_space accounting for empty parcelsTodd Kjos1-1/+1
commit cfd0d84ba28c18b531648c9d4a35ecca89ad9901 upstream. In 4.13, commit 74310e06be4d ("android: binder: Move buffer out of area shared with user space") fixed a kernel structure visibility issue. As part of that patch, sizeof(void *) was used as the buffer size for 0-length data payloads so the driver could detect abusive clients sending 0-length asynchronous transactions to a server by enforcing limits on async_free_size. Unfortunately, on the "free" side, the accounting of async_free_space did not add the sizeof(void *) back. The result was that up to 8-bytes of async_free_space were leaked on every async transaction of 8-bytes or less. These small transactions are uncommon, so this accounting issue has gone undetected for several years. The fix is to use "buffer_size" (the allocated buffer size) instead of "size" (the logical buffer size) when updating the async_free_space during the free operation. These are the same except for this corner case of asynchronous transactions with payloads < 8 bytes. Fixes: 74310e06be4d ("android: binder: Move buffer out of area shared with user space") Signed-off-by: Todd Kjos <tkjos@google.com> Cc: stable@vger.kernel.org # 4.14+ Link: https://lore.kernel.org/r/20211220190150.2107077-1-tkjos@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-12-14binder: use wake_up_pollfree()Eric Biggers1-12/+9
commit a880b28a71e39013e357fd3adccd1d8a31bc69a8 upstream. wake_up_poll() uses nr_exclusive=1, so it's not guaranteed to wake up all exclusive waiters. Yet, POLLFREE *must* wake up all waiters. epoll and aio poll are fortunately not affected by this, but it's very fragile. Thus, the new function wake_up_pollfree() has been introduced. Convert binder to use wake_up_pollfree(). Reported-by: Linus Torvalds <torvalds@linux-foundation.org> Fixes: f5cb779ba163 ("ANDROID: binder: remove waitqueue when thread exits.") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20211209010455.42744-3-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-12-01binder: fix test regression due to sender_euid changeTodd Kjos1-1/+1
commit c21a80ca0684ec2910344d72556c816cb8940c01 upstream. This is a partial revert of commit 29bc22ac5e5b ("binder: use euid from cred instead of using task"). Setting sender_euid using proc->cred caused some Android system test regressions that need further investigation. It is a partial reversion because subsequent patches rely on proc->cred. Fixes: 29bc22ac5e5b ("binder: use euid from cred instead of using task") Cc: stable@vger.kernel.org # 4.4+ Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Todd Kjos <tkjos@google.com> Change-Id: I9b1769a3510fed250bb21859ef8beebabe034c66 Link: https://lore.kernel.org/r/20211112180720.2858135-1-tkjos@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-11-12binder: don't detect sender/target during buffer cleanupTodd Kjos1-7/+7
commit 32e9f56a96d8d0f23cb2aeb2a3cd18d40393e787 upstream. When freeing txn buffers, binder_transaction_buffer_release() attempts to detect whether the current context is the target by comparing current->group_leader to proc->tsk. This is an unreliable test. Instead explicitly pass an 'is_failure' boolean. Detecting the sender was being used as a way to tell if the transaction failed to be sent. When cleaning up after failing to send a transaction, there is no need to close the fds associated with a BINDER_TYPE_FDA object. Now 'is_failure' can be used to accurately detect this case. Fixes: 44d8047f1d87 ("binder: use standard functions to allocate fds") Cc: stable <stable@vger.kernel.org> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20211015233811.3532235-1-tkjos@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-11-12binder: use cred instead of task for getsecidTodd Kjos1-10/+1
commit 4d5b5539742d2554591751b4248b0204d20dcc9d upstream. Use the 'struct cred' saved at binder_open() to lookup the security ID via security_cred_getsecid(). This ensures that the security context that opened binder is the one used to generate the secctx. Cc: stable@vger.kernel.org # 5.4+ Fixes: ec74136ded79 ("binder: create node flag to request sender's security context") Signed-off-by: Todd Kjos <tkjos@google.com> Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com> Reported-by: kernel test robot <lkp@intel.com> Acked-by: Casey Schaufler <casey@schaufler-ca.com> Signed-off-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-11-12binder: use cred instead of task for selinux checksTodd Kjos1-6/+6
commit 52f88693378a58094c538662ba652aff0253c4fe upstream. Since binder was integrated with selinux, it has passed 'struct task_struct' associated with the binder_proc to represent the source and target of transactions. The conversion of task to SID was then done in the hook implementations. It turns out that there are race conditions which can result in an incorrect security context being used. Fix by using the 'struct cred' saved during binder_open and pass it to the selinux subsystem. Cc: stable@vger.kernel.org # 5.14 (need backport for earlier stables) Fixes: 79af73079d75 ("Add security hooks to binder and implement the hooks for SELinux.") Suggested-by: Jann Horn <jannh@google.com> Signed-off-by: Todd Kjos <tkjos@google.com> Acked-by: Casey Schaufler <casey@schaufler-ca.com> Signed-off-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-11-12binder: use euid from cred instead of using taskTodd Kjos2-1/+7
commit 29bc22ac5e5bc63275e850f0c8fc549e3d0e306b upstream. Save the 'struct cred' associated with a binder process at initial open to avoid potential race conditions when converting to an euid. Set a transaction's sender_euid from the 'struct cred' saved at binder_open() instead of looking up the euid from the binder proc's 'struct task'. This ensures the euid is associated with the security context that of the task that opened binder. Cc: stable@vger.kernel.org # 4.4+ Fixes: 457b9a6f09f0 ("Staging: android: add binder driver") Signed-off-by: Todd Kjos <tkjos@google.com> Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com> Suggested-by: Jann Horn <jannh@google.com> Acked-by: Casey Schaufler <casey@schaufler-ca.com> Signed-off-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-09-14binder: make sure fd closes completeTodd Kjos1-6/+17
During BC_FREE_BUFFER processing, the BINDER_TYPE_FDA object cleanup may close 1 or more fds. The close operations are completed using the task work mechanism -- which means the thread needs to return to userspace or the file object may never be dereferenced -- which can lead to hung processes. Force the binder thread back to userspace if an fd is closed during BC_FREE_BUFFER handling. Fixes: 80cd795630d6 ("binder: fix use-after-free due to ksys_close() during fdget()") Cc: stable <stable@vger.kernel.org> Reviewed-by: Martijn Coenen <maco@android.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20210830195146.587206-1-tkjos@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-09-14binder: fix freeze raceLi Li2-6/+31
Currently cgroup freezer is used to freeze the application threads, and BINDER_FREEZE is used to freeze the corresponding binder interface. There's already a mechanism in ioctl(BINDER_FREEZE) to wait for any existing transactions to drain out before actually freezing the binder interface. But freezing an app requires 2 steps, freezing the binder interface with ioctl(BINDER_FREEZE) and then freezing the application main threads with cgroupfs. This is not an atomic operation. The following race issue might happen. 1) Binder interface is frozen by ioctl(BINDER_FREEZE); 2) Main thread A initiates a new sync binder transaction to process B; 3) Main thread A is frozen by "echo 1 > cgroup.freeze"; 4) The response from process B reaches the frozen thread, which will unexpectedly fail. This patch provides a mechanism to check if there's any new pending transaction happening between ioctl(BINDER_FREEZE) and freezing the main thread. If there's any, the main thread freezing operation can be rolled back to finish the pending transaction. Furthermore, the response might reach the binder driver before the rollback actually happens. That will still cause failed transaction. As the other process doesn't wait for another response of the response, the response transaction failure can be fixed by treating the response transaction like an oneway/async one, allowing it to reach the frozen thread. And it will be consumed when the thread gets unfrozen later. NOTE: This patch reuses the existing definition of struct binder_frozen_status_info but expands the bit assignments of __u32 member sync_recv. To ensure backward compatibility, bit 0 of sync_recv still indicates there's an outstanding sync binder transaction. This patch adds new information to bit 1 of sync_recv, indicating the binder transaction happens exactly when there's a race. If an existing userspace app runs on a new kernel, a sync binder call will set bit 0 of sync_recv so ioctl(BINDER_GET_FROZEN_INFO) still return the expected value (true). The app just doesn't check bit 1 intentionally so it doesn't have the ability to tell if there's a race. This behavior is aligned with what happens on an old kernel which doesn't set bit 1 at all. A new userspace app can 1) check bit 0 to know if there's a sync binder transaction happened when being frozen - same as before; and 2) check bit 1 to know if that sync binder transaction happened exactly when there's a race - a new information for rollback decision. the same time, confirmed the pending transactions succeeded. Fixes: 432ff1e91694 ("binder: BINDER_FREEZE ioctl") Acked-by: Todd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Signed-off-by: Li Li <dualli@google.com> Test: stress test with apps being frozen and initiating binder calls at Link: https://lore.kernel.org/r/20210910164210.2282716-2-dualli@chromium.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-08-03binder: Add invalid handle info in user error logRamji Jiyani1-2/+2
In the case of a failed transaction, only the thread and process id are logged. Add the handle info for the reference to the target node in user error log to aid debugging. Acked-by: Todd Kjos <tkjos@google.com> Signed-off-by: Ramji Jiyani <ramjiyani@google.com> Link: https://lore.kernel.org/r/20210802220446.1938347-1-ramjiyani@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-07-21binderfs: add support for feature filesCarlos Llamas1-0/+39
Provide userspace with a mechanism to discover features supported by the binder driver to refrain from using any unsupported ones in the first place. Starting with "oneway_spam_detection" only new features are to be listed under binderfs and all previous ones are assumed to be supported. Assuming an instance of binderfs has been mounted at /dev/binderfs, binder feature files can be found under /dev/binderfs/features/. Usage example: $ mkdir /dev/binderfs $ mount -t binder binder /dev/binderfs $ cat /dev/binderfs/features/oneway_spam_detection 1 Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Carlos Llamas <cmllamas@google.com> Link: https://lore.kernel.org/r/20210715031805.1725878-1-cmllamas@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-05-13binder: Return EFAULT if we fail BINDER_ENABLE_ONEWAY_SPAM_DETECTIONLuca Stefani1-1/+1
All the other ioctl paths return EFAULT in case the copy_from_user/copy_to_user call fails, make oneway spam detection follow the same paradigm. Fixes: a7dc1e6f99df ("binder: tell userspace to dump current backtrace when detected oneway spamming") Acked-by: Todd Kjos <tkjos@google.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Luca Stefani <luca.stefani.ge1@gmail.com> Link: https://lore.kernel.org/r/20210506193726.45118-1-luca.stefani.ge1@gmail.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-04-27Merge tag 'selinux-pr-20210426' of ↵Linus Torvalds1-1/+10
git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux Pull selinux updates from Paul Moore: - Add support for measuring the SELinux state and policy capabilities using IMA. - A handful of SELinux/NFS patches to compare the SELinux state of one mount with a set of mount options. Olga goes into more detail in the patch descriptions, but this is important as it allows more flexibility when using NFS and SELinux context mounts. - Properly differentiate between the subjective and objective LSM credentials; including support for the SELinux and Smack. My clumsy attempt at a proper fix for AppArmor didn't quite pass muster so John is working on a proper AppArmor patch, in the meantime this set of patches shouldn't change the behavior of AppArmor in any way. This change explains the bulk of the diffstat beyond security/. - Fix a problem where we were not properly terminating the permission list for two SELinux object classes. * tag 'selinux-pr-20210426' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux: selinux: add proper NULL termination to the secclass_map permissions smack: differentiate between subjective and objective task credentials selinux: clarify task subjective and objective credentials lsm: separate security_task_getsecid() into subjective and objective variants nfs: account for selinux security context when deciding to share superblock nfs: remove unneeded null check in nfs_fill_super() lsm,selinux: add new hook to compare new mount to an existing mount selinux: fix misspellings using codespell tool selinux: fix misspellings using codespell tool selinux: measure state and policy capabilities selinux: Allow context mounts for unpriviliged overlayfs
2021-04-10binder: tell userspace to dump current backtrace when detected oneway spammingHang Lu4-8/+48
When async binder buffer got exhausted, some normal oneway transactions will also be discarded and may cause system or application failures. By that time, the binder debug information we dump may not be relevant to the root cause. And this issue is difficult to debug if without the backtrace of the thread sending spam. This change will send BR_ONEWAY_SPAM_SUSPECT to userspace when oneway spamming is detected, request to dump current backtrace. Oneway spamming will be reported only once when exceeding the threshold (target process dips below 80% of its oneway space, and current process is responsible for either more than 50 transactions, or more than 50% of the oneway space). And the detection will restart when the async buffer has returned to a healthy state. Acked-by: Todd Kjos <tkjos@google.com> Signed-off-by: Hang Lu <hangl@codeaurora.org> Link: https://lore.kernel.org/r/1617961246-4502-3-git-send-email-hangl@codeaurora.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-04-10binder: fix the missing BR_FROZEN_REPLY in binder_return_stringsHang Lu2-2/+3
Add BR_FROZEN_REPLY in binder_return_strings to support stat function. Fixes: ae28c1be1e54 ("binder: BINDER_GET_FROZEN_INFO ioctl") Acked-by: Todd Kjos <tkjos@google.com> Signed-off-by: Hang Lu <hangl@codeaurora.org> Link: https://lore.kernel.org/r/1617961246-4502-2-git-send-email-hangl@codeaurora.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-03-24binder: BINDER_GET_FROZEN_INFO ioctlMarco Ballesio2-0/+61
User space needs to know if binder transactions occurred to frozen processes. Introduce a new BINDER_GET_FROZEN ioctl and keep track of transactions occurring to frozen proceses. Signed-off-by: Marco Ballesio <balejs@google.com> Signed-off-by: Li Li <dualli@google.com> Acked-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20210316011630.1121213-4-dualli@chromium.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-03-24binder: use EINTR for interrupted wait for workMarco Ballesio1-2/+2
when interrupted by a signal, binder_wait_for_work currently returns -ERESTARTSYS. This error code isn't propagated to user space, but a way to handle interruption due to signals must be provided to code using this API. Replace this instance of -ERESTARTSYS with -EINTR, which is propagated to user space. binder_wait_for_work Signed-off-by: Marco Ballesio <balejs@google.com> Signed-off-by: Li Li <dualli@google.com> Test: built, booted, interrupted a worker thread within Acked-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20210316011630.1121213-3-dualli@chromium.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-03-24binder: BINDER_FREEZE ioctlMarco Ballesio2-10/+141
Frozen tasks can't process binder transactions, so a way is required to inform transmitting ends of communication failures due to the frozen state of their receiving counterparts. Additionally, races are possible between transitions to frozen state and binder transactions enqueued to a specific process. Implement BINDER_FREEZE ioctl for user space to inform the binder driver about the intention to freeze or unfreeze a process. When the ioctl is called, block the caller until any pending binder transactions toward the target process are flushed. Return an error to transactions to processes marked as frozen. Co-developed-by: Todd Kjos <tkjos@google.com> Acked-by: Todd Kjos <tkjos@google.com> Signed-off-by: Marco Ballesio <balejs@google.com> Signed-off-by: Todd Kjos <tkjos@google.com> Signed-off-by: Li Li <dualli@google.com> Link: https://lore.kernel.org/r/20210316011630.1121213-2-dualli@chromium.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-03-22lsm: separate security_task_getsecid() into subjective and objective variantsPaul Moore1-1/+10
Of the three LSMs that implement the security_task_getsecid() LSM hook, all three LSMs provide the task's objective security credentials. This turns out to be unfortunate as most of the hook's callers seem to expect the task's subjective credentials, although a small handful of callers do correctly expect the objective credentials. This patch is the first step towards fixing the problem: it splits the existing security_task_getsecid() hook into two variants, one for the subjective creds, one for the objective creds. void security_task_getsecid_subj(struct task_struct *p, u32 *secid); void security_task_getsecid_obj(struct task_struct *p, u32 *secid); While this patch does fix all of the callers to use the correct variant, in order to keep this patch focused on the callers and to ease review, the LSMs continue to use the same implementation for both hooks. The net effect is that this patch should not change the behavior of the kernel in any way, it will be up to the latter LSM specific patches in this series to change the hook implementations and return the correct credentials. Acked-by: Mimi Zohar <zohar@linux.ibm.com> (IMA) Acked-by: Casey Schaufler <casey@schaufler-ca.com> Reviewed-by: Richard Guy Briggs <rgb@redhat.com> Signed-off-by: Paul Moore <paul@paul-moore.com>
2021-01-24fs: make helpers idmap mount awareChristian Brauner1-2/+4
Extend some inode methods with an additional user namespace argument. A filesystem that is aware of idmapped mounts will receive the user namespace the mount has been marked with. This can be used for additional permission checking and also to enable filesystems to translate between uids and gids if they need to. We have implemented all relevant helpers in earlier patches. As requested we simply extend the exisiting inode method instead of introducing new ones. This is a little more code churn but it's mostly mechanical and doesnt't leave us with additional inode methods. Link: https://lore.kernel.org/r/20210121131959.646623-25-christian.brauner@ubuntu.com Cc: Christoph Hellwig <hch@lst.de> Cc: David Howells <dhowells@redhat.com> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: linux-fsdevel@vger.kernel.org Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
2020-12-16Merge branch 'exec-for-v5.11' of ↵Linus Torvalds1-1/+1
git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace Pull execve updates from Eric Biederman: "This set of changes ultimately fixes the interaction of posix file lock and exec. Fundamentally most of the change is just moving where unshare_files is called during exec, and tweaking the users of files_struct so that the count of files_struct is not unnecessarily played with. Along the way fcheck and related helpers were renamed to more accurately reflect what they do. There were also many other small changes that fell out, as this is the first time in a long time much of this code has been touched. Benchmarks haven't turned up any practical issues but Al Viro has observed a possibility for a lot of pounding on task_lock. So I have some changes in progress to convert put_files_struct to always rcu free files_struct. That wasn't ready for the merge window so that will have to wait until next time" * 'exec-for-v5.11' of git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace: (27 commits) exec: Move io_uring_task_cancel after the point of no return coredump: Document coredump code exclusively used by cell spufs file: Remove get_files_struct file: Rename __close_fd_get_file close_fd_get_file file: Replace ksys_close with close_fd file: Rename __close_fd to close_fd and remove the files parameter file: Merge __alloc_fd into alloc_fd file: In f_dupfd read RLIMIT_NOFILE once. file: Merge __fd_install into fd_install proc/fd: In fdinfo seq_show don't use get_files_struct bpf/task_iter: In task_file_seq_get_next use task_lookup_next_fd_rcu proc/fd: In proc_readfd_common use task_lookup_next_fd_rcu file: Implement task_lookup_next_fd_rcu kcmp: In get_file_raw_ptr use task_lookup_fd_rcu proc/fd: In tid_fd_mode use task_lookup_fd_rcu file: Implement task_lookup_fd_rcu file: Rename fcheck lookup_fd_rcu file: Replace fcheck_files with files_lookup_fd_rcu file: Factor files_lookup_fd_locked out of fcheck_files file: Rename __fcheck_files to files_lookup_fd_raw ...
2020-12-10file: Rename __close_fd_get_file close_fd_get_fileEric W. Biederman1-1/+1
The function close_fd_get_file is explicitly a variant of __close_fd[1]. Now that __close_fd has been renamed close_fd, rename close_fd_get_file to be consistent with close_fd. When __alloc_fd, __close_fd and __fd_install were introduced the double underscore indicated that the function took a struct files_struct parameter. The function __close_fd_get_file never has so the naming has always been inconsistent. This just cleans things up so there are not any lingering mentions or references __close_fd left in the code. [1] 80cd795630d6 ("binder: fix use-after-free due to ksys_close() during fdget()") Link: https://lkml.kernel.org/r/20201120231441.29911-23-ebiederm@xmission.com Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
2020-12-09binder: add flag to clear buffer on txn completeTodd Kjos3-1/+52
Add a per-transaction flag to indicate that the buffer must be cleared when the transaction is complete to prevent copies of sensitive data from being preserved in memory. Signed-off-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20201120233743.3617529-1-tkjos@google.com Cc: stable <stable@vger.kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-11-11binder: add trace at free transaction.Frankie.Chang2-0/+47
Since the original trace_binder_transaction_received cannot precisely present the real finished time of transaction, adding a trace_binder_txn_latency_free at the point of free transaction may be more close to it. Signed-off-by: Frankie.Chang <Frankie.Chang@mediatek.com> Acked-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/1605063764-12930-3-git-send-email-Frankie.Chang@mediatek.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-11-11binder: move structs from core file to header fileFrankie.Chang2-406/+406
Moving all structs to header file makes module more extendable, and makes all these structs to be defined in the same file. Signed-off-by: Frankie.Chang <Frankie.Chang@mediatek.com> Acked-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/1605063764-12930-2-git-send-email-Frankie.Chang@mediatek.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-11-09binder: change error code from postive to negative in binder_transactionZhang Qilong1-1/+1
Depending on the context, the error return value here (extra_buffers_size < added_size) should be negative. Acked-by: Martijn Coenen <maco@android.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com> Link: https://lore.kernel.org/r/20201026110314.135481-1-zhangqilong3@huawei.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-11-09Android: binder: added a missing blank line after declarationAndrew Bridges1-0/+1
Fixed a coding style issue. Signed-off-by: Andrew Bridges <andrew@slova.app> Link: https://lore.kernel.org/r/20201027225655.650922-1-andrew@slova.app Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-10-18task_work: cleanup notification modesJens Axboe1-1/+1
A previous commit changed the notification mode from true/false to an int, allowing notify-no, notify-yes, or signal-notify. This was backwards compatible in the sense that any existing true/false user would translate to either 0 (on notification sent) or 1, the latter which mapped to TWA_RESUME. TWA_SIGNAL was assigned a value of 2. Clean this up properly, and define a proper enum for the notification mode. Now we have: - TWA_NONE. This is 0, same as before the original change, meaning no notification requested. - TWA_RESUME. This is 1, same as before the original change, meaning that we use TIF_NOTIFY_RESUME. - TWA_SIGNAL. This uses TIF_SIGPENDING/JOBCTL_TASK_WORK for the notification. Clean up all the callers, switching their 0/1/false/true to using the appropriate TWA_* mode for notifications. Fixes: e91b48162332 ("task_work: teach task_work_add() to do signal_wake_up()") Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Jens Axboe <axboe@kernel.dk>
2020-10-10binder: fix UAF when releasing todo listTodd Kjos1-25/+10
When releasing a thread todo list when tearing down a binder_proc, the following race was possible which could result in a use-after-free: 1. Thread 1: enter binder_release_work from binder_thread_release 2. Thread 2: binder_update_ref_for_handle() -> binder_dec_node_ilocked() 3. Thread 2: dec nodeA --> 0 (will free node) 4. Thread 1: ACQ inner_proc_lock 5. Thread 2: block on inner_proc_lock 6. Thread 1: dequeue work (BINDER_WORK_NODE, part of nodeA) 7. Thread 1: REL inner_proc_lock 8. Thread 2: ACQ inner_proc_lock 9. Thread 2: todo list cleanup, but work was already dequeued 10. Thread 2: free node 11. Thread 2: REL inner_proc_lock 12. Thread 1: deref w->type (UAF) The problem was that for a BINDER_WORK_NODE, the binder_work element must not be accessed after releasing the inner_proc_lock while processing the todo list elements since another thread might be handling a deref on the node containing the binder_work element leading to the node being freed. Signed-off-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20201009232455.4054810-1-tkjos@google.com Cc: <stable@vger.kernel.org> # 4.14, 4.19, 5.4, 5.8 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-10-05binder: simplify the return expression of binder_mmapLiu Shixin1-14/+4
Simplify the return expression. Acked-by: Martijn Coenen <maco@android.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Liu Shixin <liushixin2@huawei.com> Link: https://lore.kernel.org/r/20200929015216.1829946-1-liushixin2@huawei.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-09-16binder: remove redundant assignment to pointer nColin Ian King1-1/+1
The pointer n 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. Acked-by: Todd Kjos <tkjos@google.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Colin Ian King <colin.king@canonical.com> Link: https://lore.kernel.org/r/20200910151221.751464-1-colin.king@canonical.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-09-03binder: print warnings when detecting oneway spamming.Martijn Coenen4-6/+58
The most common cause of the binder transaction buffer filling up is a client rapidly firing oneway transactions into a process, before it has a chance to handle them. Yet the root cause of this is often hard to debug, because either the system or the app will stop, and by that time binder debug information we dump in bugreports is no longer relevant. This change warns as soon as a process dips below 80% of its oneway space (less than 100kB available in the configuration), when any one process is responsible for either more than 50 transactions, or more than 50% of the oneway space. Signed-off-by: Martijn Coenen <maco@android.com> Acked-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20200821122544.1277051-1-maco@android.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-09-03binderfs: make symbol 'binderfs_fs_parameters' staticWei Yongjun1-1/+1
The sparse tool complains as follows: drivers/android/binderfs.c:66:32: warning: symbol 'binderfs_fs_parameters' was not declared. Should it be static? This variable is not used outside of binderfs.c, so this commit marks it static. Fixes: 095cf502b31e ("binderfs: port to new mount api") Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20200818112245.43891-1-weiyongjun1@huawei.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-09-03binder: Modify commentsYangHui1-1/+1
The function name should is binder_alloc_new_buf() Signed-off-by: YangHui <yanghui.def@gmail.com> Reviewed-by: Martijn Coenen <maco@android.com> Link: https://lore.kernel.org/r/1597714444-3614-1-git-send-email-yanghui.def@gmail.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-09-03binder: Remove bogus warning on failed same-process transactionJann Horn1-2/+0
While binder transactions with the same binder_proc as sender and recipient are forbidden, transactions with the same task_struct as sender and recipient are possible (even though currently there is a weird check in binder_transaction() that rejects them in the target==0 case). Therefore, task_struct identities can't be used to distinguish whether the caller is running in the context of the sender or the recipient. Since I see no easy way to make this WARN_ON() useful and correct, let's just remove it. Fixes: 44d8047f1d87 ("binder: use standard functions to allocate fds") Reported-by: syzbot+e113a0b970b7b3f394ba@syzkaller.appspotmail.com Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Acked-by: Todd Kjos <tkjos@google.com> Signed-off-by: Jann Horn <jannh@google.com> Link: https://lore.kernel.org/r/20200806165359.2381483-1-jannh@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-07-29drivers: android: Fix the SPDX comment styleMrinal Pandey1-1/+1
C source files should have `//` as SPDX comment and not `/**/`. Fix this by running checkpatch on the file. Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> Link: https://lore.kernel.org/r/20200724131449.zvjutbemg3vqhrzh@mrinalpandey Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-07-29drivers: android: Fix a variable declaration coding style issueMrinal Pandey1-0/+1
Add a blank line after variable declarations as suggested by checkpatch. Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> Link: https://lore.kernel.org/r/20200724131433.stf3ycooogawyzb3@mrinalpandey Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-07-29drivers: android: Remove braces for a single statement if-else blockMrinal Pandey1-3/+2
Remove braces for both if and else block as suggested by checkpatch. Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> Link: https://lore.kernel.org/r/20200724131403.dahfhdwa3wirzkxj@mrinalpandey Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-07-29drivers: android: Remove the use of else after returnMrinal Pandey1-2/+1
Remove the unnecessary else branch after return statement as suggested by checkpatch. Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> Link: https://lore.kernel.org/r/20200724131348.haz4ocxcferdcsgn@mrinalpandey Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-07-29drivers: android: Fix a variable declaration coding style issueMrinal Pandey1-0/+1
Add a blank line after variable declarations as suggested by checkpatch. Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> Link: https://lore.kernel.org/r/20200724131254.qxbvderrws36dzzq@mrinalpandey Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-07-29binder: Prevent context manager from incrementing ref 0Jann Horn1-1/+14
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 Handa1-1/+1
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 Kjos1-7/+7
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-13treewide: replace '---help---' in Kconfig files with 'help'Masahiro Yamada1-5/+5
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 Lespinasse1-2/+2
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 Lespinasse1-5/+5
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 King1-1/+1
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 Bin1-1/+1
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>
generated by cgit v1.2.3 (git 2.39.0) at 2024-11-25 12:29:20 +0300