binder: fix offset calculation in debug log

The vma start address should be substracted from the buffer's user data
address and not the other way around.

Cc: Tiffany Y. Yang <[email protected]>
Cc

binder: fix offset calculation in debug log

The vma start address should be substracted from the buffer's user data
address and not the other way around.

Cc: Tiffany Y. Yang <[email protected]>
Cc: stable <[email protected]>
Fixes: 162c79731448 ("binder: avoid user addresses in debug logs")
Signed-off-by: Carlos Llamas <[email protected]>
Reviewed-by: Tiffany Y. Yang <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: log transaction code on failure

When a transaction fails, log the 'tr->code' to help indentify the
problematic userspace call path. This additional information will
simplify debugging effort

binder: log transaction code on failure

When a transaction fails, log the 'tr->code' to help indentify the
problematic userspace call path. This additional information will
simplify debugging efforts.

Cc: Steven Moreland <[email protected]>
Signed-off-by: Carlos Llamas <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: fix kernel-doc warning of 'file' member

The 'struct file' member in 'binder_task_work_cb' definition was renamed
to 'file' between patch versions but its kernel-doc reference kept the
old na

binder: fix kernel-doc warning of 'file' member

The 'struct file' member in 'binder_task_work_cb' definition was renamed
to 'file' between patch versions but its kernel-doc reference kept the
old name 'fd'. Update the naming to fix the W=1 build warning.

Cc: Todd Kjos <[email protected]>
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Signed-off-by: Carlos Llamas <[email protected]>
Acked-by: Todd Kjos <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binderfs: add new binder devices to binder_devices

When binderfs is not enabled, the binder driver parses the kernel
config to create all binder devices. All of the new binder devices
are stored in

binderfs: add new binder devices to binder_devices

When binderfs is not enabled, the binder driver parses the kernel
config to create all binder devices. All of the new binder devices
are stored in the list binder_devices.

When binderfs is enabled, the binder driver creates new binder devices
dynamically when userspace applications call BINDER_CTL_ADD ioctl. But
the devices created in this way are not stored in the same list.

This patch fixes that.

Signed-off-by: Li Li <[email protected]>
Acked-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: rename alloc->buffer to vm_start

The alloc->buffer field in struct binder_alloc stores the starting
address of the mapped vma, rename this field to alloc->vm_start to
better reflect its purp

binder: rename alloc->buffer to vm_start

The alloc->buffer field in struct binder_alloc stores the starting
address of the mapped vma, rename this field to alloc->vm_start to
better reflect its purpose. It also avoids confusion with the binder
buffer concept, e.g. transaction->buffer.

No functional changes in this patch.

Reviewed-by: Suren Baghdasaryan <[email protected]>
Signed-off-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: initialize lsm_context structure

It is possible to reach the end of binder_transaction() without
having set lsmctx. As the variable value is checked there it needs
to be initialized.

Sugges

binder: initialize lsm_context structure

It is possible to reach the end of binder_transaction() without
having set lsmctx. As the variable value is checked there it needs
to be initialized.

Suggested-by: Kees Bakker <[email protected]>
[PM: subj tweak to fit convention]
Signed-off-by: Casey Schaufler <[email protected]>
Signed-off-by: Paul Moore <[email protected]>

show more ...

lsm: replace context+len with lsm_context

Replace the (secctx,seclen) pointer pair with a single
lsm_context pointer to allow return of the LSM identifier
along with the context and context length.

lsm: replace context+len with lsm_context

Replace the (secctx,seclen) pointer pair with a single
lsm_context pointer to allow return of the LSM identifier
along with the context and context length. This allows
security_release_secctx() to know how to release the
context. Callers have been modified to use or save the
returned data from the new structure.

security_secid_to_secctx() and security_lsmproc_to_secctx()
will now return the length value on success instead of 0.

Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Todd Kjos <[email protected]>
Signed-off-by: Casey Schaufler <[email protected]>
[PM: subject tweak, kdoc fix, signedness fix from Dan Carpenter]
Signed-off-by: Paul Moore <[email protected]>

show more ...

lsm: ensure the correct LSM context releaser

Add a new lsm_context data structure to hold all the information about a
"security context", including the string, its size and which LSM allocated
the s

lsm: ensure the correct LSM context releaser

Add a new lsm_context data structure to hold all the information about a
"security context", including the string, its size and which LSM allocated
the string. The allocation information is necessary because LSMs have
different policies regarding the lifecycle of these strings. SELinux
allocates and destroys them on each use, whereas Smack provides a pointer
to an entry in a list that never goes away.

Update security_release_secctx() to use the lsm_context instead of a
(char *, len) pair. Change its callers to do likewise. The LSMs
supporting this hook have had comments added to remind the developer
that there is more work to be done.

The BPF security module provides all LSM hooks. While there has yet to
be a known instance of a BPF configuration that uses security contexts,
the possibility is real. In the existing implementation there is
potential for multiple frees in that case.

Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
To: Pablo Neira Ayuso <[email protected]>
Cc: [email protected]
Cc: Todd Kjos <[email protected]>
Signed-off-by: Casey Schaufler <[email protected]>
[PM: subject tweak]
Signed-off-by: Paul Moore <[email protected]>

show more ...

binder: add delivered_freeze to debugfs output

Add the pending proc->delivered_freeze work to the debugfs output. This
information was omitted in the original implementation of the freeze
notificati

binder: add delivered_freeze to debugfs output

Add the pending proc->delivered_freeze work to the debugfs output. This
information was omitted in the original implementation of the freeze
notification and can be valuable for debugging issues.

Fixes: d579b04a52a1 ("binder: frozen notification")
Cc: [email protected]
Signed-off-by: Carlos Llamas <[email protected]>
Acked-by: Todd Kjos <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: fix memleak of proc->delivered_freeze

If a freeze notification is cleared with BC_CLEAR_FREEZE_NOTIFICATION
before calling binder_freeze_notification_done(), then it is detached
from its ref

binder: fix memleak of proc->delivered_freeze

If a freeze notification is cleared with BC_CLEAR_FREEZE_NOTIFICATION
before calling binder_freeze_notification_done(), then it is detached
from its reference (e.g. ref->freeze) but the work remains queued in
proc->delivered_freeze. This leads to a memory leak when the process
exits as any pending entries in proc->delivered_freeze are not freed:

unreferenced object 0xffff38e8cfa36180 (size 64):
comm "binder-util", pid 655, jiffies 4294936641
hex dump (first 32 bytes):
b8 e9 9e c8 e8 38 ff ff b8 e9 9e c8 e8 38 ff ff .....8.......8..
0b 00 00 00 00 00 00 00 3c 1f 4b 00 00 00 00 00 ........<.K.....
backtrace (crc 95983b32):
[<000000000d0582cf>] kmemleak_alloc+0x34/0x40
[<000000009c99a513>] __kmalloc_cache_noprof+0x208/0x280
[<00000000313b1704>] binder_thread_write+0xdec/0x439c
[<000000000cbd33bb>] binder_ioctl+0x1b68/0x22cc
[<000000002bbedeeb>] __arm64_sys_ioctl+0x124/0x190
[<00000000b439adee>] invoke_syscall+0x6c/0x254
[<00000000173558fc>] el0_svc_common.constprop.0+0xac/0x230
[<0000000084f72311>] do_el0_svc+0x40/0x58
[<000000008b872457>] el0_svc+0x38/0x78
[<00000000ee778653>] el0t_64_sync_handler+0x120/0x12c
[<00000000a8ec61bf>] el0t_64_sync+0x190/0x194

This patch fixes the leak by ensuring that any pending entries in
proc->delivered_freeze are freed during binder_deferred_release().

Fixes: d579b04a52a1 ("binder: frozen notification")
Cc: [email protected]
Signed-off-by: Carlos Llamas <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Acked-by: Todd Kjos <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: allow freeze notification for dead nodes

Alice points out that binder_request_freeze_notification() should not
return EINVAL when the relevant node is dead [1]. The node can die at
any point

binder: allow freeze notification for dead nodes

Alice points out that binder_request_freeze_notification() should not
return EINVAL when the relevant node is dead [1]. The node can die at
any point even if the user input is valid. Instead, allow the request
to be allocated but skip the initial notification for dead nodes. This
avoids propagating unnecessary errors back to userspace.

Fixes: d579b04a52a1 ("binder: frozen notification")
Cc: [email protected]
Suggested-by: Alice Ryhl <[email protected]>
Link: https://lore.kernel.org/all/CAH5fLghapZJ4PbbkC8V5A6Zay-_sgTzwVpwqk6RWWUNKKyJC_Q@mail.gmail.com/ [1]
Signed-off-by: Carlos Llamas <[email protected]>
Acked-by: Todd Kjos <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: fix BINDER_WORK_CLEAR_FREEZE_NOTIFICATION debug logs

proc 699
context binder-test
thread 699: l 00 need_return 0 tr 0
ref 25: desc 1 node 20 s 1 w 0 d 00000000c03e09a3
unknown work: ty

binder: fix BINDER_WORK_CLEAR_FREEZE_NOTIFICATION debug logs

proc 699
context binder-test
thread 699: l 00 need_return 0 tr 0
ref 25: desc 1 node 20 s 1 w 0 d 00000000c03e09a3
unknown work: type 11

proc 640
context binder-test
thread 640: l 00 need_return 0 tr 0
ref 8: desc 1 node 3 s 1 w 0 d 000000002bb493e1
has cleared freeze notification

Fixes: d579b04a52a1 ("binder: frozen notification")
Cc: [email protected]
Suggested-by: Alice Ryhl <[email protected]>
Signed-off-by: Carlos Llamas <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Acked-by: Todd Kjos <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: fix BINDER_WORK_FROZEN_BINDER debug logs

The BINDER_WORK_FROZEN_BINDER type is not handled in the binder_logs
entries and it shows up as "unknown work" when logged:

proc 649
context bin

binder: fix BINDER_WORK_FROZEN_BINDER debug logs

The BINDER_WORK_FROZEN_BINDER type is not handled in the binder_logs
entries and it shows up as "unknown work" when logged:

proc 649
context binder-test
thread 649: l 00 need_return 0 tr 0
ref 13: desc 1 node 8 s 1 w 0 d 0000000053c4c0c3
unknown work: type 10

This patch add the freeze work type and is now logged as such:

proc 637
context binder-test
thread 637: l 00 need_return 0 tr 0
ref 8: desc 1 node 3 s 1 w 0 d 00000000dc39e9c6
has frozen binder

Fixes: d579b04a52a1 ("binder: frozen notification")
Cc: [email protected]
Acked-by: Todd Kjos <[email protected]>
Signed-off-by: Carlos Llamas <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: fix freeze UAF in binder_release_work()

When a binder reference is cleaned up, any freeze work queued in the
associated process should also be removed. Otherwise, the reference is
freed whil

binder: fix freeze UAF in binder_release_work()

When a binder reference is cleaned up, any freeze work queued in the
associated process should also be removed. Otherwise, the reference is
freed while its ref->freeze.work is still queued in proc->work leading
to a use-after-free issue as shown by the following KASAN report:

==================================================================
BUG: KASAN: slab-use-after-free in binder_release_work+0x398/0x3d0
Read of size 8 at addr ffff31600ee91488 by task kworker/5:1/211

CPU: 5 UID: 0 PID: 211 Comm: kworker/5:1 Not tainted 6.11.0-rc7-00382-gfc6c92196396 #22
Hardware name: linux,dummy-virt (DT)
Workqueue: events binder_deferred_func
Call trace:
binder_release_work+0x398/0x3d0
binder_deferred_func+0xb60/0x109c
process_one_work+0x51c/0xbd4
worker_thread+0x608/0xee8

Allocated by task 703:
__kmalloc_cache_noprof+0x130/0x280
binder_thread_write+0xdb4/0x42a0
binder_ioctl+0x18f0/0x25ac
__arm64_sys_ioctl+0x124/0x190
invoke_syscall+0x6c/0x254

Freed by task 211:
kfree+0xc4/0x230
binder_deferred_func+0xae8/0x109c
process_one_work+0x51c/0xbd4
worker_thread+0x608/0xee8
==================================================================

This commit fixes the issue by ensuring any queued freeze work is removed
when cleaning up a binder reference.

Fixes: d579b04a52a1 ("binder: frozen notification")
Cc: [email protected]
Acked-by: Todd Kjos <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Signed-off-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: fix OOB in binder_add_freeze_work()

In binder_add_freeze_work() we iterate over the proc->nodes with the
proc->inner_lock held. However, this lock is temporarily dropped to
acquire the node-

binder: fix OOB in binder_add_freeze_work()

In binder_add_freeze_work() we iterate over the proc->nodes with the
proc->inner_lock held. However, this lock is temporarily dropped to
acquire the node->lock first (lock nesting order). This can race with
binder_deferred_release() which removes the nodes from the proc->nodes
rbtree and adds them into binder_dead_nodes list. This leads to a broken
iteration in binder_add_freeze_work() as rb_next() will use data from
binder_dead_nodes, triggering an out-of-bounds access:

==================================================================
BUG: KASAN: global-out-of-bounds in rb_next+0xfc/0x124
Read of size 8 at addr ffffcb84285f7170 by task freeze/660

CPU: 8 UID: 0 PID: 660 Comm: freeze Not tainted 6.11.0-07343-ga727812a8d45 #18
Hardware name: linux,dummy-virt (DT)
Call trace:
rb_next+0xfc/0x124
binder_add_freeze_work+0x344/0x534
binder_ioctl+0x1e70/0x25ac
__arm64_sys_ioctl+0x124/0x190

The buggy address belongs to the variable:
binder_dead_nodes+0x10/0x40
[...]
==================================================================

This is possible because proc->nodes (rbtree) and binder_dead_nodes
(list) share entries in binder_node through a union:

struct binder_node {
[...]
union {
struct rb_node rb_node;
struct hlist_node dead_node;
};

Fix the race by checking that the proc is still alive. If not, simply
break out of the iteration.

Fixes: d579b04a52a1 ("binder: frozen notification")
Cc: [email protected]
Reviewed-by: Alice Ryhl <[email protected]>
Acked-by: Todd Kjos <[email protected]>
Signed-off-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: fix node UAF in binder_add_freeze_work()

In binder_add_freeze_work() we iterate over the proc->nodes with the
proc->inner_lock held. However, this lock is temporarily dropped in
order to acq

binder: fix node UAF in binder_add_freeze_work()

In binder_add_freeze_work() we iterate over the proc->nodes with the
proc->inner_lock held. However, this lock is temporarily dropped in
order to acquire the node->lock first (lock nesting order). This can
race with binder_node_release() and trigger a use-after-free:

==================================================================
BUG: KASAN: slab-use-after-free in _raw_spin_lock+0xe4/0x19c
Write of size 4 at addr ffff53c04c29dd04 by task freeze/640

CPU: 5 UID: 0 PID: 640 Comm: freeze Not tainted 6.11.0-07343-ga727812a8d45 #17
Hardware name: linux,dummy-virt (DT)
Call trace:
_raw_spin_lock+0xe4/0x19c
binder_add_freeze_work+0x148/0x478
binder_ioctl+0x1e70/0x25ac
__arm64_sys_ioctl+0x124/0x190

Allocated by task 637:
__kmalloc_cache_noprof+0x12c/0x27c
binder_new_node+0x50/0x700
binder_transaction+0x35ac/0x6f74
binder_thread_write+0xfb8/0x42a0
binder_ioctl+0x18f0/0x25ac
__arm64_sys_ioctl+0x124/0x190

Freed by task 637:
kfree+0xf0/0x330
binder_thread_read+0x1e88/0x3a68
binder_ioctl+0x16d8/0x25ac
__arm64_sys_ioctl+0x124/0x190
==================================================================

Fix the race by taking a temporary reference on the node before
releasing the proc->inner lock. This ensures the node remains alive
while in use.

Fixes: d579b04a52a1 ("binder: frozen notification")
Cc: [email protected]
Reviewed-by: Alice Ryhl <[email protected]>
Acked-by: Todd Kjos <[email protected]>
Signed-off-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: modify the comment for binder_proc_unlock

Modify the comment for binder_proc_unlock() to clearly indicate which
spinlock it releases and to better match the acquire comment block
in binder_p

binder: modify the comment for binder_proc_unlock

Modify the comment for binder_proc_unlock() to clearly indicate which
spinlock it releases and to better match the acquire comment block
in binder_proc_lock().

Signed-off-by: Ba Jing <[email protected]>
Acked-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: fix UAF caused by offsets overwrite

Binder objects are processed and copied individually into the target
buffer during transactions. Any raw data in-between these objects is
copied as well.

binder: fix UAF caused by offsets overwrite

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 <[email protected]>
Cc: [email protected]
Signed-off-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: fix typo in comment

Correct spelling on 'currently' in comment

Signed-off-by: Ruffalo Lavoisier <[email protected]>
Acked-by: Carlos Llamas <[email protected]>
Link: https://lore

binder: fix typo in comment

Correct spelling on 'currently' in comment

Signed-off-by: Ruffalo Lavoisier <[email protected]>
Acked-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: frozen notification

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

binder: frozen notification

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 <[email protected]>
Acked-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: fix descriptor lookup for context manager

In commit 15d9da3f818c ("binder: use bitmap for faster descriptor
lookup"), it was incorrectly assumed that references to the context
manager node s

binder: fix descriptor lookup for context manager

In commit 15d9da3f818c ("binder: use bitmap for faster descriptor
lookup"), it was incorrectly assumed that references to the context
manager node should always get descriptor zero assigned to them.

However, if the context manager dies and a new process takes its place,
then assigning descriptor zero to the new context manager might lead to
collisions, as there could still be references to the older node. This
issue was reported by syzbot with the following trace:

kernel BUG at drivers/android/binder.c:1173!
Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
Modules linked in:
CPU: 1 PID: 447 Comm: binder-util Not tainted 6.10.0-rc6-00348-g31643d84b8c3 #10
Hardware name: linux,dummy-virt (DT)
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : binder_inc_ref_for_node+0x500/0x544
lr : binder_inc_ref_for_node+0x1e4/0x544
sp : ffff80008112b940
x29: ffff80008112b940 x28: ffff0e0e40310780 x27: 0000000000000000
x26: 0000000000000001 x25: ffff0e0e40310738 x24: ffff0e0e4089ba34
x23: ffff0e0e40310b00 x22: ffff80008112bb50 x21: ffffaf7b8f246970
x20: ffffaf7b8f773f08 x19: ffff0e0e4089b800 x18: 0000000000000000
x17: 0000000000000000 x16: 0000000000000000 x15: 000000002de4aa60
x14: 0000000000000000 x13: 2de4acf000000000 x12: 0000000000000020
x11: 0000000000000018 x10: 0000000000000020 x9 : ffffaf7b90601000
x8 : ffff0e0e48739140 x7 : 0000000000000000 x6 : 000000000000003f
x5 : ffff0e0e40310b28 x4 : 0000000000000000 x3 : ffff0e0e40310720
x2 : ffff0e0e40310728 x1 : 0000000000000000 x0 : ffff0e0e40310710
Call trace:
binder_inc_ref_for_node+0x500/0x544
binder_transaction+0xf68/0x2620
binder_thread_write+0x5bc/0x139c
binder_ioctl+0xef4/0x10c8
[...]

This patch adds back the previous behavior of assigning the next
non-zero descriptor if references to previous context managers still
exist. It amends both strategies, the newer dbitmap code and also the
legacy slow_desc_lookup_olocked(), by allowing them to start looking
for available descriptors at a given offset.

Fixes: 15d9da3f818c ("binder: use bitmap for faster descriptor lookup")
Cc: [email protected]
Reported-and-tested-by: [email protected]
Closes: https://lore.kernel.org/all/[email protected]/
Reviewed-by: Alice Ryhl <[email protected]>
Signed-off-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: fix hang of unregistered readers

With the introduction of binder_available_for_proc_work_ilocked() in
commit 1b77e9dcc3da ("ANDROID: binder: remove proc waitqueue") a binder
thread can only

binder: fix hang of unregistered readers

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: [email protected]
Cc: Martijn Coenen <[email protected]>
Cc: Arve Hjønnevåg <[email protected]>
Signed-off-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: use bitmap for faster descriptor lookup

When creating new binder references, the driver assigns a descriptor id
that is shared with userspace. Regrettably, the driver needs to keep the
descr

binder: use bitmap for faster descriptor lookup

When creating new binder references, the driver assigns a descriptor id
that is shared with userspace. Regrettably, the driver needs to keep the
descriptors small enough to accommodate userspace potentially using them
as Vector indexes. Currently, the driver performs a linear search on the
rb-tree of references to find the smallest available descriptor id. This
approach, however, scales poorly as the number of references grows.

This patch introduces the usage of bitmaps to boost the performance of
descriptor assignments. This optimization results in notable performance
gains, particularly in processes with a large number of references. The
following benchmark with 100,000 references showcases the difference in
latency between the dbitmap implementation and the legacy approach:

[ 587.145098] get_ref_desc_olocked: 15us (dbitmap on)
[ 602.788623] get_ref_desc_olocked: 47343us (dbitmap off)

Note the bitmap size is dynamically adjusted in line with the number of
references, ensuring efficient memory usage. In cases where growing the
bitmap is not possible, the driver falls back to the slow legacy method.

A previous attempt to solve this issue was proposed in [1]. However,
such method involved adding new ioctls which isn't great, plus older
userspace code would not have benefited from the optimizations either.

Link: https://lore.kernel.org/all/[email protected]/ [1]
Cc: Tim Murray <[email protected]>
Cc: Arve Hjønnevåg <[email protected]>
Cc: Alice Ryhl <[email protected]>
Cc: Martijn Coenen <[email protected]>
Cc: Todd Kjos <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Steven Moreland <[email protected]>
Suggested-by: Nick Chen <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Signed-off-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: fix max_thread type inconsistency

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 kernel

binder: fix max_thread type inconsistency

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: a9350fc859ae ("staging: android: binder: fix BINDER_SET_MAX_THREADS declaration")
Reported-by: Arve Hjønnevåg <[email protected]>
Cc: [email protected]
Signed-off-by: Carlos Llamas <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

binder: check offset alignment in binder_get_object()

Commit 6d98eb95b450 ("binder: avoid potential data leakage when copying
txn") introduced changes to how binder objects are copied. In doing so,

binder: check offset alignment in binder_get_object()

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: [email protected]
Signed-off-by: Carlos Llamas <[email protected]>
Acked-by: Todd Kjos <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...