The check should be done against 'eproc' before it gets dereferenced.
Fixes: d49297739550 ("BACKPORT: binder: use euid from cred instead of using task")
Change-Id: Ief0c08212c4da8bdfdf628474de9dd30ee5a8db0
Signed-off-by: Carlos Llamas <cmllamas@google.com>
The patches to support binder's frozen notification feature break the
KMI. This change fixes such issues by (1) moving proc->delivered_freeze
into the existing proc_wrapper struction, (2) dropping the frozen stats
support and (3) amending the STG due to a harmless enum binder_work_type
addition.
These are the reported KMI issues fixed by this patch:
function symbol 'int __traceiter_binder_transaction_received(void*, struct binder_transaction*)' changed
CRC changed from 0x74e9c98b to 0xfe0f8640
type 'struct binder_proc' changed
byte size changed from 584 to 632
member 'struct list_head delivered_death' changed
offset changed by 256
member 'struct list_head delivered_freeze' was added
13 members ('u32 max_threads' .. 'u64 android_oem_data1') changed
offset changed by 384
type 'struct binder_thread' changed
byte size changed from 464 to 496
2 members ('atomic_t tmp_ref' .. 'bool is_dead') changed
offset changed by 224
4 members ('struct task_struct* task' .. 'enum binder_prio_state prio_state') changed
offset changed by 256
type 'struct binder_stats' changed
byte size changed from 216 to 244
member changed from 'atomic_t br[21]' to 'atomic_t br[23]'
type changed from 'atomic_t[21]' to 'atomic_t[23]'
number of elements changed from 21 to 23
member changed from 'atomic_t bc[19]' to 'atomic_t bc[22]'
offset changed from 672 to 736
type changed from 'atomic_t[19]' to 'atomic_t[22]'
number of elements changed from 19 to 22
member changed from 'atomic_t obj_created[7]' to 'atomic_t obj_created[8]'
offset changed from 1280 to 1440
type changed from 'atomic_t[7]' to 'atomic_t[8]'
number of elements changed from 7 to 8
member changed from 'atomic_t obj_deleted[7]' to 'atomic_t obj_deleted[8]'
offset changed from 1504 to 1696
type changed from 'atomic_t[7]' to 'atomic_t[8]'
number of elements changed from 7 to 8
type 'enum binder_work_type' changed
enumerator 'BINDER_WORK_FROZEN_BINDER' (10) was added
enumerator 'BINDER_WORK_CLEAR_FREEZE_NOTIFICATION' (11) was added
Bug: 363013421
Change-Id: If9f1f14a2eda215a4c9cb0823c50c8e0e8079ef1
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Frozen processes present a significant challenge in binder transactions.
When a process is frozen, it cannot, by design, accept and/or respond to
binder transactions. As a result, the sender needs to adjust its
behavior, such as postponing transactions until the peer process
unfreezes. However, there is currently no way to subscribe to these
state change events, making it impossible to implement frozen-aware
behaviors efficiently.
Introduce a binder API for subscribing to frozen state change events.
This allows programs to react to changes in peer process state,
mitigating issues related to binder transactions sent to frozen
processes.
Implementation details:
For a given binder_ref, the state of frozen notification can be one of
the followings:
1. Userspace doesn't want a notification. binder_ref->freeze is null.
2. Userspace wants a notification but none is in flight.
list_empty(&binder_ref->freeze->work.entry) = true
3. A notification is in flight and waiting to be read by userspace.
binder_ref_freeze.sent is false.
4. A notification was read by userspace and kernel is waiting for an ack.
binder_ref_freeze.sent is true.
When a notification is in flight, new state change events are coalesced into
the existing binder_ref_freeze struct. If userspace hasn't picked up the
notification yet, the driver simply rewrites the state. Otherwise, the
notification is flagged as requiring a resend, which will be performed
once userspace acks the original notification that's inflight.
See https://r.android.com/3070045 for how userspace is going to use this
feature.
Signed-off-by: Yu-Ting Tseng <yutingtseng@google.com>
Acked-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20240709070047.4055369-4-yutingtseng@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Bug: 363013421
(cherry picked from commit d579b04a52a183db47dfcb7a44304d7747d551e1
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
char-misc-next)
Change-Id: I5dd32abba932ca7d03ae58660143e075ed778b81
[cmllamas: fix merge conflicts due to missing 0567461a7a6e]
Signed-off-by: Carlos Llamas <cmllamas@google.com>
commit 4df153652cc46545722879415937582028c18af5 upstream.
Binder objects are processed and copied individually into the target
buffer during transactions. Any raw data in-between these objects is
copied as well. However, this raw data copy lacks an out-of-bounds
check. If the raw data exceeds the data section size then the copy
overwrites the offsets section. This eventually triggers an error that
attempts to unwind the processed objects. However, at this point the
offsets used to index these objects are now corrupted.
Unwinding with corrupted offsets can result in decrements of arbitrary
nodes and lead to their premature release. Other users of such nodes are
left with a dangling pointer triggering a use-after-free. This issue is
made evident by the following KASAN report (trimmed):
==================================================================
BUG: KASAN: slab-use-after-free in _raw_spin_lock+0xe4/0x19c
Write of size 4 at addr ffff47fc91598f04 by task binder-util/743
CPU: 9 UID: 0 PID: 743 Comm: binder-util Not tainted 6.11.0-rc4 #1
Hardware name: linux,dummy-virt (DT)
Call trace:
_raw_spin_lock+0xe4/0x19c
binder_free_buf+0x128/0x434
binder_thread_write+0x8a4/0x3260
binder_ioctl+0x18f0/0x258c
[...]
Allocated by task 743:
__kmalloc_cache_noprof+0x110/0x270
binder_new_node+0x50/0x700
binder_transaction+0x413c/0x6da8
binder_thread_write+0x978/0x3260
binder_ioctl+0x18f0/0x258c
[...]
Freed by task 745:
kfree+0xbc/0x208
binder_thread_read+0x1c5c/0x37d4
binder_ioctl+0x16d8/0x258c
[...]
==================================================================
To avoid this issue, let's check that the raw data copy is within the
boundaries of the data section.
Fixes: 6d98eb95b450 ("binder: avoid potential data leakage when copying txn")
Cc: Todd Kjos <tkjos@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20240822182353.2129600-1-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit 4df153652cc46545722879415937582028c18af5 upstream.
Binder objects are processed and copied individually into the target
buffer during transactions. Any raw data in-between these objects is
copied as well. However, this raw data copy lacks an out-of-bounds
check. If the raw data exceeds the data section size then the copy
overwrites the offsets section. This eventually triggers an error that
attempts to unwind the processed objects. However, at this point the
offsets used to index these objects are now corrupted.
Unwinding with corrupted offsets can result in decrements of arbitrary
nodes and lead to their premature release. Other users of such nodes are
left with a dangling pointer triggering a use-after-free. This issue is
made evident by the following KASAN report (trimmed):
==================================================================
BUG: KASAN: slab-use-after-free in _raw_spin_lock+0xe4/0x19c
Write of size 4 at addr ffff47fc91598f04 by task binder-util/743
CPU: 9 UID: 0 PID: 743 Comm: binder-util Not tainted 6.11.0-rc4 #1
Hardware name: linux,dummy-virt (DT)
Call trace:
_raw_spin_lock+0xe4/0x19c
binder_free_buf+0x128/0x434
binder_thread_write+0x8a4/0x3260
binder_ioctl+0x18f0/0x258c
[...]
Allocated by task 743:
__kmalloc_cache_noprof+0x110/0x270
binder_new_node+0x50/0x700
binder_transaction+0x413c/0x6da8
binder_thread_write+0x978/0x3260
binder_ioctl+0x18f0/0x258c
[...]
Freed by task 745:
kfree+0xbc/0x208
binder_thread_read+0x1c5c/0x37d4
binder_ioctl+0x16d8/0x258c
[...]
==================================================================
To avoid this issue, let's check that the raw data copy is within the
boundaries of the data section.
Fixes: 6d98eb95b450 ("binder: avoid potential data leakage when copying txn")
Cc: Todd Kjos <tkjos@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20240822182353.2129600-1-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit 31643d84b8c3d9c846aa0e20bc033e46c68c7e7d upstream.
With the introduction of binder_available_for_proc_work_ilocked() in
commit 1b77e9dcc3da ("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: 1b77e9dcc3da ("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>
commit aaef73821a3b0194a01bd23ca77774f704a04d40 upstream.
Commit 6d98eb95b450 ("binder: avoid potential data leakage when copying
txn") introduced changes to how binder objects are copied. In doing so,
it unintentionally removed an offset alignment check done through calls
to binder_alloc_copy_from_buffer() -> check_buffer().
These calls were replaced in binder_get_object() with copy_from_user(),
so now an explicit offset alignment check is needed here. This avoids
later complications when unwinding the objects gets harder.
It is worth noting this check existed prior to commit 7a67a39320df
("binder: add function to copy binder object from buffer"), likely
removed due to redundancy at the time.
Fixes: 6d98eb95b450 ("binder: avoid potential data leakage when copying txn")
Cc: stable@vger.kernel.org
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Acked-by: Todd Kjos <tkjos@google.com>
Link: https://lore.kernel.org/r/20240330190115.1877819-1-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit 97830f3c3088638ff90b20dfba2eb4d487bf14d7 upstream.
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: 457b9a6f09f0 ("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: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit 6ac061db9c58ca5b9270b1b3940d2464fb3ff183 upstream.
Use EPOLLERR instead of POLLERR to make sure it is cast to the correct
__poll_t type. This fixes the following sparse issue:
drivers/android/binder.c:5030:24: warning: incorrect type in return expression (different base types)
drivers/android/binder.c:5030:24: expected restricted __poll_t
drivers/android/binder.c:5030:24: got int
Fixes: f88982679f54 ("binder: check for binder_thread allocation failure in binder_poll()")
Cc: stable@vger.kernel.org
Cc: Eric Biggers <ebiggers@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20231201172212.1813387-2-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
According to Google we should set this to 0
as there is excessive logging in specific usecases
which has a negative impact on latency.
Signed-off-by: UtsavBalar1231 <utsavbalar1231@gmail.com>
Change-Id: Id619335848802e9d9a9bc13100d09a2cadbab07a
