Skip to content

Commit a7dc1e6

Browse files
Hang Lugregkh
authored andcommitted
binder: tell userspace to dump current backtrace when detected oneway spamming
When async binder buffer got exhausted, some normal oneway transactions will also be discarded and may cause system or application failures. By that time, the binder debug information we dump may not be relevant to the root cause. And this issue is difficult to debug if without the backtrace of the thread sending spam. This change will send BR_ONEWAY_SPAM_SUSPECT to userspace when oneway spamming is detected, request to dump current backtrace. Oneway spamming will be reported only once when exceeding the threshold (target process dips below 80% of its oneway space, and current process is responsible for either more than 50 transactions, or more than 50% of the oneway space). And the detection will restart when the async buffer has returned to a healthy state. Acked-by: Todd Kjos <[email protected]> Signed-off-by: Hang Lu <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 0051691 commit a7dc1e6

File tree

5 files changed

+56
-8
lines changed

5 files changed

+56
-8
lines changed

drivers/android/binder.c

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3020,7 +3020,10 @@ static void binder_transaction(struct binder_proc *proc,
30203020
goto err_bad_object_type;
30213021
}
30223022
}
3023-
tcomplete->type = BINDER_WORK_TRANSACTION_COMPLETE;
3023+
if (t->buffer->oneway_spam_suspect)
3024+
tcomplete->type = BINDER_WORK_TRANSACTION_ONEWAY_SPAM_SUSPECT;
3025+
else
3026+
tcomplete->type = BINDER_WORK_TRANSACTION_COMPLETE;
30243027
t->work.type = BINDER_WORK_TRANSACTION;
30253028

30263029
if (reply) {
@@ -3893,9 +3896,14 @@ static int binder_thread_read(struct binder_proc *proc,
38933896

38943897
binder_stat_br(proc, thread, cmd);
38953898
} break;
3896-
case BINDER_WORK_TRANSACTION_COMPLETE: {
3899+
case BINDER_WORK_TRANSACTION_COMPLETE:
3900+
case BINDER_WORK_TRANSACTION_ONEWAY_SPAM_SUSPECT: {
3901+
if (proc->oneway_spam_detection_enabled &&
3902+
w->type == BINDER_WORK_TRANSACTION_ONEWAY_SPAM_SUSPECT)
3903+
cmd = BR_ONEWAY_SPAM_SUSPECT;
3904+
else
3905+
cmd = BR_TRANSACTION_COMPLETE;
38973906
binder_inner_proc_unlock(proc);
3898-
cmd = BR_TRANSACTION_COMPLETE;
38993907
kfree(w);
39003908
binder_stats_deleted(BINDER_STAT_TRANSACTION_COMPLETE);
39013909
if (put_user(cmd, (uint32_t __user *)ptr))
@@ -4897,6 +4905,18 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
48974905
}
48984906
break;
48994907
}
4908+
case BINDER_ENABLE_ONEWAY_SPAM_DETECTION: {
4909+
uint32_t enable;
4910+
4911+
if (copy_from_user(&enable, ubuf, sizeof(enable))) {
4912+
ret = -EINVAL;
4913+
goto err;
4914+
}
4915+
binder_inner_proc_lock(proc);
4916+
proc->oneway_spam_detection_enabled = (bool)enable;
4917+
binder_inner_proc_unlock(proc);
4918+
break;
4919+
}
49004920
default:
49014921
ret = -EINVAL;
49024922
goto err;
@@ -5561,6 +5581,7 @@ static const char * const binder_return_strings[] = {
55615581
"BR_CLEAR_DEATH_NOTIFICATION_DONE",
55625582
"BR_FAILED_REPLY",
55635583
"BR_FROZEN_REPLY",
5584+
"BR_ONEWAY_SPAM_SUSPECT",
55645585
};
55655586

55665587
static const char * const binder_command_strings[] = {

drivers/android/binder_alloc.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ static inline struct vm_area_struct *binder_alloc_get_vma(
338338
return vma;
339339
}
340340

341-
static void debug_low_async_space_locked(struct binder_alloc *alloc, int pid)
341+
static bool debug_low_async_space_locked(struct binder_alloc *alloc, int pid)
342342
{
343343
/*
344344
* Find the amount and size of buffers allocated by the current caller;
@@ -366,13 +366,19 @@ static void debug_low_async_space_locked(struct binder_alloc *alloc, int pid)
366366

367367
/*
368368
* Warn if this pid has more than 50 transactions, or more than 50% of
369-
* async space (which is 25% of total buffer size).
369+
* async space (which is 25% of total buffer size). Oneway spam is only
370+
* detected when the threshold is exceeded.
370371
*/
371372
if (num_buffers > 50 || total_alloc_size > alloc->buffer_size / 4) {
372373
binder_alloc_debug(BINDER_DEBUG_USER_ERROR,
373374
"%d: pid %d spamming oneway? %zd buffers allocated for a total size of %zd\n",
374375
alloc->pid, pid, num_buffers, total_alloc_size);
376+
if (!alloc->oneway_spam_detected) {
377+
alloc->oneway_spam_detected = true;
378+
return true;
379+
}
375380
}
381+
return false;
376382
}
377383

378384
static struct binder_buffer *binder_alloc_new_buf_locked(
@@ -525,6 +531,7 @@ static struct binder_buffer *binder_alloc_new_buf_locked(
525531
buffer->async_transaction = is_async;
526532
buffer->extra_buffers_size = extra_buffers_size;
527533
buffer->pid = pid;
534+
buffer->oneway_spam_suspect = false;
528535
if (is_async) {
529536
alloc->free_async_space -= size + sizeof(struct binder_buffer);
530537
binder_alloc_debug(BINDER_DEBUG_BUFFER_ALLOC_ASYNC,
@@ -536,7 +543,9 @@ static struct binder_buffer *binder_alloc_new_buf_locked(
536543
* of async space left (which is less than 10% of total
537544
* buffer size).
538545
*/
539-
debug_low_async_space_locked(alloc, pid);
546+
buffer->oneway_spam_suspect = debug_low_async_space_locked(alloc, pid);
547+
} else {
548+
alloc->oneway_spam_detected = false;
540549
}
541550
}
542551
return buffer;

drivers/android/binder_alloc.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ struct binder_transaction;
2626
* @clear_on_free: %true if buffer must be zeroed after use
2727
* @allow_user_free: %true if user is allowed to free buffer
2828
* @async_transaction: %true if buffer is in use for an async txn
29+
* @oneway_spam_suspect: %true if total async allocate size just exceed
30+
* spamming detect threshold
2931
* @debug_id: unique ID for debugging
3032
* @transaction: pointer to associated struct binder_transaction
3133
* @target_node: struct binder_node associated with this buffer
@@ -45,7 +47,8 @@ struct binder_buffer {
4547
unsigned clear_on_free:1;
4648
unsigned allow_user_free:1;
4749
unsigned async_transaction:1;
48-
unsigned debug_id:28;
50+
unsigned oneway_spam_suspect:1;
51+
unsigned debug_id:27;
4952

5053
struct binder_transaction *transaction;
5154

@@ -87,6 +90,8 @@ struct binder_lru_page {
8790
* @buffer_size: size of address space specified via mmap
8891
* @pid: pid for associated binder_proc (invariant after init)
8992
* @pages_high: high watermark of offset in @pages
93+
* @oneway_spam_detected: %true if oneway spam detection fired, clear that
94+
* flag once the async buffer has returned to a healthy state
9095
*
9196
* Bookkeeping structure for per-proc address space management for binder
9297
* buffers. It is normally initialized during binder_init() and binder_mmap()
@@ -107,6 +112,7 @@ struct binder_alloc {
107112
uint32_t buffer_free;
108113
int pid;
109114
size_t pages_high;
115+
bool oneway_spam_detected;
110116
};
111117

112118
#ifdef CONFIG_ANDROID_BINDER_IPC_SELFTEST

drivers/android/binder_internal.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ enum binder_stat_types {
155155
};
156156

157157
struct binder_stats {
158-
atomic_t br[_IOC_NR(BR_FROZEN_REPLY) + 1];
158+
atomic_t br[_IOC_NR(BR_ONEWAY_SPAM_SUSPECT) + 1];
159159
atomic_t bc[_IOC_NR(BC_REPLY_SG) + 1];
160160
atomic_t obj_created[BINDER_STAT_COUNT];
161161
atomic_t obj_deleted[BINDER_STAT_COUNT];
@@ -174,6 +174,7 @@ struct binder_work {
174174
enum binder_work_type {
175175
BINDER_WORK_TRANSACTION = 1,
176176
BINDER_WORK_TRANSACTION_COMPLETE,
177+
BINDER_WORK_TRANSACTION_ONEWAY_SPAM_SUSPECT,
177178
BINDER_WORK_RETURN_ERROR,
178179
BINDER_WORK_NODE,
179180
BINDER_WORK_DEAD_BINDER,
@@ -409,6 +410,8 @@ struct binder_ref {
409410
* @outer_lock: no nesting under innor or node lock
410411
* Lock order: 1) outer, 2) node, 3) inner
411412
* @binderfs_entry: process-specific binderfs log file
413+
* @oneway_spam_detection_enabled: process enabled oneway spam detection
414+
* or not
412415
*
413416
* Bookkeeping structure for binder processes
414417
*/
@@ -444,6 +447,7 @@ struct binder_proc {
444447
spinlock_t inner_lock;
445448
spinlock_t outer_lock;
446449
struct dentry *binderfs_entry;
450+
bool oneway_spam_detection_enabled;
447451
};
448452

449453
/**

include/uapi/linux/android/binder.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ struct binder_frozen_status_info {
241241
#define BINDER_SET_CONTEXT_MGR_EXT _IOW('b', 13, struct flat_binder_object)
242242
#define BINDER_FREEZE _IOW('b', 14, struct binder_freeze_info)
243243
#define BINDER_GET_FROZEN_INFO _IOWR('b', 15, struct binder_frozen_status_info)
244+
#define BINDER_ENABLE_ONEWAY_SPAM_DETECTION _IOW('b', 16, __u32)
244245

245246
/*
246247
* NOTE: Two special error codes you should check for when calling
@@ -428,6 +429,13 @@ enum binder_driver_return_protocol {
428429
* The target of the last transaction (either a bcTRANSACTION or
429430
* a bcATTEMPT_ACQUIRE) is frozen. No parameters.
430431
*/
432+
433+
BR_ONEWAY_SPAM_SUSPECT = _IO('r', 19),
434+
/*
435+
* Current process sent too many oneway calls to target, and the last
436+
* asynchronous transaction makes the allocated async buffer size exceed
437+
* detection threshold. No parameters.
438+
*/
431439
};
432440

433441
enum binder_driver_command_protocol {

0 commit comments

Comments
 (0)