linux.git/drivers/android, branch v5.4.165 Clone of https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git binder: use wake_up_pollfree() 2021-12-14T13:49:02+00:00 Eric Biggers ebiggers@google.com 2021-12-10T23:50:51+00:00 1a478a0522e5618480ee298e886eed18dcac1459 commit a880b28a71e39013e357fd3adccd1d8a31bc69a8 upstream. wake_up_poll() uses nr_exclusive=1, so it's not guaranteed to wake up all exclusive waiters. Yet, POLLFREE *must* wake up all waiters. epoll and aio poll are fortunately not affected by this, but it's very fragile. Thus, the new function wake_up_pollfree() has been introduced. Convert binder to use wake_up_pollfree(). Reported-by: Linus Torvalds <torvalds@linux-foundation.org> Fixes: f5cb779ba163 ("ANDROID: binder: remove waitqueue when thread exits.") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20211209010455.42744-3-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit a880b28a71e39013e357fd3adccd1d8a31bc69a8 upstream.

wake_up_poll() uses nr_exclusive=1, so it's not guaranteed to wake up
all exclusive waiters.  Yet, POLLFREE *must* wake up all waiters.  epoll
and aio poll are fortunately not affected by this, but it's very
fragile.  Thus, the new function wake_up_pollfree() has been introduced.

Convert binder to use wake_up_pollfree().

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Fixes: f5cb779ba163 ("ANDROID: binder: remove waitqueue when thread exits.")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20211209010455.42744-3-ebiggers@kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
binder: fix test regression due to sender_euid change 2021-12-01T08:23:28+00:00 Todd Kjos tkjos@google.com 2021-11-12T18:07:20+00:00 aaa83768ba39586fe0abe2fb1825b89f3bf6270c commit c21a80ca0684ec2910344d72556c816cb8940c01 upstream. This is a partial revert of commit 29bc22ac5e5b ("binder: use euid from cred instead of using task"). Setting sender_euid using proc->cred caused some Android system test regressions that need further investigation. It is a partial reversion because subsequent patches rely on proc->cred. Fixes: 29bc22ac5e5b ("binder: use euid from cred instead of using task") Cc: stable@vger.kernel.org # 4.4+ Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Todd Kjos <tkjos@google.com> Change-Id: I9b1769a3510fed250bb21859ef8beebabe034c66 Link: https://lore.kernel.org/r/20211112180720.2858135-1-tkjos@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit c21a80ca0684ec2910344d72556c816cb8940c01 upstream.

This is a partial revert of commit
29bc22ac5e5b ("binder: use euid from cred instead of using task").
Setting sender_euid using proc->cred caused some Android system test
regressions that need further investigation. It is a partial
reversion because subsequent patches rely on proc->cred.

Fixes: 29bc22ac5e5b ("binder: use euid from cred instead of using task")
Cc: stable@vger.kernel.org # 4.4+
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Todd Kjos <tkjos@google.com>
Change-Id: I9b1769a3510fed250bb21859ef8beebabe034c66
Link: https://lore.kernel.org/r/20211112180720.2858135-1-tkjos@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
binder: use cred instead of task for getsecid 2021-11-17T08:48:16+00:00 Todd Kjos tkjos@google.com 2021-11-10T23:00:25+00:00 db39f49ee7d50e68bf46796c05d9deae256ce986 commit 4d5b5539742d2554591751b4248b0204d20dcc9d upstream. Use the 'struct cred' saved at binder_open() to lookup the security ID via security_cred_getsecid(). This ensures that the security context that opened binder is the one used to generate the secctx. Cc: stable@vger.kernel.org # 5.4+ Fixes: ec74136ded79 ("binder: create node flag to request sender's security context") Signed-off-by: Todd Kjos <tkjos@google.com> Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com> Reported-by: kernel test robot <lkp@intel.com> Acked-by: Casey Schaufler <casey@schaufler-ca.com> Signed-off-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit 4d5b5539742d2554591751b4248b0204d20dcc9d upstream.

Use the 'struct cred' saved at binder_open() to lookup
the security ID via security_cred_getsecid(). This
ensures that the security context that opened binder
is the one used to generate the secctx.

Cc: stable@vger.kernel.org # 5.4+
Fixes: ec74136ded79 ("binder: create node flag to request sender's security context")
Signed-off-by: Todd Kjos <tkjos@google.com>
Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Reported-by: kernel test robot <lkp@intel.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
binder: use cred instead of task for selinux checks 2021-11-17T08:48:16+00:00 Todd Kjos tkjos@google.com 2021-11-10T23:00:24+00:00 fc9c470cd519e5bb37cf4c92603b2d4e604f5b71 commit 52f88693378a58094c538662ba652aff0253c4fe upstream. Since binder was integrated with selinux, it has passed 'struct task_struct' associated with the binder_proc to represent the source and target of transactions. The conversion of task to SID was then done in the hook implementations. It turns out that there are race conditions which can result in an incorrect security context being used. Fix by using the 'struct cred' saved during binder_open and pass it to the selinux subsystem. Cc: stable@vger.kernel.org # 5.14 (need backport for earlier stables) Fixes: 79af73079d75 ("Add security hooks to binder and implement the hooks for SELinux.") Suggested-by: Jann Horn <jannh@google.com> Signed-off-by: Todd Kjos <tkjos@google.com> Acked-by: Casey Schaufler <casey@schaufler-ca.com> Signed-off-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit 52f88693378a58094c538662ba652aff0253c4fe upstream.

Since binder was integrated with selinux, it has passed
'struct task_struct' associated with the binder_proc
to represent the source and target of transactions.
The conversion of task to SID was then done in the hook
implementations. It turns out that there are race conditions
which can result in an incorrect security context being used.

Fix by using the 'struct cred' saved during binder_open and pass
it to the selinux subsystem.

Cc: stable@vger.kernel.org # 5.14 (need backport for earlier stables)
Fixes: 79af73079d75 ("Add security hooks to binder and implement the hooks for SELinux.")
Suggested-by: Jann Horn <jannh@google.com>
Signed-off-by: Todd Kjos <tkjos@google.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
binder: use euid from cred instead of using task 2021-11-17T08:48:16+00:00 Todd Kjos tkjos@google.com 2021-11-10T23:00:23+00:00 28a1e470b000d45bcf6c05f18a01d07cdc0b3235 commit 29bc22ac5e5bc63275e850f0c8fc549e3d0e306b upstream. Save the 'struct cred' associated with a binder process at initial open to avoid potential race conditions when converting to an euid. Set a transaction's sender_euid from the 'struct cred' saved at binder_open() instead of looking up the euid from the binder proc's 'struct task'. This ensures the euid is associated with the security context that of the task that opened binder. Cc: stable@vger.kernel.org # 4.4+ Fixes: 457b9a6f09f0 ("Staging: android: add binder driver") Signed-off-by: Todd Kjos <tkjos@google.com> Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com> Suggested-by: Jann Horn <jannh@google.com> Acked-by: Casey Schaufler <casey@schaufler-ca.com> Signed-off-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit 29bc22ac5e5bc63275e850f0c8fc549e3d0e306b upstream.

Save the 'struct cred' associated with a binder process
at initial open to avoid potential race conditions
when converting to an euid.

Set a transaction's sender_euid from the 'struct cred'
saved at binder_open() instead of looking up the euid
from the binder proc's 'struct task'. This ensures
the euid is associated with the security context that
of the task that opened binder.

Cc: stable@vger.kernel.org # 4.4+
Fixes: 457b9a6f09f0 ("Staging: android: add binder driver")
Signed-off-by: Todd Kjos <tkjos@google.com>
Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Suggested-by: Jann Horn <jannh@google.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
binder: don't detect sender/target during buffer cleanup 2021-11-12T13:43:03+00:00 Todd Kjos tkjos@google.com 2021-10-15T23:38:11+00:00 589ac131b3abb3beba51782d62ace557b8a314ee commit 32e9f56a96d8d0f23cb2aeb2a3cd18d40393e787 upstream. When freeing txn buffers, binder_transaction_buffer_release() attempts to detect whether the current context is the target by comparing current->group_leader to proc->tsk. This is an unreliable test. Instead explicitly pass an 'is_failure' boolean. Detecting the sender was being used as a way to tell if the transaction failed to be sent. When cleaning up after failing to send a transaction, there is no need to close the fds associated with a BINDER_TYPE_FDA object. Now 'is_failure' can be used to accurately detect this case. Fixes: 44d8047f1d87 ("binder: use standard functions to allocate fds") Cc: stable <stable@vger.kernel.org> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20211015233811.3532235-1-tkjos@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit 32e9f56a96d8d0f23cb2aeb2a3cd18d40393e787 upstream.

When freeing txn buffers, binder_transaction_buffer_release()
attempts to detect whether the current context is the target by
comparing current->group_leader to proc->tsk. This is an unreliable
test. Instead explicitly pass an 'is_failure' boolean.

Detecting the sender was being used as a way to tell if the
transaction failed to be sent.  When cleaning up after
failing to send a transaction, there is no need to close
the fds associated with a BINDER_TYPE_FDA object. Now
'is_failure' can be used to accurately detect this case.

Fixes: 44d8047f1d87 ("binder: use standard functions to allocate fds")
Cc: stable <stable@vger.kernel.org>
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Todd Kjos <tkjos@google.com>
Link: https://lore.kernel.org/r/20211015233811.3532235-1-tkjos@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
binder: make sure fd closes complete 2021-09-30T08:09:21+00:00 Todd Kjos tkjos@google.com 2021-08-30T19:51:46+00:00 aa2c274c279ff365a06a4cba263f04965895166e commit 5fdb55c1ac9585eb23bb2541d5819224429e103d 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: 80cd795630d6 ("binder: fix use-after-free due to ksys_close() during fdget()") Cc: stable <stable@vger.kernel.org> Reviewed-by: Martijn Coenen <maco@android.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20210830195146.587206-1-tkjos@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit 5fdb55c1ac9585eb23bb2541d5819224429e103d 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: 80cd795630d6 ("binder: fix use-after-free due to ksys_close() during fdget()")
Cc: stable <stable@vger.kernel.org>
Reviewed-by: Martijn Coenen <maco@android.com>
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Todd Kjos <tkjos@google.com>
Link: https://lore.kernel.org/r/20210830195146.587206-1-tkjos@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
binder: add flag to clear buffer on txn complete 2020-12-30T10:51:35+00:00 Todd Kjos tkjos@google.com 2020-11-20T23:37:43+00:00 5fbf84689f11dd68566f3bd155111f2b72a6d193 commit 0f966cba95c78029f491b433ea95ff38f414a761 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: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20201120233743.3617529-1-tkjos@google.com Cc: stable <stable@vger.kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit 0f966cba95c78029f491b433ea95ff38f414a761 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: Todd Kjos <tkjos@google.com>
Link: https://lore.kernel.org/r/20201120233743.3617529-1-tkjos@google.com
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
binder: Remove bogus warning on failed same-process transaction 2020-10-29T08:57:37+00:00 Jann Horn jannh@google.com 2020-08-06T16:53:59+00:00 2b150aa2e3ef864df904acceb976d688d7ea19a6 [ Upstream commit e8b8ae7ce32e17a5c29f0289e9e2a39c7dcaa1b8 ] While binder transactions with the same binder_proc as sender and recipient are forbidden, transactions with the same task_struct as sender and recipient are possible (even though currently there is a weird check in binder_transaction() that rejects them in the target==0 case). Therefore, task_struct identities can't be used to distinguish whether the caller is running in the context of the sender or the recipient. Since I see no easy way to make this WARN_ON() useful and correct, let's just remove it. Fixes: 44d8047f1d87 ("binder: use standard functions to allocate fds") Reported-by: syzbot+e113a0b970b7b3f394ba@syzkaller.appspotmail.com Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Acked-by: Todd Kjos <tkjos@google.com> Signed-off-by: Jann Horn <jannh@google.com> Link: https://lore.kernel.org/r/20200806165359.2381483-1-jannh@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Sasha Levin <sashal@kernel.org> 
[ Upstream commit e8b8ae7ce32e17a5c29f0289e9e2a39c7dcaa1b8 ]

While binder transactions with the same binder_proc as sender and recipient
are forbidden, transactions with the same task_struct as sender and
recipient are possible (even though currently there is a weird check in
binder_transaction() that rejects them in the target==0 case).
Therefore, task_struct identities can't be used to distinguish whether
the caller is running in the context of the sender or the recipient.

Since I see no easy way to make this WARN_ON() useful and correct, let's
just remove it.

Fixes: 44d8047f1d87 ("binder: use standard functions to allocate fds")
Reported-by: syzbot+e113a0b970b7b3f394ba@syzkaller.appspotmail.com
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Acked-by: Todd Kjos <tkjos@google.com>
Signed-off-by: Jann Horn <jannh@google.com>
Link: https://lore.kernel.org/r/20200806165359.2381483-1-jannh@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
binder: fix UAF when releasing todo list 2020-10-29T08:57:25+00:00 Todd Kjos tkjos@google.com 2020-10-09T23:24:55+00:00 401d4d79a8ed5ac1c78031a00f8ac414e6605a38 commit f3277cbfba763cd2826396521b9296de67cf1bbc upstream. When releasing a thread todo list when tearing down a binder_proc, the following race was possible which could result in a use-after-free: 1. Thread 1: enter binder_release_work from binder_thread_release 2. Thread 2: binder_update_ref_for_handle() -> binder_dec_node_ilocked() 3. Thread 2: dec nodeA --> 0 (will free node) 4. Thread 1: ACQ inner_proc_lock 5. Thread 2: block on inner_proc_lock 6. Thread 1: dequeue work (BINDER_WORK_NODE, part of nodeA) 7. Thread 1: REL inner_proc_lock 8. Thread 2: ACQ inner_proc_lock 9. Thread 2: todo list cleanup, but work was already dequeued 10. Thread 2: free node 11. Thread 2: REL inner_proc_lock 12. Thread 1: deref w->type (UAF) The problem was that for a BINDER_WORK_NODE, the binder_work element must not be accessed after releasing the inner_proc_lock while processing the todo list elements since another thread might be handling a deref on the node containing the binder_work element leading to the node being freed. Signed-off-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20201009232455.4054810-1-tkjos@google.com Cc: <stable@vger.kernel.org> # 4.14, 4.19, 5.4, 5.8 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit f3277cbfba763cd2826396521b9296de67cf1bbc upstream.

When releasing a thread todo list when tearing down
a binder_proc, the following race was possible which
could result in a use-after-free:

1.  Thread 1: enter binder_release_work from binder_thread_release
2.  Thread 2: binder_update_ref_for_handle() -> binder_dec_node_ilocked()
3.  Thread 2: dec nodeA --> 0 (will free node)
4.  Thread 1: ACQ inner_proc_lock
5.  Thread 2: block on inner_proc_lock
6.  Thread 1: dequeue work (BINDER_WORK_NODE, part of nodeA)
7.  Thread 1: REL inner_proc_lock
8.  Thread 2: ACQ inner_proc_lock
9.  Thread 2: todo list cleanup, but work was already dequeued
10. Thread 2: free node
11. Thread 2: REL inner_proc_lock
12. Thread 1: deref w->type (UAF)

The problem was that for a BINDER_WORK_NODE, the binder_work element
must not be accessed after releasing the inner_proc_lock while
processing the todo list elements since another thread might be
handling a deref on the node containing the binder_work element
leading to the node being freed.

Signed-off-by: Todd Kjos <tkjos@google.com>
Link: https://lore.kernel.org/r/20201009232455.4054810-1-tkjos@google.com
Cc: <stable@vger.kernel.org> # 4.14, 4.19, 5.4, 5.8
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>