commit 31643d84b8c3d9c846aa0e20bc033e46c68c7e7d upstream.
With the introduction of binder_available_for_proc_work_ilocked() in
commit 1b77e9dcc3 ("ANDROID: binder: remove proc waitqueue") a binder
thread can only "wait_for_proc_work" after its thread->looper has been
marked as BINDER_LOOPER_STATE_{ENTERED|REGISTERED}.
This means an unregistered reader risks waiting indefinitely for work
since it never gets added to the proc->waiting_threads. If there are no
further references to its waitqueue either the task will hang. The same
applies to readers using the (e)poll interface.
I couldn't find the rationale behind this restriction. So this patch
restores the previous behavior of allowing unregistered threads to
"wait_for_proc_work". Note that an error message for this scenario,
which had previously become unreachable, is now re-enabled.
Fixes: 1b77e9dcc3 ("ANDROID: binder: remove proc waitqueue")
Cc: stable@vger.kernel.org
Cc: Martijn Coenen <maco@google.com>
Cc: Arve Hjønnevåg <arve@google.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20240711201452.2017543-1-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 229670361c29381b0e1677763590e4dbc209ecbe)
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
The OOM reaper makes it possible to immediately release anonymous memory
from a dying process in order to free up memory faster. This provides
immediate relief under heavy memory pressure instead of waiting for victim
processes to naturally release their memory.
Utilize the OOM reaper by creating another kthread in Simple LMK to perform
victim reaping. Similar to the OOM reaper kthread (which is unused with
Simple LMK), this new kthread allows reaping to race with exit_mmap() in
order to preclude the need to take a reference to an mm's address space and
thus potentially mmput() an mm's last reference. Doing so would stall the
reaper kthread, preventing it from being able to quickly reap new victims.
Reaping is done on victims one at a time by descending order of anonymous
pages, so that the most promising victims with the most anonymous pages
are reaped first. Victims are also marked for reaping via MMF_OOM_VICTIM so
that they reap themselves first in exit_mmap(). Even if a victim isn't
reaped by the reaper thread, it'll free its anonymous memory first thing in
exit_mmap() as a small win towards making memory available sooner.
By relieving memory pressure faster via reaping, Simple LMK not only
doesn't need to kill as many processes, but also improves system
responsiveness when memory is low since memory pressure is relieved sooner.
Although not strictly required, Simple LMK should be the only one utilizing
the OOM reaper. Any other code that may utilize the OOM reaper, such as
patches that invoke the OOM reaper for all SIGKILLs, should be disabled.
Signed-off-by: Sultan Alsawaf <sultan@kerneltoast.com>
We can check if the waitqueue is actually active before calling wake_up()
in order to avoid an unnecessary wake_up() if the reclaim thread is already
running. Furthermore, the release barrier when zeroing needs_reclaim is
unnecessary, so remove it.
Signed-off-by: Sultan Alsawaf <sultan@kerneltoast.com>
As it turns out, victim scheduling priority elevation has always been
broken for two reasons:
1. The minimum valid RT priority is 1, not 0. As a result,
sched_setscheduler_nocheck() always fails with -EINVAL.
2. The thread within a victim thread group which happens to hold the mm is
not necessarily the only thread with references to the mm, and isn't
necessarily the thread which will release the final mm reference. As a
result, victim threads which hold mm references may take a while to
release them, and the unlucky thread which puts the final mm reference
may take a very long time to release all memory if it doesn't have RT
scheduling priority.
These issues cause victims to often take a very long time to release their
memory, possibly up to several seconds depending on system load. This, in
turn, causes Simple LMK to constantly hit the reclaim timeout and kill more
processes, with Simple LMK being rather ineffective since victims may not
release any memory for several seconds.
Fix the broken scheduling priority elevation by changing the RT priority to
the valid lowest priority of 1 and applying it to all threads in the thread
group, instead of just the thread which holds the mm.
This reverts commit 28bb8017e8b5a42f9b10fbf01aaf4de83dd3c38f.
Dying victims at max prio will starve other threads and make the system
unresponsive, so revert it.
Signed-off-by: Kazuki Hashimoto <kazukih@tuta.io>
- These values are adequate for 4GB and 6GB ram variants.
- Also grab conditional check fix from [1].
[1]: 31428c7c88
Signed-off-by: Cyber Knight <cyberknight755@gmail.com>
commit 42316941335644a98335f209daafa4c122f28983 upstream.
The type defined for the BINDER_SET_MAX_THREADS ioctl was changed from
size_t to __u32 in order to avoid incompatibility issues between 32 and
64-bit kernels. However, the internal types used to copy from user and
store the value were never updated. Use u32 to fix the inconsistency.
Fixes: a9350fc859 ("staging: android: binder: fix BINDER_SET_MAX_THREADS declaration")
Reported-by: Arve Hjønnevåg <arve@android.com>
Cc: stable@vger.kernel.org
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20240421173750.3117808-1-cmllamas@google.com
[cmllamas: resolve minor conflicts due to missing commit 421518a2740f]
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit c437184be3c16f7123e5c59b4c85c1101f4dc96b)
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
[ Upstream commit 97830f3c3088638ff90b20dfba2eb4d487bf14d7 ]
In (e)poll mode, threads often depend on I/O events to determine when
data is ready for consumption. Within binder, a thread may initiate a
command via BINDER_WRITE_READ without a read buffer and then make use
of epoll_wait() or similar to consume any responses afterwards.
It is then crucial that epoll threads are signaled via wakeup when they
queue their own work. Otherwise, they risk waiting indefinitely for an
event leaving their work unhandled. What is worse, subsequent commands
won't trigger a wakeup either as the thread has pending work.
Fixes: 457b9a6f09 ("Staging: android: add binder driver")
Cc: Arve Hjønnevåg <arve@android.com>
Cc: Martijn Coenen <maco@android.com>
Cc: Alice Ryhl <aliceryhl@google.com>
Cc: Steven Moreland <smoreland@google.com>
Cc: stable@vger.kernel.org # v4.19+
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20240131215347.1808751-1-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
[ Upstream commit 148ade2c4d4f46b3ecc1ddad1c762371e8708e35 ]
This flag determines whether the thread should currently
process the work in the thread->todo worklist.
The prime usecase for this is improving the performance
of synchronous transactions: all synchronous transactions
post a BR_TRANSACTION_COMPLETE to the calling thread,
but there's no reason to return that command to userspace
right away - userspace anyway needs to wait for the reply.
Likewise, a synchronous transaction that contains a binder
object can cause a BC_ACQUIRE/BC_INCREFS to be returned to
userspace; since the caller must anyway hold a strong/weak
ref for the duration of the call, postponing these commands
until the reply comes in is not a problem.
Note that this flag is not used to determine whether a
thread can handle process work; a thread should never pick
up process work when thread work is still pending.
Before patch:
------------------------------------------------------------------
Benchmark Time CPU Iterations
------------------------------------------------------------------
BM_sendVec_binderize/4 45959 ns 20288 ns 34351
BM_sendVec_binderize/8 45603 ns 20080 ns 34909
BM_sendVec_binderize/16 45528 ns 20113 ns 34863
BM_sendVec_binderize/32 45551 ns 20122 ns 34881
BM_sendVec_binderize/64 45701 ns 20183 ns 34864
BM_sendVec_binderize/128 45824 ns 20250 ns 34576
BM_sendVec_binderize/256 45695 ns 20171 ns 34759
BM_sendVec_binderize/512 45743 ns 20211 ns 34489
BM_sendVec_binderize/1024 46169 ns 20430 ns 34081
After patch:
------------------------------------------------------------------
Benchmark Time CPU Iterations
------------------------------------------------------------------
BM_sendVec_binderize/4 42939 ns 17262 ns 40653
BM_sendVec_binderize/8 42823 ns 17243 ns 40671
BM_sendVec_binderize/16 42898 ns 17243 ns 40594
BM_sendVec_binderize/32 42838 ns 17267 ns 40527
BM_sendVec_binderize/64 42854 ns 17249 ns 40379
BM_sendVec_binderize/128 42881 ns 17288 ns 40427
BM_sendVec_binderize/256 42917 ns 17297 ns 40429
BM_sendVec_binderize/512 43184 ns 17395 ns 40411
BM_sendVec_binderize/1024 43119 ns 17357 ns 40432
Signed-off-by: Martijn Coenen <maco@android.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Stable-dep-of: 97830f3c3088 ("binder: signal epoll threads of self-work")
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
commit c6d05e0762ab276102246d24affd1e116a46aa0c upstream.
Each transaction is associated with a 'struct binder_buffer' that stores
the metadata about its buffer area. Since commit 74310e06be ("android:
binder: Move buffer out of area shared with user space") this struct is
no longer embedded within the buffer itself but is instead allocated on
the heap to prevent userspace access to this driver-exclusive info.
Unfortunately, the space of this struct is still being accounted for in
the total buffer size calculation, specifically for async transactions.
This results in an additional 104 bytes added to every async buffer
request, and this area is never used.
This wasted space can be substantial. If we consider the maximum mmap
buffer space of SZ_4M, the driver will reserve half of it for async
transactions, or 0x200000. This area should, in theory, accommodate up
to 262,144 buffers of the minimum 8-byte size. However, after adding
the extra 'sizeof(struct binder_buffer)', the total number of buffers
drops to only 18,724, which is a sad 7.14% of the actual capacity.
This patch fixes the buffer size calculation to enable the utilization
of the entire async buffer space. This is expected to reduce the number
of -ENOSPC errors that are seen on the field.
Fixes: 74310e06be ("android: binder: Move buffer out of area shared with user space")
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-6-cmllamas@google.com
[cmllamas: fix trivial conflict with missing 261e7818f06e.]
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit e2425a67b5ed67496959d0dfb99816f5757164b0)
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
commit 9a9ab0d963621d9d12199df9817e66982582d5a5 upstream.
Task A calls binder_update_page_range() to allocate and insert pages on
a remote address space from Task B. For this, Task A pins the remote mm
via mmget_not_zero() first. This can race with Task B do_exit() and the
final mmput() refcount decrement will come from Task A.
Task A | Task B
------------------+------------------
mmget_not_zero() |
| do_exit()
| exit_mm()
| mmput()
mmput() |
exit_mmap() |
remove_vma() |
fput() |
In this case, the work of ____fput() from Task B is queued up in Task A
as TWA_RESUME. So in theory, Task A returns to userspace and the cleanup
work gets executed. However, Task A instead sleep, waiting for a reply
from Task B that never comes (it's dead).
This means the binder_deferred_release() is blocked until an unrelated
binder event forces Task A to go back to userspace. All the associated
death notifications will also be delayed until then.
In order to fix this use mmput_async() that will schedule the work in
the corresponding mm->async_put_work WQ instead of Task A.
Fixes: 457b9a6f09 ("Staging: android: add binder driver")
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-4-cmllamas@google.com
[cmllamas: fix trivial conflict with missing d8ed45c5dcd4.]
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 95b1d336b0642198b56836b89908d07b9a0c9608)
[fix conflict due to missing commit 720c241924046aff83f5f2323232f34a30a4c281
("ANDROID: binder: change down_write to down_read")]
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
Among the kmem caches, the duplicate ones get merged with the system kmem
caches that cover basically every allocation size <64K which is a power of
2. In such a case, use of kmem cache is useless. Remove them.
Suggested-by: Sultan Alsawaf <sultan@kerneltoast.com>
Signed-off-by: Tashfin Shakeer Rhythm <tashfinshakeerrhythm@gmail.com>
Revert "kgsl: move worker thread to hp mask"
This reverts commit 9ede9a602a214b9d11b30efb7119fa29176a39a0.
Revert "kernel: irq: manage: use a different way of affining perf IRQs"
This reverts commit 7b6ed81bb5.
Revert "kernel: irq: properly disallow userspace from changing IRQs affinity"
This reverts commit 6192075134.
Revert "kernel: Add a mixed big and prime cluster mode to the perf-critical API."
This reverts commit e8b11fd3fa.
Revert "kernel: Split PF_PERF_CRITICAL into 6 standalone flags"
This reverts commit 514e32786f.
Revert "drm: Affine IRQ to the prime CPU cluster"
This reverts commit a2e2127328b5fae0e306ba43a9509d4efdd25166.
Revert "simple_lmk: Run reclaim kthread on big CPU cluster"
This reverts commit 5cc18c8f61.
Revert "msm_thermal_simple: update for prime affining"
This reverts commit f6250a3c23.
Revert "devfreq_boost: Run boost kthreads on big CPU cluster"
This reverts commit 4476a2602b.
Revert "rcu: Run nocb kthreads on little CPUs"
This reverts commit f1dfdd3bff.
Revert "kernel: Warn when an IRQ's affinity notifier gets overwritten"
This reverts commit e48923d05f.
Revert "kernel: Don't allow IRQ affinity masks to have more than one CPU"
This reverts commit c6074310eb.
Revert "kernel: Extend the perf-critical API to little CPUs"
This reverts commit 57fb9de6b1b3efd9d809cd3b000841cbacd4a82a.
Revert "kernel: Add tri-cluster API to affine IRQs and kthreads to fast CPUs"
This reverts commit f8e46638b4b2752ce15dad1e8f18b90f58d476a1.
Revert "cpumask: Add cpu_hp_mask which contains both the big and prime clusters."
This reverts commit cd8a444526.
Revert "cpumask: Add cpu_hp_mask which contains both the big and prime clusters."
This reverts commit 486f73aa6a34cb24f2844cc377595c77d13bf285.
Revert "[ADAPTED] cpumask: Add cpumasks for first and second Big CPUs on Big cluster"
This reverts commit 216a7226495b68061bafb49a9aad762f5848e7f5.
Revert "input: fingerprint: goodix_ta: Affine IRQ to the big CPU cluster"
This reverts commit 1004d2079b.
treewide: Remove affine
Revert "msm: kgsl: Affine kgsl_3d0_irq and worker kthread to the big CPU cluster"
This reverts commit e8a2c62980.
Most of binder's memory allocations are tiny, and they're allocated
and freed extremely frequently. The latency from going through the page
allocator all the time for such small allocations ends up being quite
high, especially when the system is low on memory. Binder is
performance-critical, so this is suboptimal.
Instead of using kzalloc to allocate a struct every time, reserve caches
specifically for allocating each struct quickly.
Signed-off-by: Sultan Alsawaf <sultan@kerneltoast.com>
Signed-off-by: Danny Lin <danny@kdrag0n.dev>
[ Tashar02: Fix compilation on k5.10 binder ]
Co-authored-by: Tashfin Shakeer Rhythm <tashfinshakeerrhythm@gmail.com>
Signed-off-by: Tashfin Shakeer Rhythm <tashfinshakeerrhythm@gmail.com>
When calling binder_do_set_priority() with the same policy and priority
values as the current task, we exit early since there is nothing to do.
However, the BINDER_PRIO_PENDING state might be set and in this case we
fail to update it. A subsequent call to binder_transaction_priority()
will then read an incorrect state and save the wrong priority. Fix this
by setting thread->prio_state to BINDER_PRIO_SET on our way out.
Bug: 199309216
Fixes: cac827f2619b ("ANDROID: binder: fix race in priority restore")
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Change-Id: I21e906cf4b2ebee908af41fe101ecd458ae1991c
(cherry picked from commit 72193be6d4bd9ad29dacd998c14dff97f7a6c6c9)
When the target process is busy, incoming oneway transactions are
queued in the async_todo list. If the clients continue sending extra
oneway transactions while the target process is frozen, this queue can
become too large to accommodate new transactions. That's why binder
driver introduced ONEWAY_SPAM_DETECTION to detect this situation. It's
helpful to debug the async binder buffer exhausting issue, but the
issue itself isn't solved directly.
In real cases applications are designed to send oneway transactions
repeatedly, delivering updated inforamtion to the target process.
Typical examples are Wi-Fi signal strength and some real time sensor
data. Even if the apps might only care about the lastet information,
all outdated oneway transactions are still accumulated there until the
frozen process is thawed later. For this kind of situations, there's
no existing method to skip those outdated transactions and deliver the
latest one only.
This patch introduces a new transaction flag TF_UPDATE_TXN. To use it,
use apps can set this new flag along with TF_ONE_WAY. When such an
oneway transaction is to be queued into the async_todo list of a frozen
process, binder driver will check if any previous pending transactions
can be superseded by comparing their code, flags and target node. If
such an outdated pending transaction is found, the latest transaction
will supersede that outdated one. This effectively prevents the async
binder buffer running out and saves unnecessary binder read workloads.
Acked-by: Todd Kjos <tkjos@google.com>
Signed-off-by: Li Li <dualli@google.com>
Link: https://lore.kernel.org/r/20220526220018.3334775-2-dualli@chromium.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 231624308
Test: manually check async binder buffer size of frozen apps
Test: stress test with kernel 4.14/4.19/5.10/5.15
(cherry picked from commit 9864bb4801331daa48514face9d0f4861e4d485b
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
char-misc-next)
Change-Id: I1c4bff1eda1ca15aaaad5bf696c8fc00be743176
During a reply, the target gets woken up and then the priority of the
replier is restored. The order is such to allow the target to process
the reply ASAP. Otherwise, we risk the sender getting scheduled out
before the wakeup happens. This strategy reduces transaction latency.
However, a subsequent transaction from the same target could be started
before the priority of the replier gets restored. At this point we save
the wrong priority and it gets reinstated at the end of the transaction.
This patch allows the incoming transaction to detect the race condition
and save the correct next priority. Additionally, the replier will abort
its pending priority restore which allows the new transaction to always
run at the desired priority.
Bug: 148101660
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Change-Id: I6fec41ae1a1342023f78212ab1f984e26f068221
(cherry picked from commit cac827f2619b280d418e546a09f25da600dafe5a)
[cmllamas: fixed trivial merge conflict]
Signed-off-by: Andrzej Perczak <linux@andrzejperczak.com>
Refactor binder priority functions to take in 'struct binder_thread *'
instead of just 'struct task_struct *'. This allows access to other
thread fields used in subsequent patches. In any case, the same task
reference is still available under thread->task.
There is no functional impact from this patch.
Bug: 148101660
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Change-Id: I67b599884580d957d776500e467827e5035c99f6
(cherry picked from commit 759d98484b5b51932d3d11651fa83c6bb268ce03)
Signed-off-by: Andrzej Perczak <linux@andrzejperczak.com>
Avoid making unnecessary stack copies of struct binder_priority and pass
the argument by reference instead. Rename 'desired_prio' to 'desired' to
match the usage in other priority functions.
There is no functional impact from this patch.
Bug: 148101660
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Change-Id: I66ff5305296e7b9dba56ed265236f2af518f66e0
(cherry picked from commit 52d85f8a16467ce0bca374f885de24918f017371)
[cmllamas: fixed conflict with vendor hook patch]
Signed-off-by: Andrzej Perczak <linux@andrzejperczak.com>
The setup of node_prio is always the same, so just fold this logic into
binder_transaction_priority() to avoid duplication. Let's pass the node
reference instead, which also gives access to node->inherit_rt.
There is no functional impact from this patch.
Bug: 148101660
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Change-Id: Ib390204556e69c4bc8492cd9cd873773f9cdce42
(cherry picked from commit 498bf715b77c68e54d0289fa66e3f112278f87dc)
[cmllamas: fixed conflict with vendor hook patch]
Signed-off-by: Andrzej Perczak <linux@andrzejperczak.com>
This reverts commit f0416df755b7a52adebea9c4714934a8bf084e89.
Reason for revert: This was a "temporary" reversion to workaround what is believed to be a user-space issue.
Change-Id: I5322aecfe57cd8237e6657525eb33975c4840059
Bug: 166779391
Signed-off-by: Todd Kjos <tkjos@google.com>
(cherry picked from commit d1c6df6dc86a04a3cabc6b5e2fc01198bcf0a29d)
[cmllamas: Resolved merge conflict with vendor hook in binder.c]
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Signed-off-by: Andrzej Perczak <linux@andrzejperczak.com>
commit a0e44c64b6061dda7e00b7c458e4523e2331b739 upstream.
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: Christian Brauner (Microsoft) <brauner@kernel.org>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Cc: stable <stable@kernel.org> # 4.14+
Link: https://lore.kernel.org/r/20220801182511.3371447-1-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[ Upstream commit fe6b1869243f23a485a106c214bcfdc7aa0ed593 ]
If a memory copy function fails to copy the whole buffer,
a positive integar with the remaining bytes is returned.
In binder_translate_fd_array() this can result in an fd being
skipped due to the failed copy, but the loop continues
processing fds since the early return condition expects a
negative integer on error.
Fix by returning "ret > 0 ? -EINVAL : ret" to handle this case.
Fixes: bb4a2e48d510 ("binder: return errors from buffer copy functions")
Suggested-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Todd Kjos <tkjos@google.com>
Link: https://lore.kernel.org/r/20211130185152.437403-2-tkjos@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Andrzej Perczak <linux@andrzejperczak.com>
Every binder operation is being logged. This impacts performance and
increases memory footprint. Since binder is critical for Android
operation, doing any logging on production builds isn't best idea.
Quick grep over Android sources revealed that only lshal and dumpsys
binaries use binder_log directory, these are not critical so breaking
them won't hurt much. Anyways, I was able to succesfully run both and
bacis functionality was still there.
Benchmarks showed significant decrease in transaction time with an avg
of 1000ns.
Also, change all DEBUG ifdefs to new define.
Signed-off-by: Andrzej Perczak <linux@andrzejperczak.com>
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: 44d8047 ("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: Andrzej Perczak <linux@andrzejperczak.com>
