- 18 1月, 2023 2 次提交
-
-
由 Carlos Llamas 提交于
stable inclusion from stable-v5.10.154 commit 015ac18be7de25d17d6e5f1643cb3b60bfbe859e category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I68WW5 CVE: CVE-2023-20928 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=015ac18be7de25d17d6e5f1643cb3b60bfbe859e -------------------------------- In commit 720c2419 ("ANDROID: binder: change down_write to down_read") binder assumed the mmap read lock is sufficient to protect alloc->vma inside binder_update_page_range(). This used to be accurate until commit dd2283f2 ("mm: mmap: zap pages with read mmap_sem in munmap"), which now downgrades the mmap_lock after detaching the vma from the rbtree in munmap(). Then it proceeds to teardown and free the vma with only the read lock held. This means that accesses to alloc->vma in binder_update_page_range() now will race with vm_area_free() in munmap() and can cause a UAF as shown in the following KASAN trace: ================================================================== BUG: KASAN: use-after-free in vm_insert_page+0x7c/0x1f0 Read of size 8 at addr ffff16204ad00600 by task server/558 CPU: 3 PID: 558 Comm: server Not tainted 5.10.150-00001-gdc8dcf942daa #1 Hardware name: linux,dummy-virt (DT) Call trace: dump_backtrace+0x0/0x2a0 show_stack+0x18/0x2c dump_stack+0xf8/0x164 print_address_description.constprop.0+0x9c/0x538 kasan_report+0x120/0x200 __asan_load8+0xa0/0xc4 vm_insert_page+0x7c/0x1f0 binder_update_page_range+0x278/0x50c binder_alloc_new_buf+0x3f0/0xba0 binder_transaction+0x64c/0x3040 binder_thread_write+0x924/0x2020 binder_ioctl+0x1610/0x2e5c __arm64_sys_ioctl+0xd4/0x120 el0_svc_common.constprop.0+0xac/0x270 do_el0_svc+0x38/0xa0 el0_svc+0x1c/0x2c el0_sync_handler+0xe8/0x114 el0_sync+0x180/0x1c0 Allocated by task 559: kasan_save_stack+0x38/0x6c __kasan_kmalloc.constprop.0+0xe4/0xf0 kasan_slab_alloc+0x18/0x2c kmem_cache_alloc+0x1b0/0x2d0 vm_area_alloc+0x28/0x94 mmap_region+0x378/0x920 do_mmap+0x3f0/0x600 vm_mmap_pgoff+0x150/0x17c ksys_mmap_pgoff+0x284/0x2dc __arm64_sys_mmap+0x84/0xa4 el0_svc_common.constprop.0+0xac/0x270 do_el0_svc+0x38/0xa0 el0_svc+0x1c/0x2c el0_sync_handler+0xe8/0x114 el0_sync+0x180/0x1c0 Freed by task 560: kasan_save_stack+0x38/0x6c kasan_set_track+0x28/0x40 kasan_set_free_info+0x24/0x4c __kasan_slab_free+0x100/0x164 kasan_slab_free+0x14/0x20 kmem_cache_free+0xc4/0x34c vm_area_free+0x1c/0x2c remove_vma+0x7c/0x94 __do_munmap+0x358/0x710 __vm_munmap+0xbc/0x130 __arm64_sys_munmap+0x4c/0x64 el0_svc_common.constprop.0+0xac/0x270 do_el0_svc+0x38/0xa0 el0_svc+0x1c/0x2c el0_sync_handler+0xe8/0x114 el0_sync+0x180/0x1c0 [...] ================================================================== To prevent the race above, revert back to taking the mmap write lock inside binder_update_page_range(). One might expect an increase of mmap lock contention. However, binder already serializes these calls via top level alloc->mutex. Also, there was no performance impact shown when running the binder benchmark tests. Note this patch is specific to stable branches 5.4 and 5.10. Since in newer kernel releases binder no longer caches a pointer to the vma. Instead, it has been refactored to use vma_lookup() which avoids the issue described here. This switch was introduced in commit a43cfc87 ("android: binder: stop saving a pointer to the VMA"). Fixes: dd2283f2 ("mm: mmap: zap pages with read mmap_sem in munmap") Reported-by: NJann Horn <jannh@google.com> Cc: <stable@vger.kernel.org> # 5.10.x Cc: Minchan Kim <minchan@kernel.org> Cc: Yang Shi <yang.shi@linux.alibaba.com> Cc: Liam Howlett <liam.howlett@oracle.com> Signed-off-by: NCarlos Llamas <cmllamas@google.com> Acked-by: NTodd Kjos <tkjos@google.com> Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: NChen Jiahao <chenjiahao16@huawei.com> Reviewed-by: NLiao Chang <liaochang1@huawei.com> Reviewed-by: NZhang Jianhua <chris.zjh@huawei.com> Signed-off-by: NJialin Zhang <zhangjialin11@huawei.com>
-
由 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>
-
- 27 10月, 2022 1 次提交
-
-
由 Al Viro 提交于
mainline inclusion from mainline-v6.1-rc1 commit 9d64d240 category: bugfix bugzilla: 187857, https://gitee.com/src-openeuler/kernel/issues/I5X9DT CVE: CVE-2022-3577 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9d64d2405f7d30d49818f6682acd0392348f0fdb -------------------------------- So far we relied on .put_super = binderfs_put_super() to destroy info we stashed in sb->s_fs_info. This gave us the required ordering between ->evict_inode() and sb->s_fs_info destruction. But the current implementation of binderfs_fill_super() has a memory leak in the rare circumstance that d_make_root() fails because ->put_super() is only called when sb->s_root is initialized. Fix this by removing ->put_super() and simply do all that work in binderfs_kill_super(). Reported-by: NDongliang Mu <mudongliangabcd@gmail.com> Signed-off-by: NAl Viro <viro@zeniv.linux.org.uk> Signed-off-by: NChristian Brauner (Microsoft) <brauner@kernel.org> Link: https://lore.kernel.org/r/20220823095339.853371-1-brauner@kernel.orgSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: NLi Huafei <lihuafei1@huawei.com> Reviewed-by: NKuohai Xu <xukuohai@huawei.com> Signed-off-by: NZheng Zengkai <zhengzengkai@huawei.com>
-
- 19 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>
-
- 28 1月, 2022 1 次提交
-
-
由 Todd Kjos 提交于
stable inclusion from stable-v5.10.90 commit 1cb8444f3114f0bb2f6e3bcadcf09aa4a28425d4 bugzilla: 186168 https://gitee.com/openeuler/kernel/issues/I4SHY1 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=1cb8444f3114f0bb2f6e3bcadcf09aa4a28425d4 -------------------------------- commit cfd0d84b upstream. In 4.13, commit 74310e06 ("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: 74310e06 ("android: binder: Move buffer out of area shared with user space") Signed-off-by: NTodd Kjos <tkjos@google.com> Cc: stable@vger.kernel.org # 4.14+ Link: https://lore.kernel.org/r/20211220190150.2107077-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>
-
- 22 1月, 2022 2 次提交
-
-
由 Jialin Zhang 提交于
hulk inclusion category: feature bugzilla: https://gitee.com/openeuler/kernel/issues/I4RCS8 CVE: NA Reference: https://android.googlesource.com/kernel/common/ --------------------------- Make android vendor hooks feature generic. Signed-off-by: NJialin Zhang <zhangjialin11@huawei.com> Reviewed-by: NWei Li <liwei391@huawei.com> Reviewed-by: NXie XiuQi <xiexiuqi@huawei.com> Signed-off-by: NZheng Zengkai <zhengzengkai@huawei.com>
-
由 Todd Kjos 提交于
aosp inclusion category: feature bugzilla: https://gitee.com/openeuler/kernel/issues/I4RCS8 CVE: NA Reference: https://android.googlesource.com/kernel/common/+/7f62740112ef --------------------------- Add support for vendor hooks. Adds include/trace/hooks directory for trace definition headers where hooks can be defined and vendor_hook.c for instantiating and exporting them for vendor modules. There are two variants of vendor hooks, both based on tracepoints: Normal: this uses the DECLARE_HOOK macro to create a tracepoint function with the name trace_<name> where <name> is the unique identifier for the trace. Restricted: restricted hooks are needed for cases like scheduler hooks where the attached function must be called even if the cpu is offline or requires a non-atomic context. Restricted vendor hooks cannot be detached, so modules that attach to a restricted hook can never unload. Also, only 1 attachment is allowed (any other attempts to attach will fail with -EBUSY). For either case, modules attach to the hook by using register_trace_<name>(func_ptr, NULL). New hooks should be defined in headers in the include/trace/hooks/ directory using the DECLARE_HOOK() or DECLARE_RESTRICTED_HOOK() macros. New files added to include/trace/hooks should be #include'd from drivers/android/vendor_hooks.c. The EXPORT_TRACEPOINT_SYMBOL_GPL() should be also added to drivers/android/vendor_hooks.c. For example, if a new hook, 'android_vh_foo(int &ret)' is added in do_exit() in exit.c, these changes are needed: 1. create a new header file include/trace/hooks/foo.h which contains: #include <trace/hooks/vendor_hooks.h> ... DECLARE_HOOK(android_vh_foo, TP_PROTO(int *retp), TP_ARGS(retp); 2. in exit.c, add #include <trace/hooks/foo.h> ... int ret = 0; ... android_vh_foo(&ret); if (ret) return ret; ... 3. in drivers/android/vendor_hooks.c, add #include <trace/hooks/foo.h> ... EXPORT_TRACEPOINT_SYMBOL_GPL(android_vh_foo); The hook can then be attached by adding the registration code to the module: #include <trace/hooks/sched.h> ... static void my_foo(int *retp) { *retp = 0; } ... rc = register_trace_android_vh_sched_exit(my_foo, NULL); Bug: 156285741 Signed-off-by: NTodd Kjos <tkjos@google.com> Change-Id: I6a7d1c8919dae91c965e2a0450df50eac2d282db Signed-off-by: NJialin Zhang <zhangjialin11@huawei.com> Reviewed-by: NWei Li <liwei391@huawei.com> Reviewed-by: NXie XiuQi <xiexiuqi@huawei.com> Signed-off-by: NZheng Zengkai <zhengzengkai@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>
-
- 16 9月, 2020 1 次提交
-
-
由 Colin Ian King 提交于
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: NTodd Kjos <tkjos@google.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NColin Ian King <colin.king@canonical.com> Link: https://lore.kernel.org/r/20200910151221.751464-1-colin.king@canonical.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 04 9月, 2020 4 次提交
-
-
由 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>
-
由 Wei Yongjun 提交于
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: 095cf502 ("binderfs: port to new mount api") Reported-by: NHulk Robot <hulkci@huawei.com> Signed-off-by: NWei Yongjun <weiyongjun1@huawei.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20200818112245.43891-1-weiyongjun1@huawei.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 YangHui 提交于
The function name should is binder_alloc_new_buf() Signed-off-by: NYangHui <yanghui.def@gmail.com> Reviewed-by: NMartijn Coenen <maco@android.com> Link: https://lore.kernel.org/r/1597714444-3614-1-git-send-email-yanghui.def@gmail.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 6 次提交
-
-
由 Mrinal Pandey 提交于
C source files should have `//` as SPDX comment and not `/**/`. Fix this by running checkpatch on the file. Signed-off-by: NMrinal Pandey <mrinalmni@gmail.com> Link: https://lore.kernel.org/r/20200724131449.zvjutbemg3vqhrzh@mrinalpandeySigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Mrinal Pandey 提交于
Add a blank line after variable declarations as suggested by checkpatch. Signed-off-by: NMrinal Pandey <mrinalmni@gmail.com> Link: https://lore.kernel.org/r/20200724131433.stf3ycooogawyzb3@mrinalpandeySigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 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>
-
由 Mrinal Pandey 提交于
Add a blank line after variable declarations as suggested by checkpatch. Signed-off-by: NMrinal Pandey <mrinalmni@gmail.com> Link: https://lore.kernel.org/r/20200724131254.qxbvderrws36dzzq@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 7月, 2020 1 次提交
-
-
由 Tetsuo Handa 提交于
syzbot is reporting that mmput() from shrinker function has a risk of deadlock [1], for delayed_uprobe_add() from update_ref_ctr() calls kzalloc(GFP_KERNEL) with delayed_uprobe_lock held, and uprobe_clear_state() from __mmput() also holds delayed_uprobe_lock. Commit a1b2289c ("android: binder: drop lru lock in isolate callback") replaced mmput() with mmput_async() in order to avoid sleeping with spinlock held. But this patch replaces mmput() with mmput_async() in order not to start __mmput() from shrinker context. [1] https://syzkaller.appspot.com/bug?id=bc9e7303f537c41b2b0cc2dfcea3fc42964c2d45Reported-by: Nsyzbot <syzbot+1068f09c44d151250c33@syzkaller.appspotmail.com> Reported-by: Nsyzbot <syzbot+e5344baa319c9a96edec@syzkaller.appspotmail.com> Signed-off-by: NTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Reviewed-by: NMichal Hocko <mhocko@suse.com> Acked-by: NTodd Kjos <tkjos@google.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Cc: stable <stable@vger.kernel.org> Link: https://lore.kernel.org/r/4ba9adb2-43f5-2de0-22de-f6075c1fab50@i-love.sakura.ne.jpSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 23 6月, 2020 1 次提交
-
-
由 Todd Kjos 提交于
The binder driver makes the assumption proc->context pointer is invariant after initialization (as documented in the kerneldoc header for struct proc). However, in commit f0fe2c0f ("binder: prevent UAF for binderfs devices II") proc->context is set to NULL during binder_deferred_release(). Another proc was in the middle of setting up a transaction to the dying process and crashed on a NULL pointer deref on "context" which is a local set to &proc->context: new_ref->data.desc = (node == context->binder_context_mgr_node) ? 0 : 1; Here's the stack: [ 5237.855435] Call trace: [ 5237.855441] binder_get_ref_for_node_olocked+0x100/0x2ec [ 5237.855446] binder_inc_ref_for_node+0x140/0x280 [ 5237.855451] binder_translate_binder+0x1d0/0x388 [ 5237.855456] binder_transaction+0x2228/0x3730 [ 5237.855461] binder_thread_write+0x640/0x25bc [ 5237.855466] binder_ioctl_write_read+0xb0/0x464 [ 5237.855471] binder_ioctl+0x30c/0x96c [ 5237.855477] do_vfs_ioctl+0x3e0/0x700 [ 5237.855482] __arm64_sys_ioctl+0x78/0xa4 [ 5237.855488] el0_svc_common+0xb4/0x194 [ 5237.855493] el0_svc_handler+0x74/0x98 [ 5237.855497] el0_svc+0x8/0xc The fix is to move the kfree of the binder_device to binder_free_proc() so the binder_device is freed when we know there are no references remaining on the binder_proc. Fixes: f0fe2c0f ("binder: prevent UAF for binderfs devices II") Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NTodd Kjos <tkjos@google.com> Cc: stable <stable@vger.kernel.org> Link: https://lore.kernel.org/r/20200622200715.114382-1-tkjos@google.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 14 6月, 2020 1 次提交
-
-
由 Masahiro Yamada 提交于
Since commit 84af7a61 ("checkpatch: kconfig: prefer 'help' over '---help---'"), the number of '---help---' has been gradually decreasing, but there are still more than 2400 instances. This commit finishes the conversion. While I touched the lines, I also fixed the indentation. There are a variety of indentation styles found. a) 4 spaces + '---help---' b) 7 spaces + '---help---' c) 8 spaces + '---help---' d) 1 space + 1 tab + '---help---' e) 1 tab + '---help---' (correct indentation) f) 1 tab + 1 space + '---help---' g) 1 tab + 2 spaces + '---help---' In order to convert all of them to 1 tab + 'help', I ran the following commend: $ find . -name 'Kconfig*' | xargs sed -i 's/^[[:space:]]*---help---/\thelp/' Signed-off-by: NMasahiro Yamada <masahiroy@kernel.org>
-
- 10 6月, 2020 2 次提交
-
-
由 Michel Lespinasse 提交于
Convert comments that reference old mmap_sem APIs to reference corresponding new mmap locking APIs instead. Signed-off-by: NMichel Lespinasse <walken@google.com> Signed-off-by: NAndrew Morton <akpm@linux-foundation.org> Reviewed-by: NVlastimil Babka <vbabka@suse.cz> Reviewed-by: NDavidlohr Bueso <dbueso@suse.de> Reviewed-by: NDaniel Jordan <daniel.m.jordan@oracle.com> Cc: David Rientjes <rientjes@google.com> Cc: Hugh Dickins <hughd@google.com> Cc: Jason Gunthorpe <jgg@ziepe.ca> Cc: Jerome Glisse <jglisse@redhat.com> Cc: John Hubbard <jhubbard@nvidia.com> Cc: Laurent Dufour <ldufour@linux.ibm.com> Cc: Liam Howlett <Liam.Howlett@oracle.com> Cc: Matthew Wilcox <willy@infradead.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ying Han <yinghan@google.com> Link: http://lkml.kernel.org/r/20200520052908.204642-12-walken@google.comSigned-off-by: NLinus Torvalds <torvalds@linux-foundation.org>
-
由 Michel Lespinasse 提交于
This change converts the existing mmap_sem rwsem calls to use the new mmap locking API instead. The change is generated using coccinelle with the following rule: // spatch --sp-file mmap_lock_api.cocci --in-place --include-headers --dir . @@ expression mm; @@ ( -init_rwsem +mmap_init_lock | -down_write +mmap_write_lock | -down_write_killable +mmap_write_lock_killable | -down_write_trylock +mmap_write_trylock | -up_write +mmap_write_unlock | -downgrade_write +mmap_write_downgrade | -down_read +mmap_read_lock | -down_read_killable +mmap_read_lock_killable | -down_read_trylock +mmap_read_trylock | -up_read +mmap_read_unlock ) -(&mm->mmap_sem) +(mm) Signed-off-by: NMichel Lespinasse <walken@google.com> Signed-off-by: NAndrew Morton <akpm@linux-foundation.org> Reviewed-by: NDaniel Jordan <daniel.m.jordan@oracle.com> Reviewed-by: NLaurent Dufour <ldufour@linux.ibm.com> Reviewed-by: NVlastimil Babka <vbabka@suse.cz> Cc: Davidlohr Bueso <dbueso@suse.de> Cc: David Rientjes <rientjes@google.com> Cc: Hugh Dickins <hughd@google.com> Cc: Jason Gunthorpe <jgg@ziepe.ca> Cc: Jerome Glisse <jglisse@redhat.com> Cc: John Hubbard <jhubbard@nvidia.com> Cc: Liam Howlett <Liam.Howlett@oracle.com> Cc: Matthew Wilcox <willy@infradead.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ying Han <yinghan@google.com> Link: http://lkml.kernel.org/r/20200520052908.204642-5-walken@google.comSigned-off-by: NLinus Torvalds <torvalds@linux-foundation.org>
-
- 23 4月, 2020 2 次提交
-
-
由 Colin Ian King 提交于
The pointer ctx is being initialized with a value that is never read and it is being updated later with a new value. The initialization is redundant and can be removed. Addresses-Coverity: ("Unused value") Signed-off-by: NColin Ian King <colin.king@canonical.com> Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Link: https://lore.kernel.org/r/20200402105000.506296-1-colin.king@canonical.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
由 Tang Bin 提交于
Fix missing braces compilation warning in the ARM compiler environment: drivers/android/binderfs.c: In function 'binderfs_fill_super': drivers/android/binderfs.c:650:9: warning: missing braces around initializer [-Wmissing-braces] struct binderfs_device device_info = { 0 }; drivers/android/binderfs.c:650:9: warning: (near initialization for ‘device_info.name’) [-Wmissing-braces] Acked-by: NChristian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NTang Bin <tangbin@cmss.chinamobile.com> Link: https://lore.kernel.org/r/20200411145151.5576-1-tangbin@cmss.chinamobile.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 19 3月, 2020 1 次提交
-
-
由 Christian Brauner 提交于
When I first wrote binderfs the new mount api had not yet landed. Now that it has been around for a little while and a bunch of filesystems have already been ported we should do so too. When Al sent his mount-api-conversion pr he requested that binderfs (and a few others) be ported separately. It's time we port binderfs. We can make use of the new option parser, get nicer infrastructure and it will be easier if we ever add any new mount options. This survives testing with the binderfs selftests: for i in `seq 1 1000`; do ./binderfs_test; done including the new stress tests I sent out for review today: TAP version 13 1..1 # selftests: filesystems/binderfs: binderfs_test # [==========] Running 3 tests from 1 test cases. # [ RUN ] global.binderfs_stress # [ XFAIL! ] Tests are not run as root. Skipping privileged tests # [==========] Running 3 tests from 1 test cases. # [ RUN ] global.binderfs_stress # [ OK ] global.binderfs_stress # [ RUN ] global.binderfs_test_privileged # [ OK ] global.binderfs_test_privileged # [ RUN ] global.binderfs_test_unprivileged # # Allocated new binder device with major 243, minor 4, and name my-binder # # Detected binder version: 8 # [==========] Running 3 tests from 1 test cases. # [ RUN ] global.binderfs_stress # [ OK ] global.binderfs_stress # [ RUN ] global.binderfs_test_privileged # [ OK ] global.binderfs_test_privileged # [ RUN ] global.binderfs_test_unprivileged # [ OK ] global.binderfs_test_unprivileged # [==========] 3 / 3 tests passed. # [ PASSED ] ok 1 selftests: filesystems/binderfs: binderfs_test Cc: Todd Kjos <tkjos@google.com> Signed-off-by: NChristian Brauner <christian.brauner@ubuntu.com> Reviewed-by: NKees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20200313153427.141789-1-christian.brauner@ubuntu.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 12 3月, 2020 1 次提交
-
-
由 Christian Brauner 提交于
Binderfs binder-control devices are cleaned up via binderfs_evict_inode too() which will use refcount_dec_and_test(). However, we missed to set the refcount for binderfs binder-control devices and so we underflowed when the binderfs instance got unmounted. Pretty obvious oversight and should have been part of the more general UAF fix. The good news is that having test cases (suprisingly) helps. Technically, we could detect that we're about to cleanup the binder-control dentry in binderfs_evict_inode() and then simply clean it up. But that makes the assumption that the binder driver itself will never make use of a binderfs binder-control device after the binderfs instance it belongs to has been unmounted and the superblock for it been destroyed. While it is unlikely to ever come to this let's be on the safe side. Performance-wise this also really doesn't matter since the binder-control device is only every really when creating the binderfs filesystem or creating additional binder devices. Both operations are pretty rare. Fixes: f0fe2c0f ("binder: prevent UAF for binderfs devices II") Link: https://lore.kernel.org/r/CA+G9fYusdfg7PMfC9Xce-xLT7NiyKSbgojpK35GOm=Pf9jXXrA@mail.gmail.comReported-by: NNaresh Kamboju <naresh.kamboju@linaro.org> Cc: stable@vger.kernel.org Signed-off-by: NChristian Brauner <christian.brauner@ubuntu.com> Acked-by: NTodd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20200311105309.1742827-1-christian.brauner@ubuntu.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-
- 04 3月, 2020 1 次提交
-
-
由 Christian Brauner 提交于
This is a necessary follow up to the first fix I proposed and we merged in 2669b8b0 ("binder: prevent UAF for binderfs devices"). I have been overly optimistic that the simple fix I proposed would work. But alas, ihold() + iput() won't work since the inodes won't survive the destruction of the superblock. So all we get with my prior fix is a different race with a tinier race-window but it doesn't solve the issue. Fwiw, the problem lies with generic_shutdown_super(). It even has this cozy Al-style comment: if (!list_empty(&sb->s_inodes)) { printk("VFS: Busy inodes after unmount of %s. " "Self-destruct in 5 seconds. Have a nice day...\n", sb->s_id); } On binder_release(), binder_defer_work(proc, BINDER_DEFERRED_RELEASE) is called which punts the actual cleanup operation to a workqueue. At some point, binder_deferred_func() will be called which will end up calling binder_deferred_release() which will retrieve and cleanup the binder_context attach to this struct binder_proc. If we trace back where this binder_context is attached to binder_proc we see that it is set in binder_open() and is taken from the struct binder_device it is associated with. This obviously assumes that the struct binder_device that context is attached to is _never_ freed. While that might be true for devtmpfs binder devices it is most certainly wrong for binderfs binder devices. So, assume binder_open() is called on a binderfs binder devices. We now stash away the struct binder_context associated with that struct binder_devices: proc->context = &binder_dev->context; /* binderfs stashes devices in i_private */ if (is_binderfs_device(nodp)) { binder_dev = nodp->i_private; info = nodp->i_sb->s_fs_info; binder_binderfs_dir_entry_proc = info->proc_log_dir; } else { . . . proc->context = &binder_dev->context; Now let's assume that the binderfs instance for that binder devices is shutdown via umount() and/or the mount namespace associated with it goes away. As long as there is still an fd open for that binderfs binder device things are fine. But let's assume we now close the last fd for that binderfs binder device. Now binder_release() is called and punts to the workqueue. Assume that the workqueue has quite a bit of stuff to do and doesn't get to cleaning up the struct binder_proc and the associated struct binder_context with it for that binderfs binder device right away. In the meantime, the VFS is killing the super block and is ultimately calling sb->evict_inode() which means it will call binderfs_evict_inode() which does: static void binderfs_evict_inode(struct inode *inode) { struct binder_device *device = inode->i_private; struct binderfs_info *info = BINDERFS_I(inode); clear_inode(inode); if (!S_ISCHR(inode->i_mode) || !device) return; mutex_lock(&binderfs_minors_mutex); --info->device_count; ida_free(&binderfs_minors, device->miscdev.minor); mutex_unlock(&binderfs_minors_mutex); kfree(device->context.name); kfree(device); } thereby freeing the struct binder_device including struct binder_context. Now the workqueue finally has time to get around to cleaning up struct binder_proc and is now trying to access the associate struct binder_context. Since it's already freed it will OOPs. Fix this by introducing a refounct on binder devices. This is an alternative fix to 51d8a7ec ("binder: prevent UAF read in print_binder_transaction_log_entry()"). Fixes: 3ad20fe3 ("binder: implement binderfs") Fixes: 2669b8b0 ("binder: prevent UAF for binderfs devices") Fixes: 03e2e07e ("binder: Make transaction_log available in binderfs") Related : 51d8a7ec ("binder: prevent UAF read in print_binder_transaction_log_entry()") Cc: stable@vger.kernel.org Signed-off-by: NChristian Brauner <christian.brauner@ubuntu.com> Acked-by: NTodd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20200303164340.670054-1-christian.brauner@ubuntu.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
-