(from https://patchwork.kernel.org/patch/9954125/)
Use binder_alloc struct's mm_struct rather than getting
a reference to the mm struct through get_task_mm to
avoid a potential deadlock between lru lock, task lock and
dentry lock, since a thread can be holding the task lock
and the dentry lock while trying to acquire the lru lock.
Test: ran binderLibTest, throughputtest, interfacetest and
mempressure w/lockdep
Bug: 63926541
Change-Id: Icc661404eb7a4a2ecc5234b1bf8f0104665f9b45
Acked-by: Arve Hjønnevåg <arve@android.com>
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9954123/)
The vma argument in update_page_range is no longer
used after 74310e06 ("android: binder: Move buffer
out of area shared with user space"), since mmap_handler
no longer calls update_page_range with a vma.
Test: ran binderLibTest, throughputtest, interfacetest and mempressure
Bug: 36007193
Change-Id: Ibd6f24c11750f8f7e6ed56e40dd18c08e02ace25
Acked-by: Arve Hjønnevåg <arve@android.com>
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9945123/)
Drop the global lru lock in isolate callback
before calling zap_page_range which calls
cond_resched, and re-acquire the global lru
lock before returning. Also change return
code to LRU_REMOVED_RETRY.
Use mmput_async when fail to acquire mmap sem
in an atomic context.
Fix "BUG: sleeping function called from invalid context"
errors when CONFIG_DEBUG_ATOMIC_SLEEP is enabled.
Bug: 63926541
Change-Id: I45dbada421b715abed9a66d03d30ae2285671ca1
Fixes: f2517eb76f ("android: binder: Add global lru shrinker to binder")
Reported-by: Kyle Yan <kyan@codeaurora.org>
Acked-by: Arve Hjønnevåg <arve@android.com>
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9939409/)
commit 372e3147df ("binder: guarantee txn complete / errors delivered
in-order") incorrectly defined a local ret value. This ret value will
be invalid when out of the if block
Change-Id: If7bd963ac7e67d135aa949133263aac27bf15d1a
Signed-off-by: Xu YiPing <xuyiping@hislicon.com>
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9939405/)
commit 7a4408c6bd ("binder: make sure accesses to proc/thread are
safe") made a change to enqueue tcomplete to thread->todo before
enqueuing the transaction. However, in err_dead_proc_or_thread case,
the tcomplete is directly freed, without dequeued. It may cause the
thread->todo list to be corrupted.
So, dequeue it before freeing.
Bug: 65333488
Change-Id: Id063a4db18deaa634f4d44aa6ebca47bea32537a
Signed-off-by: Xu YiPing <xuyiping@hisilicon.com>
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9928611/)
Add the number of active, lru, and free pages for
each binder process in binder stats
Bug: 63926541
Change-Id: I12618e4eb8ecc08f4f05fe4cba454a88830897f9
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9928613/)
Add tracepoints in binder transaction allocator to
record lru hits and alloc/free page.
Bug: 63926541
Change-Id: I2e24fe8e7b6534349df4a87ff865a6843ac9a30b
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9928615/)
Hold on to the pages allocated and mapped for transaction
buffers until the system is under memory pressure. When
that happens, use linux shrinker to free pages. Without
using shrinker, patch "android: binder: Move buffer out
of area shared with user space" will cause a significant
slow down for small transactions that fit into the first
page because free list buffer header used to be inlined
with buffer data.
In addition to prevent the performance regression for
small transactions, this patch improves the performance
for transactions that take up more than one page.
Modify alloc selftest to work with the shrinker change.
Test: Run memory intensive applications (Chrome and Camera)
to trigger shrinker callbacks. Binder frees memory as expected.
Test: Run binderThroughputTest with high memory pressure
option enabled.
Bug: 63926541
Change-Id: I3abfc43b405e7e0a6228da37e0689a4b944f0e00
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9928607/)
Binder driver allocates buffer meta data in a region that is mapped
in user space. These meta data contain pointers in the kernel.
This patch allocates buffer meta data on the kernel heap that is
not mapped in user space, and uses a pointer to refer to the data mapped.
Also move alloc->buffers initialization from mmap to init since it's
now used even when mmap failed or was not called.
Bug: 36007193
Change-Id: Id5136048bdb7b796f59de066de7ea7df410498f5
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9928609/)
binder_alloc_selftest tests that alloc_new_buf handles page allocation and
deallocation properly when allocate and free buffers. The test allocates 5
buffers of various sizes to cover all possible page alignment cases, and
frees the buffers using a list of exhaustive freeing order.
Test: boot the device with ANDROID_BINDER_IPC_SELFTEST config option
enabled. Allocator selftest passes.
Bug: 36007193
Change-Id: I2fe396232b7dfe4bbc50bdba99ca0de9be63cc37
Signed-off-by: Sherry Yang <sherryy@android.com>
(from https://patchwork.kernel.org/patch/9928605/)
Use helper functions buffer_next and buffer_prev instead
of list_entry to get the next and previous buffers.
Bug: 36007193
Change-Id: I422dce84afde3d2138a6d976593b109a9cc49003
Signed-off-by: Sherry Yang <sherryy@android.com>
This can cause issues with processes using the poll()
interface:
1) client sends two oneway transactions
2) the second one gets queued on async_todo
(because the server didn't handle the first one
yet)
3) server returns from poll(), picks up the
first transaction and does transaction work
4) server is done with the transaction, sends
BC_FREE_BUFFER, and the second transaction gets
moved to thread->todo
5) libbinder's handlePolledCommands() only handles
the commands in the current data buffer, so
doesn't see the new transaction
6) the server continues running and issues a new
outgoing transaction. Now, it suddenly finds
the incoming oneway transaction on its thread
todo, and returns that to userspace.
7) userspace does not expect this to happen; it
may be holding a lock while making the outgoing
transaction, and if handling the incoming
trasnaction requires taking the same lock,
userspace will deadlock.
By queueing the async transaction to the proc
workqueue, we make sure it's only picked up when
a thread is ready for proc work.
Bug: 38201220
Bug: 63075553
Bug: 63079216
Change-Id: I84268cc112f735d7e3173793873dfdb4b268468b
Signed-off-by: Martijn Coenen <maco@android.com>
Because we're not guaranteed that subsequent calls
to poll() will have a poll_table_struct parameter
with _qproc set. When _qproc is not set, poll_wait()
is a noop, and we won't be woken up correctly.
Bug: 64552728
Change-Id: I5b904c9886b6b0994d1631a636f5c5e5f6327950
Test: binderLibTest stops hanging with new test
Signed-off-by: Martijn Coenen <maco@android.com>
This allows userspace to request death notifications without
having to worry about getting an immediate callback on the same
thread; one scenario where this would be problematic is if the
death recipient handler grabs a lock that was already taken
earlier (eg as part of a nested transaction).
Bug: 23525545
Test: binderLibTest.DeathNotificationThread passes
Change-Id: I955e16306fe3110dacb9a391ffff1bf869249495
Signed-off-by: Martijn Coenen <maco@android.com>
Because is_spin_locked() always returns false on UP
systems.
Use assert_spin_locked() instead, and remove the
WARN_ON() instances, since those were easy to verify.
Bug: 64073116
Change-Id: I9080991c6d67e91928282a3ee64db23e50c7d66a
Signed-off-by: Martijn Coenen <maco@android.com>
Because we have disabled RT priority inheritance for
the regular binder domain, the following can happen:
1) thread A (prio 98) calls into thread B
2) because RT prio inheritance is disabled, thread B
runs at the lowest nice (prio 100) instead
3) thread B calls back into A; A will run at prio 100
for the duration of the transaction
4) When thread A is done with the call from B, we will
try to restore the prio back to 98. But, we fail
because the process doesn't hold CAP_SYS_NICE,
neither is RLIMIT_RT_PRIO set.
While the proper fix going forward will be to
correctly apply CAP_SYS_NICE or RLIMIT_RT_PRIO,
for now it seems reasonable to not check permissions
on the restore path.
Change-Id: Ibede5960c9b7bb786271c001e405de50be64d944
Signed-off-by: Martijn Coenen <maco@android.com>
The BINDER_GET_NODE_DEBUG_INFO ioctl will return debug info on
a node. Each successive call reusing the previous return value
will return the next node. The data will be used by
libmemunreachable to mark the pointers with kernel references
as reachable.
Bug: 28275695
Change-Id: Idbbafa648a33822dc023862cd92b51a595cf7c1c
Signed-off-by: Colin Cross <ccross@android.com>
Signed-off-by: Martijn Coenen <maco@android.com>
Allows a binder node to specify whether it wants to
inherit real-time scheduling policy from a caller.
Change-Id: I375b6094bf441c19f19cba06d5a6be02cd07d714
Signed-off-by: Martijn Coenen <maco@android.com>
By raising the priority of a thread selected for
a transaction *before* we wake it up.
Delay restoring the priority when doing a reply
until after we wake-up the process receiving
the reply.
Change-Id: Ic332e4e0ed7d2d3ca6ab1034da4629c9eadd3405
Signed-off-by: Martijn Coenen <maco@google.com>
This change adds flags to flat_binder_object.flags
to allow indicating a minimum scheduling policy for
the node. It also clarifies the valid value range
for the priority bits in the flags.
Internally, we use the priority map that the kernel
uses, e.g. [0..99] for real-time policies and [100..139]
for the SCHED_NORMAL/SCHED_BATCH policies.
Bug: 34461621
Bug: 37293077
Change-Id: I12438deecb53df432da18c6fc77460768ae726d2
Signed-off-by: Martijn Coenen <maco@google.com>
Instead of pushing new transactions to the process
waitqueue, select a thread that is waiting on proc
work to handle the transaction. This will make it
easier to improve priority inheritance in future
patches, by setting the priority before we wake up
a thread.
If we can't find a waiting thread, submit the work
to the proc waitqueue instead as we did previously.
Change-Id: I23cbfcca867bed7b86007e22137d0a8fad4b4001
Signed-off-by: Martijn Coenen <maco@google.com>
Removes the process waitqueue, so that threads
can only wait on the thread waitqueue. Whenever
there is process work to do, pick a thread and
wake it up.
This also fixes an issue with using epoll(),
since we no longer have to block on different
waitqueues.
Bug: 34461621
Change-Id: I2950b9de6fa078ee72d53c667a03cbaf587f0849
Signed-off-by: Martijn Coenen <maco@google.com>
(from https://patchwork.kernel.org/patch/9817765/)
A race existed where one thread could register
a death notification for a node, while another
thread was cleaning up that node and sending
out death notifications for its references,
causing simultaneous access to ref->death
because different locks were held.
Test: boots, manual testing
Change-Id: Iff73312f34f70374f417beba4c4c82dd33cac119
Signed-off-by: Martijn Coenen <maco@google.com>
(from https://patchwork.kernel.org/patch/9817761/)
When printing transactions there were several race conditions
that could cause a stale pointer to be deferenced. Fixed by
reading the pointer once and using it if valid (which is
safe). The transaction buffer also needed protection via proc
lock, so it is only printed if we are holding the correct lock.
Bug: 36650912
Test: tested manually
Change-Id: I78240f99cc1a070d70a841c0d84d4306e2fd528d
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817771/)
Use proc->outer_lock to protect the binder_ref structure.
The outer lock allows functions operating on the binder_ref
to do nested acquires of node and inner locks as necessary
to attach refs to nodes atomically.
Binder refs must never be accesssed without holding the
outer lock.
Change-Id: Icf6add0eddf70473b39239960b2d9a524775b53a
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817763/)
Use the inner lock to protect thread accounting fields in
proc structure: max_threads, requested_threads,
requested_threads_started and ready_threads.
Change-Id: I5a17eb68812702f803d4e2806e7887de0b3af18e
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817779/)
This makes future changes to priority inheritance
easier, since we want to be able to look at a thread's
transaction stack when selecting a thread to inherit
priority for.
It also allows us to take just a single lock in a
few paths, where we used to take two in succession.
Change-Id: Idb1b6e9faa5c669978b2b3011fe326be8aece586
Signed-off-by: Martijn Coenen <maco@google.com>
(from https://patchwork.kernel.org/patch/9817775/)
proc->threads will need to be accessed with higher
locks of other processes held so use proc->inner_lock
to protect it. proc->tmp_ref now needs to be protected
by proc->inner_lock.
Change-Id: I176cfeca16bf7c9b34b428c16405f93db81d2ff8
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817783/)
When locks for binder_ref handling are added, proc->nodes
will need to be modified while holding the outer lock
Change-Id: I17b39e981c55130c14a62fe49900eceff6e3642b
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817769/)
node->node_lock is used to protect elements of node. No
need to acquire for fields that are invariant: debug_id,
ptr, cookie.
Change-Id: Ib7738e52fa7689767f17136e18cc05ff548b5717
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817769/)
The todo lists in the proc, thread, and node structures
are accessed by other procs/threads to place work
items on the queue.
The todo lists are protected by the new proc->inner_lock.
No locks should ever be nested under these locks. As the
name suggests, an outer lock will be introduced in
a later patch.
Change-Id: I7720bacf5ebae4af177e22fcab0900d54c94c11a
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817789/)
For correct behavior we need to hold the inner lock when
dequeuing and processing node work in binder_thread_read.
We now hold the inner lock when we enter the switch statement
and release it after processing anything that might be
affected by other threads.
We also need to hold the inner lock to protect the node
weak/strong ref tracking fields as long as node->proc
is non-NULL (if it is NULL then we are guaranteed that
we don't have any node work queued).
This means that other functions that manipulate these fields
must hold the inner lock. Refactored these functions to use
the inner lock.
Change-Id: I02c5cfdd3ab6dadea7f07f2a275faf3e27be77ad
Test: tested manually
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817791/)
There are 3 main spinlocks which must be acquired in this
order:
1) proc->outer_lock : protects most fields of binder_proc,
binder_thread, and binder_ref structures. binder_proc_lock()
and binder_proc_unlock() are used to acq/rel.
2) node->lock : protects most fields of binder_node.
binder_node_lock() and binder_node_unlock() are
used to acq/rel
3) proc->inner_lock : protects the thread and node lists
(proc->threads, proc->nodes) and all todo lists associated
with the binder_proc (proc->todo, thread->todo,
proc->delivered_death and node->async_todo).
binder_inner_proc_lock() and binder_inner_proc_unlock()
are used to acq/rel
Any lock under procA must never be nested under any lock at the same
level or below on procB.
Functions that require a lock held on entry indicate which lock
in the suffix of the function name:
foo_olocked() : requires node->outer_lock
foo_nlocked() : requires node->lock
foo_ilocked() : requires proc->inner_lock
foo_iolocked(): requires proc->outer_lock and proc->inner_lock
foo_nilocked(): requires node->lock and proc->inner_lock
Change-Id: Ied42674486092a0e3bdde64356e45b2494844558
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817795/)
When obtaining a node via binder_get_node(),
binder_get_node_from_ref() or binder_new_node(),
increment node->tmp_refs to take a
temporary reference on the node to ensure the node
persists while being used. binder_put_node() must
be called to remove the temporary reference.
Change-Id: I962b39d5cd80b2d7e4786bb87236ede7914e2db7
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817781/)
Once locks are added, binder_ref's will only be accessed
safely with the proc lock held. Refactor the inc/dec paths
to make them atomic with the binder_get_ref* paths and
node inc/dec. For example, instead of:
ref = binder_get_ref(proc, handle, strong);
...
binder_dec_ref(ref, strong);
we now have:
ret = binder_dec_ref_for_handle(proc, handle, strong, &rdata);
Since the actual ref is no longer exposed to callers, a
new struct binder_ref_data is introduced which can be used
to return a copy of ref state.
Change-Id: I7de22107f8ebc967cee63251d584fceb4ea56250
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817787/)
binder_thread and binder_proc may be accessed by other
threads when processing transaction. Therefore they
must be prevented from being freed while a transaction
is in progress that references them.
This is done by introducing a temporary reference
counter for threads and procs that indicates that the
object is in use and must not be freed. binder_thread_dec_tmpref()
and binder_proc_dec_tmpref() are used to decrement
the temporary reference.
It is safe to free a binder_thread if there
is no reference and it has been released
(indicated by thread->is_dead).
It is safe to free a binder_proc if it has no
remaining threads and no reference.
A spinlock is added to the binder_transaction
to safely access and set references for t->from
and for debug code to safely access t->to_thread
and t->to_proc.
Change-Id: I0a00a0294c3e93aea8b3f141c6f18e77ad244078
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817787/)
When initiating a transaction, the target_node must
have a strong ref on it. Then we take a second
strong ref to make sure the node survives until the
transaction is complete.
Change-Id: If7429cb43eda520ab89d45df6c19327cee97c60c
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817805/)
Since errors are tracked in the return_error/return_error2
fields of the binder_thread object and BR_TRANSACTION_COMPLETEs
can be tracked either in those fields or via the thread todo
work list, it is possible for errors to be reported ahead
of the associated txn complete.
Use the thread todo work list for errors to guarantee
order. Also changed binder_send_failed_reply to pop
the transaction even if it failed to send a reply.
Bug: 37218618
Test: tested manually
Change-Id: I196cfaeed09fdcd697f8ab25eea4e04241fdb08f
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://lkml.org/lkml/2017/6/29/754)
binder_pop_transaction needs to be split into 2 pieces to
to allow the proc lock to be held on entry to dequeue the
transaction stack, but no lock when kfree'ing the transaction.
Split into binder_pop_transaction_locked and binder_free_transaction
(the actual locks are still to be added).
Change-Id: I848ae994cc27b3cd083cff2dbd1071762784f4a3
Test: tested manually
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817807/)
The log->next index for the transaction log was
not protected when incremented. This led to a
case where log->next++ resulted in an index
larger than ARRAY_SIZE(log->entry) and eventually
a bad access to memory.
Fixed by making the log index an atomic64 and
converting to an array by using "% ARRAY_SIZE(log->entry)"
Also added "complete" field to the log entry which is
written last to tell the print code whether the
entry is complete
Bug: 62038227
Test: tested manually
Change-Id: I1bb1c1a332a6ac458a626f5bedd05022b56b91f2
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817815/)
Adds protection against malicious user code freeing
the same buffer at the same time which could cause
a crash. Cannot happen under normal use.
Bug: 36650912
Change-Id: I43e078cbf31c0789aaff5ceaf8f1a94c75f79d45
Test: tested manually
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817819/)
node is always non-NULL in binder_get_ref_for_node so the
conditional and else clause are not needed
Change-Id: I23f011ba59e1869d9577e6bf28e1f1dd38f45713
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817799/)
The looper member of struct binder_thread is a bitmask
of control bits. All of the existing bits are modified
by the affected thread except for BINDER_LOOPER_STATE_NEED_RETURN
which can be modified in binder_deferred_flush() by
another thread.
To avoid adding a spinlock around all read-mod-writes to
modify a bit, the BINDER_LOOPER_STATE_NEED_RETURN flag
is replaced by a separate field in struct binder_thread.
Bug: 33250092 32225111
Change-Id: Ia4cefbdbd683c6cb17c323ba7d278de5f2ca0745
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817813/)
Currently, the transaction complete work item is queued
after the transaction. This means that it is possible
for the transaction to be handled and a reply to be
enqueued in the current thread before the transaction
complete is enqueued, which violates the protocol
with userspace who may not expect the transaction
complete. Fixed by always enqueing the transaction
complete first.
Also, once the transaction is enqueued, it is unsafe
to access since it might be freed. Currently,
t->flags is accessed to determine whether a sync
wake is needed. Changed to access tr->flags
instead.
Change-Id: I6c01566e167a39cf17c9027c3817618182e56975
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817757/)
In binder_thread_read, the BINDER_WORK_NODE command is used
to communicate the references on the node to userspace. It
can take a couple of iterations in the loop to construct
the list of commands for user space. When locking is added,
the lock would need to be release on each iteration which
means the state could change. The work item is not dequeued
during this process which prevents a simpler queue management
that can just dequeue up front and handle the work item.
Fixed by changing the BINDER_WORK_NODE algorithm in
binder_thread_read to determine which commands to send
to userspace atomically in 1 pass so it stays consistent
with the kernel view.
The work item is now dequeued immediately since only
1 pass is needed.
Change-Id: I9b4109997b2d53ba661867b14d7336cd076be06d
Signed-off-by: Todd Kjos <tkjos@google.com>
(from https://patchwork.kernel.org/patch/9817751/)
Add additional information to determine the cause of binder
failures. Adds the following to failed transaction log and
kernel messages:
return_error : value returned for transaction
return_error_param : errno returned by binder allocator
return_error_line : line number where error detected
Also, return BR_DEAD_REPLY if an allocation error indicates
a dead proc (-ESRCH)
Bug: 36406078
Change-Id: Ifc8881fa5adfcced3f2d67f9030fbd3efa3e2cab
Test: tested manually
Signed-off-by: Todd Kjos <tkjos@google.com>
