Evolution-X-Devices/kernel_google_wahoo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

binder: make inc/ref user commands atomic with node state

Browse Source 
In binder_thread_read, the BINDER_WORK_NODE command is used
to communicate the references on the node to userspace. It
often takes a couple of iterations in the loop to construct
the list of commands for user space. During the loop, the
proc lock is released which means state can change. The
case found in the bug was a inc-after-dec where the node
briefly had 0 references and this was reported to userspace,
but before deleting it in the kernel, a new reference was
taken. Unfortunately, the node was destroyed in userspace
so when the inc was reported to userspace there was a crash.

Fixed by changing the algorithm in binder_thread_read to
determine which commands to send to userspace atomically
so it stays consistent with the kernel view. If it is the
last reference, then the node becomes a zombie when the
userspace command will cause node object destruction.

Bug: 36093037
Change-Id: I02c5cfdd3ab6dadea7f07f2a275faf3e27be77ad
Test: tested manually
Signed-off-by: Todd Kjos <tkjos@google.com>
This commit is contained in:
Todd Kjos
2017-03-21 13:06:01 -07:00
parent a790f8b2ad
commit 88c8126c20
1 changed files with 52 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

104
drivers/android/binder.c
View File

@@ -2936,10 +2936,12 @@ retry:
} break;
case BINDER_WORK_NODE: {
struct binder_node *node = container_of(w, struct binder_node, work);
uint32_t cmd = BR_NOOP;
const char *cmd_name;
uint32_t cmd[2];
const char *cmd_name[2];
int cmd_count = 0;
int strong, weak;
struct binder_proc *proc = node->proc;
int i;
binder_proc_lock(proc, __LINE__);
strong = node->internal_strong_refs ||
@@ -2948,74 +2950,72 @@ retry:
node->local_weak_refs || strong;
if (weak && !node->has_weak_ref) {
cmd = BR_INCREFS;
cmd_name = "BR_INCREFS";
cmd[cmd_count] = BR_INCREFS;
cmd_name[cmd_count++] = "BR_INCREFS";
node->has_weak_ref = 1;
node->pending_weak_ref = 1;
node->local_weak_refs++;
} else if (strong && !node->has_strong_ref) {
cmd = BR_ACQUIRE;
cmd_name = "BR_ACQUIRE";
}
if (strong && !node->has_strong_ref) {
cmd[cmd_count] = BR_ACQUIRE;
cmd_name[cmd_count++] = "BR_ACQUIRE";
node->has_strong_ref = 1;
node->pending_strong_ref = 1;
node->local_strong_refs++;
} else if (!strong && node->has_strong_ref) {
cmd = BR_RELEASE;
cmd_name = "BR_RELEASE";
}
if (!strong && node->has_strong_ref) {
cmd[cmd_count] = BR_RELEASE;
cmd_name[cmd_count++] = "BR_RELEASE";
node->has_strong_ref = 0;
} else if (!weak && node->has_weak_ref) {
cmd = BR_DECREFS;
cmd_name = "BR_DECREFS";
}
if (!weak && node->has_weak_ref) {
cmd[cmd_count] = BR_DECREFS;
cmd_name[cmd_count++] = "BR_DECREFS";
node->has_weak_ref = 0;
}
if (cmd != BR_NOOP) {
binder_proc_unlock(proc, __LINE__);
if (put_user(cmd, (uint32_t __user *)ptr)) {
binder_unfreeze_worklist(wlist);
BUG_ON(cmd_count > 2);
if (!weak && !strong && !node->is_zombie) {
binder_debug(BINDER_DEBUG_INTERNAL_REFS,
"%d:%d node %d u%016llx c%016llx deleted\n",
proc->pid, thread->pid,
node->debug_id,
(u64)node->ptr,
(u64)node->cookie);
_binder_make_node_zombie(node);
} else {
binder_debug(BINDER_DEBUG_INTERNAL_REFS,
"%d:%d node %d u%016llx c%016llx state unchanged\n",
proc->pid, thread->pid,
node->debug_id,
(u64)node->ptr,
(u64)node->cookie);
}
binder_dequeue_work(w, __LINE__);
binder_unfreeze_worklist(wlist);
binder_proc_unlock(proc, __LINE__);
for (i = 0; i < cmd_count; i++) {
if (put_user(cmd[i], (uint32_t __user *)ptr))
return -EFAULT;
}
ptr += sizeof(uint32_t);
if (put_user(node->ptr,
(binder_uintptr_t __user *)ptr)) {
binder_unfreeze_worklist(wlist);
(binder_uintptr_t __user *)ptr))
return -EFAULT;
}
ptr += sizeof(binder_uintptr_t);
if (put_user(node->cookie,
(binder_uintptr_t __user *)ptr)) {
binder_unfreeze_worklist(wlist);
return -EFAULT;
}
ptr += sizeof(binder_uintptr_t);
binder_stat_br(proc, thread, cmd);
if (put_user(node->cookie,
(binder_uintptr_t __user *)ptr))
return -EFAULT;
ptr += sizeof(binder_uintptr_t);
binder_stat_br(proc, thread, cmd[i]);
binder_debug(BINDER_DEBUG_USER_REFS,
"%d:%d %s %d u%016llx c%016llx\n",
proc->pid, thread->pid, cmd_name,
node->debug_id,
proc->pid, thread->pid,
cmd_name[i], node->debug_id,
(u64)node->ptr, (u64)node->cookie);
binder_unfreeze_worklist(wlist);
} else {
binder_dequeue_work(w, __LINE__);
if (!weak && !strong && !node->is_zombie) {
binder_debug(BINDER_DEBUG_INTERNAL_REFS,
"%d:%d node %d u%016llx c%016llx deleted\n",
proc->pid, thread->pid,
node->debug_id,
(u64)node->ptr,
(u64)node->cookie);
_binder_make_node_zombie(node);
binder_proc_unlock(proc, __LINE__);
} else {
binder_proc_unlock(proc, __LINE__);
binder_debug(BINDER_DEBUG_INTERNAL_REFS,
"%d:%d node %d u%016llx c%016llx state unchanged\n",
proc->pid, thread->pid,
node->debug_id,
(u64)node->ptr,
(u64)node->cookie);
}
binder_unfreeze_worklist(wlist);
}
} break;
case BINDER_WORK_DEAD_BINDER: