- 28 2月, 2023 1 次提交
-
-
由 Eric W. Biederman 提交于
stable inclusion from stable-v5.10.162 commit 57b20530363d127ab6a82e336275769258eb5f37 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I6BTWC CVE: NA Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v5.10.168&id=57b20530363d127ab6a82e336275769258eb5f37 -------------------------------- [ Upstream commit 9fe83c43 ] 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] 80cd7956 ("binder: fix use-after-free due to ksys_close() during fdget()") Link: https://lkml.kernel.org/r/20201120231441.29911-23-ebiederm@xmission.comSigned-off-by: NEric W. Biederman <ebiederm@xmission.com> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: NLi Lingfeng <lilingfeng3@huawei.com> Reviewed-by: NZhang Yi <yi.zhang@huawei.com> Reviewed-by: NWang Weiyang <wangweiyang2@huawei.com> Signed-off-by: NJialin Zhang <zhangjialin11@huawei.com>
-
- 18 1月, 2023 1 次提交
-
-
由 Pavel Begunkov 提交于
stable inclusion from stable-v5.10.141 commit 28d8d2737e82fc29ff9e788597661abecc7f7994 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I685FC CEV: NA Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v5.10.162&id=28d8d2737e82fc29ff9e788597661abecc7f7994 -------------------------------- Older kernels lack io_uring POLLFREE handling. As only affected files are signalfd and android binder the safest option would be to disable polling those files via io_uring and hope there are no users. Fixes: 221c5eb2 ("io_uring: add support for IORING_OP_POLL") Signed-off-by: NPavel Begunkov <asml.silence@gmail.com> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> conflicts: include/linux/fs.h Signed-off-by: NLi Lingfeng <lilingfeng3@huawei.com> Reviewed-by: NZhang Yi <yi.zhang@huawei.com> Signed-off-by: NJialin Zhang <zhangjialin11@huawei.com>
-
- 18 10月, 2022 1 次提交
-
-
由 Carlos Llamas 提交于
mainline inclusion from mainline-v6.0-rc4 commit a0e44c64 category: bugfix bugzilla: 187805, https://gitee.com/src-openeuler/kernel/issues/I5U713 CVE: CVE-2022-20421 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v6.0&id=a0e44c64b6061dda7e00b7c458e4523e2331b739 -------------------------------- A transaction of type BINDER_TYPE_WEAK_HANDLE can fail to increment the reference for a node. In this case, the target proc normally releases the failed reference upon close as expected. However, if the target is dying in parallel the call will race with binder_deferred_release(), so the target could have released all of its references by now leaving the cleanup of the new failed reference unhandled. The transaction then ends and the target proc gets released making the ref->proc now a dangling pointer. Later on, ref->node is closed and we attempt to take spin_lock(&ref->proc->inner_lock), which leads to the use-after-free bug reported below. Let's fix this by cleaning up the failed reference on the spot instead of relying on the target to do so. ================================================================== BUG: KASAN: use-after-free in _raw_spin_lock+0xa8/0x150 Write of size 4 at addr ffff5ca207094238 by task kworker/1:0/590 CPU: 1 PID: 590 Comm: kworker/1:0 Not tainted 5.19.0-rc8 #10 Hardware name: linux,dummy-virt (DT) Workqueue: events binder_deferred_func Call trace: dump_backtrace.part.0+0x1d0/0x1e0 show_stack+0x18/0x70 dump_stack_lvl+0x68/0x84 print_report+0x2e4/0x61c kasan_report+0xa4/0x110 kasan_check_range+0xfc/0x1a4 __kasan_check_write+0x3c/0x50 _raw_spin_lock+0xa8/0x150 binder_deferred_func+0x5e0/0x9b0 process_one_work+0x38c/0x5f0 worker_thread+0x9c/0x694 kthread+0x188/0x190 ret_from_fork+0x10/0x20 Acked-by: NChristian Brauner (Microsoft) <brauner@kernel.org> Signed-off-by: NCarlos Llamas <cmllamas@google.com> Cc: stable <stable@kernel.org> # 4.14+ Link: https://lore.kernel.org/r/20220801182511.3371447-1-cmllamas@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: NRen Zhijie <renzhijie2@huawei.com> Reviewed-by: NZhang Qiao <zhangqiao22@huawei.com> Reviewed-by: NChen Hui <judy.chenhui@huawei.com> Signed-off-by: NZheng Zengkai <zhengzengkai@huawei.com>
-
- 27 4月, 2022 1 次提交
-
-
由 Todd Kjos 提交于
stable inclusion from stable-v5.10.94 commit 551a785c26f6ff41cccd527e7bd9f032f91332c2 bugzilla: https://gitee.com/openeuler/kernel/issues/I531X9 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=551a785c26f6ff41cccd527e7bd9f032f91332c2 -------------------------------- [ Upstream commit fe6b1869 ] 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: bb4a2e48 ("binder: return errors from buffer copy functions") Suggested-by: NDan Carpenter <dan.carpenter@oracle.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NTodd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20211130185152.437403-2-tkjos@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: NSasha Levin <sashal@kernel.org> Signed-off-by: NZheng Zengkai <zhengzengkai@huawei.com> Acked-by: NXie XiuQi <xiexiuqi@huawei.com>
-
- 14 1月, 2022 2 次提交
-
-
由 Eric Biggers 提交于
stable inclusion from stable-v5.10.85 commit 9f3acee7eac8d8690134b09ba55e2c12164d24ae bugzilla: 186032 https://gitee.com/openeuler/kernel/issues/I4QVI4 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=9f3acee7eac8d8690134b09ba55e2c12164d24ae -------------------------------- commit a880b28a 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: NLinus Torvalds <torvalds@linux-foundation.org> Fixes: f5cb779b ("ANDROID: binder: remove waitqueue when thread exits.") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20211209010455.42744-3-ebiggers@kernel.orgSigned-off-by: NEric Biggers <ebiggers@google.com> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: NChen Jun <chenjun102@huawei.com> Signed-off-by: NZheng Zengkai <zhengzengkai@huawei.com>
-
由 Todd Kjos 提交于
stable inclusion from stable-v5.10.83 commit 4402cf0402526f7c5befa97481be13b131797838 bugzilla: 185879 https://gitee.com/openeuler/kernel/issues/I4QUVG Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=4402cf0402526f7c5befa97481be13b131797838 -------------------------------- commit c21a80ca upstream. This is a partial revert of commit 29bc22ac ("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: 29bc22ac ("binder: use euid from cred instead of using task") Cc: stable@vger.kernel.org # 4.4+ Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NTodd Kjos <tkjos@google.com> Change-Id: I9b1769a3510fed250bb21859ef8beebabe034c66 Link: https://lore.kernel.org/r/20211112180720.2858135-1-tkjos@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: NChen Jun <chenjun102@huawei.com> Signed-off-by: NZheng Zengkai <zhengzengkai@huawei.com>
-
- 06 12月, 2021 3 次提交
-
-
由 Todd Kjos 提交于
stable inclusion from stable-5.10.80 commit afbec52fbce006a775edb21f87ccae713bc0e7d6 bugzilla: 185821 https://gitee.com/openeuler/kernel/issues/I4L7CG Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=afbec52fbce006a775edb21f87ccae713bc0e7d6 -------------------------------- commit 4d5b5539 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: ec74136d ("binder: create node flag to request sender's security context") Signed-off-by: NTodd Kjos <tkjos@google.com> Suggested-by: NStephen Smalley <stephen.smalley.work@gmail.com> Reported-by: Nkernel test robot <lkp@intel.com> Acked-by: NCasey Schaufler <casey@schaufler-ca.com> Signed-off-by: NPaul Moore <paul@paul-moore.com> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: NChen Jun <chenjun102@huawei.com> Reviewed-by: NWeilong Chen <chenweilong@huawei.com> Acked-by: NWeilong Chen <chenweilong@huawei.com> Signed-off-by: NChen Jun <chenjun102@huawei.com> Signed-off-by: NZheng Zengkai <zhengzengkai@huawei.com>
-
由 Todd Kjos 提交于
stable inclusion from stable-5.10.80 commit 0d9f4ae7cd6f5283dd0e343265268c695ef592b0 bugzilla: 185821 https://gitee.com/openeuler/kernel/issues/I4L7CG Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=0d9f4ae7cd6f5283dd0e343265268c695ef592b0 -------------------------------- commit 52f88693 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: 79af7307 ("Add security hooks to binder and implement the hooks for SELinux.") Suggested-by: NJann Horn <jannh@google.com> Signed-off-by: NTodd Kjos <tkjos@google.com> Acked-by: NCasey Schaufler <casey@schaufler-ca.com> Signed-off-by: NPaul Moore <paul@paul-moore.com> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: NChen Jun <chenjun102@huawei.com> Reviewed-by: NWeilong Chen <chenweilong@huawei.com> Acked-by: NWeilong Chen <chenweilong@huawei.com> Signed-off-by: NChen Jun <chenjun102@huawei.com> Signed-off-by: NZheng Zengkai <zhengzengkai@huawei.com>
-
由 Todd Kjos 提交于
stable inclusion from stable-5.10.80 commit bd9cea41ac6e08f615030dea28b23e12b7a2674f bugzilla: 185821 https://gitee.com/openeuler/kernel/issues/I4L7CG Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=bd9cea41ac6e08f615030dea28b23e12b7a2674f -------------------------------- commit 29bc22ac 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: 457b9a6f ("Staging: android: add binder driver") Signed-off-by: NTodd Kjos <tkjos@google.com> Suggested-by: NStephen Smalley <stephen.smalley.work@gmail.com> Suggested-by: NJann Horn <jannh@google.com> Acked-by: NCasey Schaufler <casey@schaufler-ca.com> Signed-off-by: NPaul Moore <paul@paul-moore.com> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: NChen Jun <chenjun102@huawei.com> Reviewed-by: NWeilong Chen <chenweilong@huawei.com> Acked-by: NWeilong Chen <chenweilong@huawei.com> Signed-off-by: NChen Jun <chenjun102@huawei.com> Signed-off-by: NZheng Zengkai <zhengzengkai@huawei.com>
-
- 30 11月, 2021 1 次提交
-
-
由 Todd Kjos 提交于
stable inclusion from stable-5.10.79 commit 07d1db141e478917600fa32be11e5b828ff9ed13 bugzilla: 185793 https://gitee.com/openeuler/kernel/issues/I4K65C Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=07d1db141e478917600fa32be11e5b828ff9ed13 -------------------------------- commit 32e9f56a 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: 44d8047f ("binder: use standard functions to allocate fds") Cc: stable <stable@vger.kernel.org> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NTodd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20211015233811.3532235-1-tkjos@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: NChen Jun <chenjun102@huawei.com> Acked-by: NWeilong Chen <chenweilong@huawei.com> Signed-off-by: NBin Li <huawei.libin@huawei.com> Signed-off-by: NZheng Zengkai <zhengzengkai@huawei.com>
-
- 15 11月, 2021 1 次提交
-
-
由 Todd Kjos 提交于
stable inclusion from stable-5.10.70 commit d5b0473707fa53b03a5db0256ce62b2874bddbc7 bugzilla: 182949 https://gitee.com/openeuler/kernel/issues/I4I3GQ Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=d5b0473707fa53b03a5db0256ce62b2874bddbc7 -------------------------------- commit 5fdb55c1 upstream. 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: 80cd7956 ("binder: fix use-after-free due to ksys_close() during fdget()") Cc: stable <stable@vger.kernel.org> Reviewed-by: NMartijn Coenen <maco@android.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NTodd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20210830195146.587206-1-tkjos@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: NChen Jun <chenjun102@huawei.com> Acked-by: NWeilong Chen <chenweilong@huawei.com> Signed-off-by: NChen Jun <chenjun102@huawei.com> Signed-off-by: NZheng Zengkai <zhengzengkai@huawei.com>
-
- 12 1月, 2021 1 次提交
-
-
由 Todd Kjos 提交于
stable inclusion from stable-5.10.4 commit 06da7fff770170eb140a47fbf11b87189bf360d2 bugzilla: 46903 -------------------------------- commit 0f966cba upstream. 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: NTodd 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: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: NChen Jun <chenjun102@huawei.com> Acked-by: NXie XiuQi <xiexiuqi@huawei.com>
-
- 18 10月, 2020 1 次提交
-
-
由 Jens Axboe 提交于
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: e91b4816 ("task_work: teach task_work_add() to do signal_wake_up()") Reviewed-by: NThomas Gleixner <tglx@linutronix.de> Signed-off-by: NJens Axboe <axboe@kernel.dk>
-
- 10 10月, 2020 1 次提交
-
-
由 Todd Kjos 提交于
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: NTodd 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: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 05 10月, 2020 1 次提交
-
-
由 Liu Shixin 提交于
Simplify the return expression. Acked-by: NMartijn Coenen <maco@android.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NLiu Shixin <liushixin2@huawei.com> Link: https://lore.kernel.org/r/20200929015216.1829946-1-liushixin2@huawei.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 04 9月, 2020 2 次提交
-
-
由 Martijn Coenen 提交于
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: NMartijn Coenen <maco@android.com> Acked-by: NTodd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20200821122544.1277051-1-maco@android.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Jann Horn 提交于
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: 44d8047f ("binder: use standard functions to allocate fds") Reported-by: syzbot+e113a0b970b7b3f394ba@syzkaller.appspotmail.com Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Acked-by: NTodd Kjos <tkjos@google.com> Signed-off-by: NJann Horn <jannh@google.com> Link: https://lore.kernel.org/r/20200806165359.2381483-1-jannh@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 29 7月, 2020 3 次提交
-
-
由 Mrinal Pandey 提交于
Remove braces for both if and else block as suggested by checkpatch. Signed-off-by: NMrinal Pandey <mrinalmni@gmail.com> Link: https://lore.kernel.org/r/20200724131403.dahfhdwa3wirzkxj@mrinalpandeySigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Mrinal Pandey 提交于
Remove the unnecessary else branch after return statement as suggested by checkpatch. Signed-off-by: NMrinal Pandey <mrinalmni@gmail.com> Link: https://lore.kernel.org/r/20200724131348.haz4ocxcferdcsgn@mrinalpandeySigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Jann Horn 提交于
Binder is designed such that a binder_proc never has references to itself. If this rule is violated, memory corruption can occur when a process sends a transaction to itself; see e.g. <https://syzkaller.appspot.com/bug?extid=09e05aba06723a94d43d>. There is a remaining edgecase through which such a transaction-to-self can still occur from the context of a task with BINDER_SET_CONTEXT_MGR access: - task A opens /dev/binder twice, creating binder_proc instances P1 and P2 - P1 becomes context manager - P2 calls ACQUIRE on the magic handle 0, allocating index 0 in its handle table - P1 dies (by closing the /dev/binder fd and waiting a bit) - P2 becomes context manager - P2 calls ACQUIRE on the magic handle 0, allocating index 1 in its handle table [this triggers a warning: "binder: 1974:1974 tried to acquire reference to desc 0, got 1 instead"] - task B opens /dev/binder once, creating binder_proc instance P3 - P3 calls P2 (via magic handle 0) with (void*)1 as argument (two-way transaction) - P2 receives the handle and uses it to call P3 (two-way transaction) - P3 calls P2 (via magic handle 0) (two-way transaction) - P2 calls P2 (via handle 1) (two-way transaction) And then, if P2 does *NOT* accept the incoming transaction work, but instead closes the binder fd, we get a crash. Solve it by preventing the context manager from using ACQUIRE on ref 0. There shouldn't be any legitimate reason for the context manager to do that. Additionally, print a warning if someone manages to find another way to trigger a transaction-to-self bug in the future. Cc: stable@vger.kernel.org Fixes: 457b9a6f ("Staging: android: add binder driver") Acked-by: NTodd Kjos <tkjos@google.com> Signed-off-by: NJann Horn <jannh@google.com> Reviewed-by: NMartijn Coenen <maco@android.com> Link: https://lore.kernel.org/r/20200727120424.1627555-1-jannh@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 23 6月, 2020 1 次提交
-
-
由 Todd Kjos 提交于
The binder driver makes the assumption proc->context pointer is invariant after initialization (as documented in the kerneldoc header for struct proc). However, in commit f0fe2c0f ("binder: prevent UAF for binderfs devices II") proc->context is set to NULL during binder_deferred_release(). Another proc was in the middle of setting up a transaction to the dying process and crashed on a NULL pointer deref on "context" which is a local set to &proc->context: new_ref->data.desc = (node == context->binder_context_mgr_node) ? 0 : 1; Here's the stack: [ 5237.855435] Call trace: [ 5237.855441] binder_get_ref_for_node_olocked+0x100/0x2ec [ 5237.855446] binder_inc_ref_for_node+0x140/0x280 [ 5237.855451] binder_translate_binder+0x1d0/0x388 [ 5237.855456] binder_transaction+0x2228/0x3730 [ 5237.855461] binder_thread_write+0x640/0x25bc [ 5237.855466] binder_ioctl_write_read+0xb0/0x464 [ 5237.855471] binder_ioctl+0x30c/0x96c [ 5237.855477] do_vfs_ioctl+0x3e0/0x700 [ 5237.855482] __arm64_sys_ioctl+0x78/0xa4 [ 5237.855488] el0_svc_common+0xb4/0x194 [ 5237.855493] el0_svc_handler+0x74/0x98 [ 5237.855497] el0_svc+0x8/0xc The fix is to move the kfree of the binder_device to binder_free_proc() so the binder_device is freed when we know there are no references remaining on the binder_proc. Fixes: f0fe2c0f ("binder: prevent UAF for binderfs devices II") Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NTodd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Link: https://lore.kernel.org/r/20200622200715.114382-1-tkjos@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 04 3月, 2020 1 次提交
-
-
由 Christian Brauner 提交于
This is a necessary follow up to the first fix I proposed and we merged in 2669b8b0 ("binder: prevent UAF for binderfs devices"). I have been overly optimistic that the simple fix I proposed would work. But alas, ihold() + iput() won't work since the inodes won't survive the destruction of the superblock. So all we get with my prior fix is a different race with a tinier race-window but it doesn't solve the issue. Fwiw, the problem lies with generic_shutdown_super(). It even has this cozy Al-style comment: if (!list_empty(&sb->s_inodes)) { printk("VFS: Busy inodes after unmount of %s. " "Self-destruct in 5 seconds. Have a nice day...\n", sb->s_id); } On binder_release(), binder_defer_work(proc, BINDER_DEFERRED_RELEASE) is called which punts the actual cleanup operation to a workqueue. At some point, binder_deferred_func() will be called which will end up calling binder_deferred_release() which will retrieve and cleanup the binder_context attach to this struct binder_proc. If we trace back where this binder_context is attached to binder_proc we see that it is set in binder_open() and is taken from the struct binder_device it is associated with. This obviously assumes that the struct binder_device that context is attached to is _never_ freed. While that might be true for devtmpfs binder devices it is most certainly wrong for binderfs binder devices. So, assume binder_open() is called on a binderfs binder devices. We now stash away the struct binder_context associated with that struct binder_devices: proc->context = &binder_dev->context; /* binderfs stashes devices in i_private */ if (is_binderfs_device(nodp)) { binder_dev = nodp->i_private; info = nodp->i_sb->s_fs_info; binder_binderfs_dir_entry_proc = info->proc_log_dir; } else { . . . proc->context = &binder_dev->context; Now let's assume that the binderfs instance for that binder devices is shutdown via umount() and/or the mount namespace associated with it goes away. As long as there is still an fd open for that binderfs binder device things are fine. But let's assume we now close the last fd for that binderfs binder device. Now binder_release() is called and punts to the workqueue. Assume that the workqueue has quite a bit of stuff to do and doesn't get to cleaning up the struct binder_proc and the associated struct binder_context with it for that binderfs binder device right away. In the meantime, the VFS is killing the super block and is ultimately calling sb->evict_inode() which means it will call binderfs_evict_inode() which does: static void binderfs_evict_inode(struct inode *inode) { struct binder_device *device = inode->i_private; struct binderfs_info *info = BINDERFS_I(inode); clear_inode(inode); if (!S_ISCHR(inode->i_mode) || !device) return; mutex_lock(&binderfs_minors_mutex); --info->device_count; ida_free(&binderfs_minors, device->miscdev.minor); mutex_unlock(&binderfs_minors_mutex); kfree(device->context.name); kfree(device); } thereby freeing the struct binder_device including struct binder_context. Now the workqueue finally has time to get around to cleaning up struct binder_proc and is now trying to access the associate struct binder_context. Since it's already freed it will OOPs. Fix this by introducing a refounct on binder devices. This is an alternative fix to 51d8a7ec ("binder: prevent UAF read in print_binder_transaction_log_entry()"). Fixes: 3ad20fe3 ("binder: implement binderfs") Fixes: 2669b8b0 ("binder: prevent UAF for binderfs devices") Fixes: 03e2e07e ("binder: Make transaction_log available in binderfs") Related : 51d8a7ec ("binder: prevent UAF read in print_binder_transaction_log_entry()") Cc: stable@vger.kernel.org Signed-off-by: NChristian Brauner <christian.brauner@ubuntu.com> Acked-by: NTodd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20200303164340.670054-1-christian.brauner@ubuntu.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 03 3月, 2020 1 次提交
-
-
由 Christian Brauner 提交于
On binder_release(), binder_defer_work(proc, BINDER_DEFERRED_RELEASE) is called which punts the actual cleanup operation to a workqueue. At some point, binder_deferred_func() will be called which will end up calling binder_deferred_release() which will retrieve and cleanup the binder_context attach to this struct binder_proc. If we trace back where this binder_context is attached to binder_proc we see that it is set in binder_open() and is taken from the struct binder_device it is associated with. This obviously assumes that the struct binder_device that context is attached to is _never_ freed. While that might be true for devtmpfs binder devices it is most certainly wrong for binderfs binder devices. So, assume binder_open() is called on a binderfs binder devices. We now stash away the struct binder_context associated with that struct binder_devices: proc->context = &binder_dev->context; /* binderfs stashes devices in i_private */ if (is_binderfs_device(nodp)) { binder_dev = nodp->i_private; info = nodp->i_sb->s_fs_info; binder_binderfs_dir_entry_proc = info->proc_log_dir; } else { . . . proc->context = &binder_dev->context; Now let's assume that the binderfs instance for that binder devices is shutdown via umount() and/or the mount namespace associated with it goes away. As long as there is still an fd open for that binderfs binder device things are fine. But let's assume we now close the last fd for that binderfs binder device. Now binder_release() is called and punts to the workqueue. Assume that the workqueue has quite a bit of stuff to do and doesn't get to cleaning up the struct binder_proc and the associated struct binder_context with it for that binderfs binder device right away. In the meantime, the VFS is killing the super block and is ultimately calling sb->evict_inode() which means it will call binderfs_evict_inode() which does: static void binderfs_evict_inode(struct inode *inode) { struct binder_device *device = inode->i_private; struct binderfs_info *info = BINDERFS_I(inode); clear_inode(inode); if (!S_ISCHR(inode->i_mode) || !device) return; mutex_lock(&binderfs_minors_mutex); --info->device_count; ida_free(&binderfs_minors, device->miscdev.minor); mutex_unlock(&binderfs_minors_mutex); kfree(device->context.name); kfree(device); } thereby freeing the struct binder_device including struct binder_context. Now the workqueue finally has time to get around to cleaning up struct binder_proc and is now trying to access the associate struct binder_context. Since it's already freed it will OOPs. Fix this by holding an additional reference to the inode that is only released once the workqueue is done cleaning up struct binder_proc. This is an easy alternative to introducing separate refcounting on struct binder_device which we can always do later if it becomes necessary. This is an alternative fix to 51d8a7ec ("binder: prevent UAF read in print_binder_transaction_log_entry()"). Fixes: 3ad20fe3 ("binder: implement binderfs") Fixes: 03e2e07e ("binder: Make transaction_log available in binderfs") Related : 51d8a7ec ("binder: prevent UAF read in print_binder_transaction_log_entry()") Cc: stable@vger.kernel.org Signed-off-by: NChristian Brauner <christian.brauner@ubuntu.com> Acked-by: NTodd Kjos <tkjos@google.com> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 22 1月, 2020 1 次提交
-
-
由 Martin Fuzzey 提交于
Since commit 43e23b6c ("debugfs: log errors when something goes wrong") debugfs logs attempts to create existing files. However binder attempts to create multiple debugfs files with the same name when a single PID has multiple contexts, this leads to log spamming during an Android boot (17 such messages during boot on my system). Fix this by checking if we already know the PID and only create the debugfs entry for the first context per PID. Do the same thing for binderfs for symmetry. Signed-off-by: NMartin Fuzzey <martin.fuzzey@flowbird.group> Acked-by: NTodd Kjos <tkjos@google.com> Fixes: 43e23b6c ("debugfs: log errors when something goes wrong") Cc: stable <stable@vger.kernel.org> Link: https://lore.kernel.org/r/1578671054-5982-1-git-send-email-martin.fuzzey@flowbird.groupSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 21 1月, 2020 1 次提交
-
-
由 Jens Axboe 提交于
Just one caller of this, and just use filp_close() there manually. This is important to allow async close/removal of the fd. Signed-off-by: NJens Axboe <axboe@kernel.dk>
-
- 14 12月, 2019 1 次提交
-
-
由 Todd Kjos 提交于
For BINDER_TYPE_PTR and BINDER_TYPE_FDA transactions, the num_valid local was calculated incorrectly causing the range check in binder_validate_ptr() to miss out-of-bounds offsets. Fixes: bde4a19f ("binder: use userspace pointer as base of buffer space") Signed-off-by: NTodd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Link: https://lore.kernel.org/r/20191213202531.55010-1-tkjos@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 23 10月, 2019 1 次提交
-
-
由 Arnd Bergmann 提交于
The .ioctl and .compat_ioctl file operations have the same prototype so they can both point to the same function, which works great almost all the time when all the commands are compatible. One exception is the s390 architecture, where a compat pointer is only 31 bit wide, and converting it into a 64-bit pointer requires calling compat_ptr(). Most drivers here will never run in s390, but since we now have a generic helper for it, it's easy enough to use it consistently. I double-checked all these drivers to ensure that all ioctl arguments are used as pointers or are ignored, but are not interpreted as integer values. Acked-by: NJason Gunthorpe <jgg@mellanox.com> Acked-by: NDaniel Vetter <daniel.vetter@ffwll.ch> Acked-by: NMauro Carvalho Chehab <mchehab+samsung@kernel.org> Acked-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> Acked-by: NDavid Sterba <dsterba@suse.com> Acked-by: NDarren Hart (VMware) <dvhart@infradead.org> Acked-by: NJonathan Cameron <Jonathan.Cameron@huawei.com> Acked-by: NBjorn Andersson <bjorn.andersson@linaro.org> Acked-by: NDan Williams <dan.j.williams@intel.com> Signed-off-by: NArnd Bergmann <arnd@arndb.de>
-
- 17 10月, 2019 2 次提交
-
-
由 Jann Horn 提交于
SZ_1K has been defined in include/linux/sizes.h since v3.6. Get rid of the duplicate definition. Signed-off-by: NJann Horn <jannh@google.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20191016150119.154756-2-jannh@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Jann Horn 提交于
binder_mmap() tries to prevent the creation of overly big binder mappings by silently truncating the size of the VMA to 4MiB. However, this violates the API contract of mmap(). If userspace attempts to create a large binder VMA, and later attempts to unmap that VMA, it will call munmap() on a range beyond the end of the VMA, which may have been allocated to another VMA in the meantime. This can lead to userspace memory corruption. The following sequence of calls leads to a segfault without this commit: int main(void) { int binder_fd = open("/dev/binder", O_RDWR); if (binder_fd == -1) err(1, "open binder"); void *binder_mapping = mmap(NULL, 0x800000UL, PROT_READ, MAP_SHARED, binder_fd, 0); if (binder_mapping == MAP_FAILED) err(1, "mmap binder"); void *data_mapping = mmap(NULL, 0x400000UL, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0); if (data_mapping == MAP_FAILED) err(1, "mmap data"); munmap(binder_mapping, 0x800000UL); *(char*)data_mapping = 1; return 0; } Cc: stable@vger.kernel.org Signed-off-by: NJann Horn <jannh@google.com> Acked-by: NTodd Kjos <tkjos@google.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20191016150119.154756-1-jannh@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 10 10月, 2019 1 次提交
-
-
由 Christian Brauner 提交于
When a binder transaction is initiated on a binder device coming from a binderfs instance, a pointer to the name of the binder device is stashed in the binder_transaction_log_entry's context_name member. Later on it is used to print the name in print_binder_transaction_log_entry(). By the time print_binder_transaction_log_entry() accesses context_name binderfs_evict_inode() might have already freed the associated memory thereby causing a UAF. Do the simple thing and prevent this by copying the name of the binder device instead of stashing a pointer to it. Reported-by: NJann Horn <jannh@google.com> Fixes: 03e2e07e ("binder: Make transaction_log available in binderfs") Link: https://lore.kernel.org/r/CAG48ez14Q0-F8LqsvcNbyR2o6gPW8SHXsm4u5jmD9MpsteM2Tw@mail.gmail.comSigned-off-by: NChristian Brauner <christian.brauner@ubuntu.com> Reviewed-by: NJoel Fernandes (Google) <joel@joelfernandes.org> Acked-by: NTodd Kjos <tkjos@google.com> Reviewed-by: NHridya Valsaraju <hridya@google.com> Link: https://lore.kernel.org/r/20191008130159.10161-1-christian.brauner@ubuntu.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 04 9月, 2019 4 次提交
-
-
由 Hridya Valsaraju 提交于
Currently /sys/kernel/debug/binder/proc contains the debug data for every binder_proc instance. This patch makes this information also available in a binderfs instance mounted with a mount option "stats=global" in addition to debugfs. The patch does not affect the presence of the file in debugfs. If a binderfs instance is mounted at path /dev/binderfs, this file would be present at /dev/binderfs/binder_logs/proc. This change provides an alternate way to access this file when debugfs is not mounted. Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NHridya Valsaraju <hridya@google.com> Link: https://lore.kernel.org/r/20190903161655.107408-5-hridya@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Hridya Valsaraju 提交于
Currently, the binder transaction log files 'transaction_log' and 'failed_transaction_log' live in debugfs at the following locations: /sys/kernel/debug/binder/failed_transaction_log /sys/kernel/debug/binder/transaction_log This patch makes these files also available in a binderfs instance mounted with the mount option "stats=global". It does not affect the presence of these files in debugfs. If a binderfs instance is mounted at path /dev/binderfs, the location of these files will be as follows: /dev/binderfs/binder_logs/failed_transaction_log /dev/binderfs/binder_logs/transaction_log This change provides an alternate option to access these files when debugfs is not mounted. Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NHridya Valsaraju <hridya@google.com> Link: https://lore.kernel.org/r/20190903161655.107408-4-hridya@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Hridya Valsaraju 提交于
The following binder stat files currently live in debugfs. /sys/kernel/debug/binder/state /sys/kernel/debug/binder/stats /sys/kernel/debug/binder/transactions This patch makes these files available in a binderfs instance mounted with the mount option 'stats=global'. For example, if a binderfs instance is mounted at path /dev/binderfs, the above files will be available at the following locations: /dev/binderfs/binder_logs/state /dev/binderfs/binder_logs/stats /dev/binderfs/binder_logs/transactions This provides a way to access them even when debugfs is not mounted. Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NHridya Valsaraju <hridya@google.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20190903161655.107408-3-hridya@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Hridya Valsaraju 提交于
Currently, since each binderfs instance needs its own private binder devices, every time a binderfs instance is mounted, all the default binder devices need to be created via the BINDER_CTL_ADD IOCTL. This patch aims to add a solution to automatically create the default binder devices for each binderfs instance that gets mounted. To achieve this goal, when CONFIG_ANDROID_BINDERFS is set, the default binder devices specified by CONFIG_ANDROID_BINDER_DEVICES are created in each binderfs instance instead of global devices being created by the binder driver. Co-developed-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NHridya Valsaraju <hridya@google.com> Reviewed-by: NJoel Fernandes (Google) <joel@joelfernandes.org> Link: https://lore.kernel.org/r/20190808222727.132744-2-hridya@google.com Link: https://lore.kernel.org/r/20190904110704.8606-2-christian.brauner@ubuntu.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 24 7月, 2019 2 次提交
-
-
由 Hridya Valsaraju 提交于
Currently, a transaction to context manager from its own process is prevented by checking if its binder_proc struct is the same as that of the sender. However, this would not catch cases where the process opens the binder device again and uses the new fd to send a transaction to the context manager. Reported-by: syzbot+8b3c354d33c4ac78bfad@syzkaller.appspotmail.com Signed-off-by: NHridya Valsaraju <hridya@google.com> Acked-by: NTodd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Link: https://lore.kernel.org/r/20190715191804.112933-1-hridya@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Martijn Coenen 提交于
In case the target node requests a security context, the extra_buffers_size is increased with the size of the security context. But, that size is not available for use by regular scatter-gather buffers; make sure the ending of that buffer is marked correctly. Acked-by: NTodd Kjos <tkjos@google.com> Fixes: ec74136d ("binder: create node flag to request sender's security context") Signed-off-by: NMartijn Coenen <maco@android.com> Cc: stable@vger.kernel.org # 5.1+ Link: https://lore.kernel.org/r/20190709110923.220736-1-maco@android.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 01 7月, 2019 1 次提交
-
-
由 Todd Kjos 提交于
The buffer copy functions assumed the caller would ensure correct alignment and that the memory to be copied was completely within the binder buffer. There have been a few cases discovered by syzkallar where a malformed transaction created by a user could violated the assumptions and resulted in a BUG_ON. The fix is to remove the BUG_ON and always return the error to be handled appropriately by the caller. Acked-by: NMartijn Coenen <maco@android.com> Reported-by: syzbot+3ae18325f96190606754@syzkaller.appspotmail.com Fixes: bde4a19f ("binder: use userspace pointer as base of buffer space") Suggested-by: NDan Carpenter <dan.carpenter@oracle.com> Signed-off-by: NTodd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 22 6月, 2019 1 次提交
-
-
由 Todd Kjos 提交于
syzkallar found a 32-byte memory leak in a rarely executed error case. The transaction complete work item was not freed if put_user() failed when writing the BR_TRANSACTION_COMPLETE to the user command buffer. Fixed by freeing it before put_user() is called. Reported-by: syzbot+182ce46596c3f2e1eb24@syzkaller.appspotmail.com Signed-off-by: NTodd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 13 6月, 2019 1 次提交
-
-
由 Todd Kjos 提交于
There is a race between the binder driver cleaning up a completed transaction via binder_free_transaction() and a user calling binder_ioctl(BC_FREE_BUFFER) to release a buffer. It doesn't matter which is first but they need to be protected against running concurrently which can result in a UAF. Signed-off-by: NTodd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 05 6月, 2019 1 次提交
-
-
由 Thomas Gleixner 提交于
Based on 1 normalized pattern(s): this software is licensed under the terms of the gnu general public license version 2 as published by the free software foundation and may be copied distributed and modified under those terms this program is distributed in the hope that it will be useful but without any warranty without even the implied warranty of merchantability or fitness for a particular purpose see the gnu general public license for more details extracted by the scancode license scanner the SPDX license identifier GPL-2.0-only has been chosen to replace the boilerplate/reference in 285 file(s). Signed-off-by: NThomas Gleixner <tglx@linutronix.de> Reviewed-by: NAlexios Zavras <alexios.zavras@intel.com> Reviewed-by: NAllison Randal <allison@lohutok.net> Cc: linux-spdx@vger.kernel.org Link: https://lkml.kernel.org/r/20190529141900.642774971@linutronix.deSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-