diff options
| author | Blagovest Kolenichev <bkolenichev@codeaurora.org> | 2017-07-26 05:01:18 -0700 |
|---|---|---|
| committer | Blagovest Kolenichev <bkolenichev@codeaurora.org> | 2017-08-04 07:43:15 -0700 |
| commit | 220bdae208dfb20abd040e8e87dc3a4dbca72283 (patch) | |
| tree | e83194e49c350e03a5dae81b928bc8a66b8ee7e4 | |
| parent | b1b3c5a65e5bbb95d1b512c8dc8d1dd3f2451a55 (diff) | |
android: binder: Revert fixes for preemption and locking issues
These were hacks and no longer needed as next binder from
android-4.4@59ff2e1 (v4.4.78) has been reworked to use
fine-grained locking.
This is a preparation for clean merge of binder changes from
android-4.4@59ff2e1 (v4.4.78), which follows right after this
commit.
Reverted changes:
=================
773fc2f android: binder: use copy_from_user_preempt_disabled
To keep the driver consistent, and until we have
fine-grained locking in place.
Change-Id: I90fe02cdedb8a5677b900a68528fb443b9204322
Signed-off-by: Riley Andrews <riandrews@google.com>
Git-repo: https://source.codeaurora.org/quic/la/kernel/msm-4.4
Git-commit: 5c9ce54ca3a66a57e4ebfe3ae71c5733b6bcc579
Signed-off-by: Vikram Mulukutla <markivx@codeaurora.org>
821e02f CHROMIUM: android: binder: Fix potential scheduling-while-atomica
Commit f1e7f0a724f6 ("android: binder: Disable preemption while holding
the global binder lock.") re-enabled preemption around most of the sites
where calls to potentially sleeping functions were made, but missed
__alloc_fd(), which can sleep if the fdtable needs to be resized.
Re-enable preemption around __alloc_fd() as well as __fd_install() which
can now sleep in upstream kernels as of commit 8a81252b774b ("fs/file.c:
don't acquire files->file_lock in fd_install()").
BUG=chrome-os-partner:44012
TEST=Build and boot on Smaug.
Change-Id: I9819c4b95876f697e75b1b84810b6c520d9c33ec
Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/308582
Reviewed-by: Stephen Barber <smbarber@chromium.org>
Reviewed-by: Riley Andrews <riandrews@google.com>
Git-repo: https://source.codeaurora.org/quic/la/kernel/msm-4.4
Git-commit: c267ff1d548ed1bdad6a08f1c70776c5e60d569e
Signed-off-by: Vikram Mulukutla <markivx@codeaurora.org>
e4045d6 android: binder: Disable preemption while holding the global binder lock.
Change-Id: I90fe02cdedb8a5677b900a68528fb443b9204322
Signed-off-by: Riley Andrews <riandrews@google.com>
Git-repo: https://source.codeaurora.org/quic/la/kernel/msm-4.4
Git-commit: 5c9ce54ca3a66a57e4ebfe3ae71c5733b6bcc579
Signed-off-by: Vikram Mulukutla <markivx@codeaurora.org>
Change-Id: Id97e04f3694d76c2b6a1e7c0dc39da15d83c2867
Signed-off-by: Blagovest Kolenichev <bkolenichev@codeaurora.org>
| -rw-r--r-- | drivers/android/binder.c | 179 |
1 files changed, 49 insertions, 130 deletions
diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 0c3cf182e351..573e9795b236 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -417,7 +417,6 @@ static int task_get_unused_fd_flags(struct binder_proc *proc, int flags) struct files_struct *files = proc->files; unsigned long rlim_cur; unsigned long irqs; - int ret; if (files == NULL) return -ESRCH; @@ -428,11 +427,7 @@ static int task_get_unused_fd_flags(struct binder_proc *proc, int flags) rlim_cur = task_rlimit(proc->tsk, RLIMIT_NOFILE); unlock_task_sighand(proc->tsk, &irqs); - preempt_enable_no_resched(); - ret = __alloc_fd(files, 0, rlim_cur, flags); - preempt_disable(); - - return ret; + return __alloc_fd(files, 0, rlim_cur, flags); } /* @@ -441,11 +436,8 @@ static int task_get_unused_fd_flags(struct binder_proc *proc, int flags) static void task_fd_install( struct binder_proc *proc, unsigned int fd, struct file *file) { - if (proc->files) { - preempt_enable_no_resched(); + if (proc->files) __fd_install(proc->files, fd, file); - preempt_disable(); - } } /* @@ -473,7 +465,6 @@ static inline void binder_lock(struct binder_context *context, const char *tag) { trace_binder_lock(tag); mutex_lock(&context->binder_main_lock); - preempt_disable(); trace_binder_locked(tag); } @@ -482,62 +473,8 @@ static inline void binder_unlock(struct binder_context *context, { trace_binder_unlock(tag); mutex_unlock(&context->binder_main_lock); - preempt_enable(); -} - -static inline void *kzalloc_preempt_disabled(size_t size) -{ - void *ptr; - - ptr = kzalloc(size, GFP_NOWAIT); - if (ptr) - return ptr; - - preempt_enable_no_resched(); - ptr = kzalloc(size, GFP_KERNEL); - preempt_disable(); - - return ptr; } -static inline long copy_to_user_preempt_disabled(void __user *to, const void *from, long n) -{ - long ret; - - preempt_enable_no_resched(); - ret = copy_to_user(to, from, n); - preempt_disable(); - return ret; -} - -static inline long copy_from_user_preempt_disabled(void *to, const void __user *from, long n) -{ - long ret; - - preempt_enable_no_resched(); - ret = copy_from_user(to, from, n); - preempt_disable(); - return ret; -} - -#define get_user_preempt_disabled(x, ptr) \ -({ \ - int __ret; \ - preempt_enable_no_resched(); \ - __ret = get_user(x, ptr); \ - preempt_disable(); \ - __ret; \ -}) - -#define put_user_preempt_disabled(x, ptr) \ -({ \ - int __ret; \ - preempt_enable_no_resched(); \ - __ret = put_user(x, ptr); \ - preempt_disable(); \ - __ret; \ -}) - static void binder_set_nice(long nice) { long min_nice; @@ -670,8 +607,6 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate, else mm = get_task_mm(proc->tsk); - preempt_enable_no_resched(); - if (mm) { down_write(&mm->mmap_sem); vma = proc->vma; @@ -726,9 +661,6 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate, up_write(&mm->mmap_sem); mmput(mm); } - - preempt_disable(); - return 0; free_range: @@ -751,9 +683,6 @@ err_no_vma: up_write(&mm->mmap_sem); mmput(mm); } - - preempt_disable(); - return -ENOMEM; } @@ -1022,7 +951,7 @@ static struct binder_node *binder_new_node(struct binder_proc *proc, return NULL; } - node = kzalloc_preempt_disabled(sizeof(*node)); + node = kzalloc(sizeof(*node), GFP_KERNEL); if (node == NULL) return NULL; binder_stats_created(BINDER_STAT_NODE); @@ -1166,7 +1095,7 @@ static struct binder_ref *binder_get_ref_for_node(struct binder_proc *proc, else return ref; } - new_ref = kzalloc_preempt_disabled(sizeof(*ref)); + new_ref = kzalloc(sizeof(*ref), GFP_KERNEL); if (new_ref == NULL) return NULL; binder_stats_created(BINDER_STAT_REF); @@ -2039,14 +1968,14 @@ static void binder_transaction(struct binder_proc *proc, e->to_proc = target_proc->pid; /* TODO: reuse incoming transaction for reply */ - t = kzalloc_preempt_disabled(sizeof(*t)); + t = kzalloc(sizeof(*t), GFP_KERNEL); if (t == NULL) { return_error = BR_FAILED_REPLY; goto err_alloc_t_failed; } binder_stats_created(BINDER_STAT_TRANSACTION); - tcomplete = kzalloc_preempt_disabled(sizeof(*tcomplete)); + tcomplete = kzalloc(sizeof(*tcomplete), GFP_KERNEL); if (tcomplete == NULL) { return_error = BR_FAILED_REPLY; goto err_alloc_tcomplete_failed; @@ -2107,14 +2036,14 @@ static void binder_transaction(struct binder_proc *proc, ALIGN(tr->data_size, sizeof(void *))); offp = off_start; - if (copy_from_user_preempt_disabled(t->buffer->data, (const void __user *)(uintptr_t) + if (copy_from_user(t->buffer->data, (const void __user *)(uintptr_t) tr->data.ptr.buffer, tr->data_size)) { binder_user_error("%d:%d got transaction with invalid data ptr\n", proc->pid, thread->pid); return_error = BR_FAILED_REPLY; goto err_copy_data_failed; } - if (copy_from_user_preempt_disabled(offp, (const void __user *)(uintptr_t) + if (copy_from_user(offp, (const void __user *)(uintptr_t) tr->data.ptr.offsets, tr->offsets_size)) { binder_user_error("%d:%d got transaction with invalid offsets ptr\n", proc->pid, thread->pid); @@ -2232,10 +2161,9 @@ static void binder_transaction(struct binder_proc *proc, return_error = BR_FAILED_REPLY; goto err_bad_offset; } - if (copy_from_user_preempt_disabled( - sg_bufp, - (const void __user *)(uintptr_t) - bp->buffer, bp->length)) { + if (copy_from_user(sg_bufp, + (const void __user *)(uintptr_t) + bp->buffer, bp->length)) { binder_user_error("%d:%d got transaction with invalid offsets ptr\n", proc->pid, thread->pid); return_error = BR_FAILED_REPLY; @@ -2351,7 +2279,7 @@ static int binder_thread_write(struct binder_proc *proc, void __user *end = buffer + size; while (ptr < end && thread->return_error == BR_OK) { - if (get_user_preempt_disabled(cmd, (uint32_t __user *)ptr)) + if (get_user(cmd, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); trace_binder_command(cmd); @@ -2369,7 +2297,7 @@ static int binder_thread_write(struct binder_proc *proc, struct binder_ref *ref; const char *debug_string; - if (get_user_preempt_disabled(target, (uint32_t __user *)ptr)) + if (get_user(target, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); if (target == 0 && context->binder_context_mgr_node && @@ -2421,10 +2349,10 @@ static int binder_thread_write(struct binder_proc *proc, binder_uintptr_t cookie; struct binder_node *node; - if (get_user_preempt_disabled(node_ptr, (binder_uintptr_t __user *)ptr)) + if (get_user(node_ptr, (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(binder_uintptr_t); - if (get_user_preempt_disabled(cookie, (binder_uintptr_t __user *)ptr)) + if (get_user(cookie, (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(binder_uintptr_t); node = binder_get_node(proc, node_ptr); @@ -2482,7 +2410,7 @@ static int binder_thread_write(struct binder_proc *proc, binder_uintptr_t data_ptr; struct binder_buffer *buffer; - if (get_user_preempt_disabled(data_ptr, (binder_uintptr_t __user *)ptr)) + if (get_user(data_ptr, (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(binder_uintptr_t); @@ -2524,8 +2452,7 @@ static int binder_thread_write(struct binder_proc *proc, case BC_REPLY_SG: { struct binder_transaction_data_sg tr; - if (copy_from_user_preempt_disabled(&tr, ptr, - sizeof(tr))) + if (copy_from_user(&tr, ptr, sizeof(tr))) return -EFAULT; ptr += sizeof(tr); binder_transaction(proc, thread, &tr.transaction_data, @@ -2536,7 +2463,7 @@ static int binder_thread_write(struct binder_proc *proc, case BC_REPLY: { struct binder_transaction_data tr; - if (copy_from_user_preempt_disabled(&tr, ptr, sizeof(tr))) + if (copy_from_user(&tr, ptr, sizeof(tr))) return -EFAULT; ptr += sizeof(tr); binder_transaction(proc, thread, &tr, @@ -2587,10 +2514,10 @@ static int binder_thread_write(struct binder_proc *proc, struct binder_ref *ref; struct binder_ref_death *death; - if (get_user_preempt_disabled(target, (uint32_t __user *)ptr)) + if (get_user(target, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); - if (get_user_preempt_disabled(cookie, (binder_uintptr_t __user *)ptr)) + if (get_user(cookie, (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(binder_uintptr_t); ref = binder_get_ref(proc, target, false); @@ -2619,7 +2546,7 @@ static int binder_thread_write(struct binder_proc *proc, proc->pid, thread->pid); break; } - death = kzalloc_preempt_disabled(sizeof(*death)); + death = kzalloc(sizeof(*death), GFP_KERNEL); if (death == NULL) { thread->return_error = BR_ERROR; binder_debug(BINDER_DEBUG_FAILED_TRANSACTION, @@ -2673,7 +2600,8 @@ static int binder_thread_write(struct binder_proc *proc, struct binder_work *w; binder_uintptr_t cookie; struct binder_ref_death *death = NULL; - if (get_user_preempt_disabled(cookie, (binder_uintptr_t __user *)ptr)) + + if (get_user(cookie, (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(cookie); @@ -2705,8 +2633,7 @@ static int binder_thread_write(struct binder_proc *proc, wake_up_interruptible(&proc->wait); } } - } - break; + } break; default: pr_err("%d:%d unknown command %d\n", @@ -2755,7 +2682,7 @@ static int binder_thread_read(struct binder_proc *proc, int wait_for_proc_work; if (*consumed == 0) { - if (put_user_preempt_disabled(BR_NOOP, (uint32_t __user *)ptr)) + if (put_user(BR_NOOP, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); } @@ -2766,7 +2693,7 @@ retry: if (thread->return_error != BR_OK && ptr < end) { if (thread->return_error2 != BR_OK) { - if (put_user_preempt_disabled(thread->return_error2, (uint32_t __user *)ptr)) + if (put_user(thread->return_error2, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); binder_stat_br(proc, thread, thread->return_error2); @@ -2774,7 +2701,7 @@ retry: goto done; thread->return_error2 = BR_OK; } - if (put_user_preempt_disabled(thread->return_error, (uint32_t __user *)ptr)) + if (put_user(thread->return_error, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); binder_stat_br(proc, thread, thread->return_error); @@ -2852,7 +2779,7 @@ retry: } break; case BINDER_WORK_TRANSACTION_COMPLETE: { cmd = BR_TRANSACTION_COMPLETE; - if (put_user_preempt_disabled(cmd, (uint32_t __user *) ptr)) + if (put_user(cmd, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); @@ -2894,14 +2821,14 @@ retry: node->has_weak_ref = 0; } if (cmd != BR_NOOP) { - if (put_user_preempt_disabled(cmd, (uint32_t __user *) ptr)) + if (put_user(cmd, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); - if (put_user_preempt_disabled(node->ptr, (binder_uintptr_t __user *) + if (put_user(node->ptr, (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(binder_uintptr_t); - if (put_user_preempt_disabled(node->cookie, (binder_uintptr_t __user *) + if (put_user(node->cookie, (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(binder_uintptr_t); @@ -2945,10 +2872,11 @@ retry: cmd = BR_CLEAR_DEATH_NOTIFICATION_DONE; else cmd = BR_DEAD_BINDER; - if (put_user_preempt_disabled(cmd, (uint32_t __user *) ptr)) + if (put_user(cmd, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); - if (put_user_preempt_disabled(death->cookie, (binder_uintptr_t __user *) ptr)) + if (put_user(death->cookie, + (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(binder_uintptr_t); binder_stat_br(proc, thread, cmd); @@ -3015,10 +2943,10 @@ retry: ALIGN(t->buffer->data_size, sizeof(void *)); - if (put_user_preempt_disabled(cmd, (uint32_t __user *) ptr)) + if (put_user(cmd, (uint32_t __user *)ptr)) return -EFAULT; ptr += sizeof(uint32_t); - if (copy_to_user_preempt_disabled(ptr, &tr, sizeof(tr))) + if (copy_to_user(ptr, &tr, sizeof(tr))) return -EFAULT; ptr += sizeof(tr); @@ -3060,7 +2988,7 @@ done: binder_debug(BINDER_DEBUG_THREADS, "%d:%d BR_SPAWN_LOOPER\n", proc->pid, thread->pid); - if (put_user_preempt_disabled(BR_SPAWN_LOOPER, (uint32_t __user *) buffer)) + if (put_user(BR_SPAWN_LOOPER, (uint32_t __user *)buffer)) return -EFAULT; binder_stat_br(proc, thread, BR_SPAWN_LOOPER); } @@ -3135,7 +3063,7 @@ static struct binder_thread *binder_get_thread(struct binder_proc *proc) break; } if (*p == NULL) { - thread = kzalloc_preempt_disabled(sizeof(*thread)); + thread = kzalloc(sizeof(*thread), GFP_KERNEL); if (thread == NULL) return NULL; binder_stats_created(BINDER_STAT_THREAD); @@ -3239,7 +3167,7 @@ static int binder_ioctl_write_read(struct file *filp, ret = -EINVAL; goto out; } - if (copy_from_user_preempt_disabled(&bwr, ubuf, sizeof(bwr))) { + if (copy_from_user(&bwr, ubuf, sizeof(bwr))) { ret = -EFAULT; goto out; } @@ -3257,7 +3185,7 @@ static int binder_ioctl_write_read(struct file *filp, trace_binder_write_done(ret); if (ret < 0) { bwr.read_consumed = 0; - if (copy_to_user_preempt_disabled(ubuf, &bwr, sizeof(bwr))) + if (copy_to_user(ubuf, &bwr, sizeof(bwr))) ret = -EFAULT; goto out; } @@ -3271,7 +3199,7 @@ static int binder_ioctl_write_read(struct file *filp, if (!list_empty(&proc->todo)) wake_up_interruptible(&proc->wait); if (ret < 0) { - if (copy_to_user_preempt_disabled(ubuf, &bwr, sizeof(bwr))) + if (copy_to_user(ubuf, &bwr, sizeof(bwr))) ret = -EFAULT; goto out; } @@ -3281,7 +3209,7 @@ static int binder_ioctl_write_read(struct file *filp, proc->pid, thread->pid, (u64)bwr.write_consumed, (u64)bwr.write_size, (u64)bwr.read_consumed, (u64)bwr.read_size); - if (copy_to_user_preempt_disabled(ubuf, &bwr, sizeof(bwr))) { + if (copy_to_user(ubuf, &bwr, sizeof(bwr))) { ret = -EFAULT; goto out; } @@ -3362,7 +3290,7 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) goto err; break; case BINDER_SET_MAX_THREADS: - if (copy_from_user_preempt_disabled(&proc->max_threads, ubuf, sizeof(proc->max_threads))) { + if (copy_from_user(&proc->max_threads, ubuf, sizeof(proc->max_threads))) { ret = -EINVAL; goto err; } @@ -3385,8 +3313,9 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) ret = -EINVAL; goto err; } - if (put_user_preempt_disabled(BINDER_CURRENT_PROTOCOL_VERSION, &ver->protocol_version)) { - ret = -EINVAL; + if (put_user(BINDER_CURRENT_PROTOCOL_VERSION, + &ver->protocol_version)) { + ret = -EINVAL; goto err; } break; @@ -3447,7 +3376,6 @@ static const struct vm_operations_struct binder_vm_ops = { static int binder_mmap(struct file *filp, struct vm_area_struct *vma) { int ret; - struct vm_struct *area; struct binder_proc *proc = filp->private_data; const char *failure_string; @@ -3508,11 +3436,7 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma) vma->vm_ops = &binder_vm_ops; vma->vm_private_data = proc; - /* binder_update_page_range assumes preemption is disabled */ - preempt_disable(); - ret = binder_update_page_range(proc, 1, proc->buffer, proc->buffer + PAGE_SIZE, vma); - preempt_enable_no_resched(); - if (ret) { + if (binder_update_page_range(proc, 1, proc->buffer, proc->buffer + PAGE_SIZE, vma)) { ret = -ENOMEM; failure_string = "alloc small buf"; goto err_alloc_small_buf_failed; @@ -3803,12 +3727,9 @@ static void binder_deferred_func(struct work_struct *work) int defer; do { - trace_binder_lock(__func__); - mutex_lock(&context->binder_main_lock); - trace_binder_locked(__func__); + binder_lock(context, __func__); mutex_lock(&context->binder_deferred_lock); - preempt_disable(); if (!hlist_empty(&context->binder_deferred_list)) { proc = hlist_entry(context->binder_deferred_list.first, struct binder_proc, deferred_work_node); @@ -3834,9 +3755,7 @@ static void binder_deferred_func(struct work_struct *work) if (defer & BINDER_DEFERRED_RELEASE) binder_deferred_release(proc); /* frees proc */ - trace_binder_unlock(__func__); - mutex_unlock(&context->binder_main_lock); - preempt_enable_no_resched(); + binder_unlock(context, __func__); if (files) put_files_struct(files); } while (proc); |
