March 2018 - Linux-stable-mirror

Re: [PATCH AUTOSEL for 4.15 026/102] ASoC: fsl_ssi: only enable proper channel slots in AC'97 mode

by Sasha Levin

On Mon, Mar 05, 2018 at 10:20:09AM +0000, Mark Brown wrote: >On Sat, Mar 03, 2018 at 10:24:24PM +0000, Sasha Levin wrote: > >> We need to make sure that only proper channel slots (in SACCST register) >> are enabled at playback start time since some AC'97 CODECs (like VT1613 on >> UDOO board) were observed requesting via SLOTREQ spurious ones just after >> an AC'97 link is started but before the CODEC is configured by its driver. > >This was part of a wider AC'97 rework IIRC and I'm very suspicious of >backporting it independently. I'll drop it. Thank you! -- Thanks, Sasha

6 years, 9 months

1
0
0 0

Re: [PATCH AUTOSEL for 4.9 111/219] ASoc: rt5645: Add OF device ID table

by Sasha Levin

On Mon, Mar 05, 2018 at 10:42:03AM +0000, Mark Brown wrote: >On Sat, Mar 03, 2018 at 10:29:00PM +0000, Sasha Levin wrote: >> From: Javier Martinez Canillas <javier(a)osg.samsung.com> >> >> [ Upstream commit 9ba2da5f5d18daaa365ab5426b05e16f1d114786 ] >> >> The driver doesn't have a struct of_device_id table but supported devices >> are registered via Device Trees. This is working on the assumption that a >> I2C device registered via OF will always match a legacy I2C device ID and >> that the MODALIAS reported will always be of the form i2c:<device>. >> >> But this could change in the future so the correct approach is to have an >> OF device ID table if the devices are registered via OF. > >As the commit message itself says this is not fixing anything, it's >defence against future changes. I was under the impression that this refers to future HW changes rather than code, I'll drop all 3. Thanks! -- Thanks, Sasha

6 years, 9 months

1
0
0 0

Re: [PATCH AUTOSEL for 4.9 002/219] spi/bcm63xx: make spi subsystem aware of message size limits

by Sasha Levin

On Mon, Mar 05, 2018 at 10:23:10AM +0000, Mark Brown wrote: >On Sat, Mar 03, 2018 at 10:27:56PM +0000, Sasha Levin wrote: >> From: Jonas Gorski <jonas.gorski(a)gmail.com> >> >> [ Upstream commit 0135c03df914f0481c61f097c78d37cece84f330 ] > >Why are there so many more patches for v4.9 than for more recent >kernels? The v4.11..v4.14 range got processed, and all those commits are now being pushed into 4.9 and older. >> The bcm63xx SPI controller does not allow manual control of the CS >> lines and will toggle it automatically before and after sending data, >> so we are limited to messages that fit in the FIFO buffer. Since the CS >> lines aren't available as GPIOs either, we will need to make slave >> drivers aware of this limitation so they can handle them accordingly. > >This seems really aggressive for stable... Why so? -- Thanks, Sasha

6 years, 9 months

1
0
0 0

[PATCH 100/103] binder: fix proc->files use-after-free

by micky387

From: Todd Kjos <tkjos(a)android.com> proc->files cleanup is initiated by binder_vma_close. Therefore a reference on the binder_proc is not enough to prevent the files_struct from being released while the binder_proc still has a reference. This can lead to an attempt to dereference the stale pointer obtained from proc->files prior to proc->files cleanup. This has been seen once in task_get_unused_fd_flags() when __alloc_fd() is called with a stale "files". The fix is to protect proc->files with a mutex to prevent cleanup while in use. Signed-off-by: Todd Kjos <tkjos(a)google.com> Cc: stable <stable(a)vger.kernel.org> # 4.14 Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/android/binder.c | 44 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index eb2d94d..dc05fec 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -481,7 +481,8 @@ enum binder_deferred_state { * @tsk task_struct for group_leader of process * (invariant after initialized) * @files files_struct for process - * (invariant after initialized) + * (protected by @files_lock) + * @files_lock mutex to protect @files * @deferred_work_node: element for binder_deferred_list * (protected by binder_deferred_lock) * @deferred_work: bitmap of deferred work to perform @@ -527,6 +528,7 @@ struct binder_proc { int pid; struct task_struct *tsk; struct files_struct *files; + struct mutex files_lock; struct hlist_node deferred_work_node; int deferred_work; bool is_dead; @@ -873,20 +875,26 @@ static void binder_inc_node_tmpref_ilocked(struct binder_node *node); 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; - - if (!lock_task_sighand(proc->tsk, &irqs)) - return -EMFILE; - + mutex_lock(&proc->files_lock); + if (proc->files == NULL) { + ret = -ESRCH; + goto err; + } + if (!lock_task_sighand(proc->tsk, &irqs)) { + ret = -EMFILE; + goto err; + } rlim_cur = task_rlimit(proc->tsk, RLIMIT_NOFILE); unlock_task_sighand(proc->tsk, &irqs); - return __alloc_fd(files, 0, rlim_cur, flags); + ret = __alloc_fd(proc->files, 0, rlim_cur, flags); +err: + mutex_unlock(&proc->files_lock); + return ret; } /* @@ -895,8 +903,10 @@ 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) { + mutex_lock(&proc->files_lock); if (proc->files) __fd_install(proc->files, fd, file); + mutex_unlock(&proc->files_lock); } /* @@ -906,9 +916,11 @@ static long task_close_fd(struct binder_proc *proc, unsigned int fd) { int retval; - if (proc->files == NULL) - return -ESRCH; - + mutex_lock(&proc->files_lock); + if (proc->files == NULL) { + retval = -ESRCH; + goto err; + } retval = __close_fd(proc->files, fd); /* can't restart close syscall because file table entry was cleared */ if (unlikely(retval == -ERESTARTSYS || @@ -916,7 +928,8 @@ static long task_close_fd(struct binder_proc *proc, unsigned int fd) retval == -ERESTARTNOHAND || retval == -ERESTART_RESTARTBLOCK)) retval = -EINTR; - +err: + mutex_unlock(&proc->files_lock); return retval; } @@ -4621,7 +4634,9 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma) ret = binder_alloc_mmap_handler(&proc->alloc, vma); if (ret) return ret; + mutex_lock(&proc->files_lock); proc->files = get_files_struct(current); + mutex_unlock(&proc->files_lock); return 0; err_bad_arg: @@ -4645,6 +4660,7 @@ static int binder_open(struct inode *nodp, struct file *filp) spin_lock_init(&proc->outer_lock); get_task_struct(current->group_leader); proc->tsk = current->group_leader; + mutex_init(&proc->files_lock); INIT_LIST_HEAD(&proc->todo); proc->default_priority = task_nice(current); binder_dev = container_of(filp->private_data, struct binder_device, @@ -4897,9 +4913,11 @@ static void binder_deferred_func(struct work_struct *work) files = NULL; if (defer & BINDER_DEFERRED_PUT_FILES) { + mutex_lock(&proc->files_lock); files = proc->files; if (files) proc->files = NULL; + mutex_unlock(&proc->files_lock); } if (defer & BINDER_DEFERRED_FLUSH) -- 2.7.4

6 years, 9 months

1
0
0 0

[PATCH 097/103] ANDROID: binder: remove waitqueue when thread exits.

by micky387

From: Martijn Coenen <maco(a)android.com> binder_poll() passes the thread->wait waitqueue that can be slept on for work. When a thread that uses epoll explicitly exits using BINDER_THREAD_EXIT, the waitqueue is freed, but it is never removed from the corresponding epoll data structure. When the process subsequently exits, the epoll cleanup code tries to access the waitlist, which results in a use-after-free. Prevent this by using POLLFREE when the thread exits. Signed-off-by: Martijn Coenen <maco(a)android.com> Reported-by: syzbot <syzkaller(a)googlegroups.com> Cc: stable <stable(a)vger.kernel.org> # 4.14 Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/android/binder.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index c96623d..77b1b95 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -4269,6 +4269,18 @@ static int binder_thread_release(struct binder_proc *proc, if (t) spin_lock(&t->lock); } + + /* + * If this thread used poll, make sure we remove the waitqueue + * from any epoll data structures holding it with POLLFREE. + * waitqueue_active() is safe to use here because we're holding + * the inner lock. + */ + if ((thread->looper & BINDER_LOOPER_STATE_POLL) && + waitqueue_active(&thread->wait)) { + wake_up_poll(&thread->wait, POLLHUP | POLLFREE); + } + binder_inner_proc_unlock(thread->proc); if (send_reply) -- 2.7.4

6 years, 9 months

1
0
0 0

[PATCH 093/103] android: binder: use VM_ALLOC to get vm area

by micky387

From: Ganesh Mahendran <opensource.ganesh(a)gmail.com> VM_IOREMAP is used to access hardware through a mechanism called I/O mapped memory. Android binder is a IPC machanism which will not access I/O memory. And VM_IOREMAP has alignment requiement which may not needed in binder. __get_vm_area_node() { ... if (flags & VM_IOREMAP) align = 1ul << clamp_t(int, fls_long(size), PAGE_SHIFT, IOREMAP_MAX_ORDER); ... } This patch will save some kernel vm area, especially for 32bit os. In 32bit OS, kernel vm area is only 240MB. We may got below error when launching a app: <3>[ 4482.440053] binder_alloc: binder_alloc_mmap_handler: 15728 8ce67000-8cf65000 get_vm_area failed -12 <3>[ 4483.218817] binder_alloc: binder_alloc_mmap_handler: 15745 8ce67000-8cf65000 get_vm_area failed -12 Signed-off-by: Ganesh Mahendran <opensource.ganesh(a)gmail.com> Acked-by: Martijn Coenen <maco(a)android.com> Acked-by: Todd Kjos <tkjos(a)google.com> Cc: stable <stable(a)vger.kernel.org> ---- V3: update comments V2: update comments Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/android/binder_alloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c index 8989e31..6da12c5 100644 --- a/drivers/android/binder_alloc.c +++ b/drivers/android/binder_alloc.c @@ -626,7 +626,7 @@ int binder_alloc_mmap_handler(struct binder_alloc *alloc, goto err_already_mapped; } - area = get_vm_area(vma->vm_end - vma->vm_start, VM_IOREMAP); + area = get_vm_area(vma->vm_end - vma->vm_start, VM_ALLOC); if (area == NULL) { ret = -ENOMEM; failure_string = "get_vm_area"; -- 2.7.4

6 years, 9 months

1
0
0 0

[PATCH 092/103] binder: check for binder_thread allocation failure in binder_poll()

by micky387

From: Eric Biggers <ebiggers(a)google.com> If the kzalloc() in binder_get_thread() fails, binder_poll() dereferences the resulting NULL pointer. Fix it by returning POLLERR if the memory allocation failed. This bug was found by syzkaller using fault injection. Reported-by: syzbot <syzkaller(a)googlegroups.com> Fixes: 457b9a6f09f0 ("Staging: android: add binder driver") Cc: stable(a)vger.kernel.org Signed-off-by: Eric Biggers <ebiggers(a)google.com> Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/android/binder.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index abf05aa..ed23507 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -4289,6 +4289,8 @@ static unsigned int binder_poll(struct file *filp, bool wait_for_proc_work; thread = binder_get_thread(proc); + if (!thread) + return POLLERR; binder_inner_proc_lock(thread->proc); thread->looper |= BINDER_LOOPER_STATE_POLL; -- 2.7.4

6 years, 9 months

1
0
0 0

[PATCH 091/103] ANDROID: binder: remove WARN() for redundant txn error

by micky387

From: Todd Kjos <tkjos(a)android.com> binder_send_failed_reply() is called when a synchronous transaction fails. It reports an error to the thread that is waiting for the completion. Given that the transaction is synchronous, there should never be more than 1 error response to that thread -- this was being asserted with a WARN(). However, when exercising the driver with syzbot tests, cases were observed where multiple "synchronous" requests were sent without waiting for responses, so it is possible that multiple errors would be reported to the thread. This testing was conducted with panic_on_warn set which forced the crash. This is easily reproduced by sending back-to-back "synchronous" transactions without checking for any response (eg, set read_size to 0): bwr.write_buffer = (uintptr_t)&bc1; bwr.write_size = sizeof(bc1); bwr.read_buffer = (uintptr_t)&br; bwr.read_size = 0; ioctl(fd, BINDER_WRITE_READ, &bwr); sleep(1); bwr2.write_buffer = (uintptr_t)&bc2; bwr2.write_size = sizeof(bc2); bwr2.read_buffer = (uintptr_t)&br; bwr2.read_size = 0; ioctl(fd, BINDER_WRITE_READ, &bwr2); sleep(1); The first transaction is sent to the servicemanager and the reply fails because no VMA is set up by this client. After binder_send_failed_reply() is called, the BINDER_WORK_RETURN_ERROR is sitting on the thread's todo list since the read_size was 0 and the client is not waiting for a response. The 2nd transaction is sent and the BINDER_WORK_RETURN_ERROR has not been consumed, so the thread's reply_error.cmd is still set (normally cleared when the BINDER_WORK_RETURN_ERROR is handled). Therefore when the servicemanager attempts to reply to the 2nd failed transaction, the error is already set and it triggers this warning. This is a user error since it is not waiting for the synchronous transaction to complete. If it ever does check, it will see an error. Changed the WARN() to a pr_warn(). Signed-off-by: Todd Kjos <tkjos(a)android.com> Reported-by: syzbot <syzkaller(a)googlegroups.com> Cc: stable <stable(a)vger.kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/android/binder.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index c88582a..abf05aa 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -1919,8 +1919,14 @@ static void binder_send_failed_reply(struct binder_transaction *t, &target_thread->todo); wake_up_interruptible(&target_thread->wait); } else { - WARN(1, "Unexpected reply error: %u\n", - target_thread->reply_error.cmd); + /* + * Cannot get here for normal operation, but + * we can if multiple synchronous transactions + * are sent without blocking for responses. + * Just ignore the 2nd error in this case. + */ + pr_warn("Unexpected reply error: %u\n", + target_thread->reply_error.cmd); } binder_inner_proc_unlock(target_thread->proc); binder_thread_dec_tmpref(target_thread); -- 2.7.4

6 years, 9 months

1
0
0 0

[PATCH 090/103] binder: replace "%p" with "%pK"

by micky387

From: Todd Kjos <tkjos(a)android.com> The format specifier "%p" can leak kernel addresses. Use "%pK" instead. There were 4 remaining cases in binder.c. Signed-off-by: Todd Kjos <tkjos(a)google.com> Cc: stable <stable(a)vger.kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/android/binder.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 7ab7726..c88582a 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2101,7 +2101,7 @@ static void binder_transaction_buffer_release(struct binder_proc *proc, int debug_id = buffer->debug_id; binder_debug(BINDER_DEBUG_TRANSACTION, - "%d buffer release %d, size %zd-%zd, failed at %p\n", + "%d buffer release %d, size %zd-%zd, failed at %pK\n", proc->pid, buffer->debug_id, buffer->data_size, buffer->offsets_size, failed_at); @@ -3613,7 +3613,7 @@ static int binder_thread_write(struct binder_proc *proc, } } binder_debug(BINDER_DEBUG_DEAD_BINDER, - "%d:%d BC_DEAD_BINDER_DONE %016llx found %p\n", + "%d:%d BC_DEAD_BINDER_DONE %016llx found %pK\n", proc->pid, thread->pid, (u64)cookie, death); if (death == NULL) { @@ -4921,7 +4921,7 @@ static void print_binder_transaction_ilocked(struct seq_file *m, spin_lock(&t->lock); to_proc = t->to_proc; seq_printf(m, - "%s %d: %p from %d:%d to %d:%d code %x flags %x pri %ld r%d", + "%s %d: %pK from %d:%d to %d:%d code %x flags %x pri %ld r%d", prefix, t->debug_id, t, t->from ? t->from->proc->pid : 0, t->from ? t->from->pid : 0, @@ -4945,7 +4945,7 @@ static void print_binder_transaction_ilocked(struct seq_file *m, } if (buffer->target_node) seq_printf(m, " node %d", buffer->target_node->debug_id); - seq_printf(m, " size %zd:%zd data %p\n", + seq_printf(m, " size %zd:%zd data %pK\n", buffer->data_size, buffer->offsets_size, buffer->data); } -- 2.7.4

6 years, 9 months

1
0
0 0

[PATCH 086/103] binder: fix use-after-free in binder_transaction()

by micky387

From: Todd Kjos <tkjos(a)android.com> User-space normally keeps the node alive when creating a transaction since it has a reference to the target. The local strong ref keeps it alive if the sending process dies before the target process processes the transaction. If the source process is malicious or has a reference counting bug, this can fail. In this case, when we attempt to decrement the node in the failure path, the node has already been freed. This is fixed by taking a tmpref on the node while constructing the transaction. To avoid re-acquiring the node lock and inner proc lock to increment the proc's tmpref, a helper is used that does the ref increments on both the node and proc. Signed-off-by: Todd Kjos <tkjos(a)google.com> Cc: stable <stable(a)vger.kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/android/binder.c | 93 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 66 insertions(+), 27 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 3f9d702..fa23e45 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2582,6 +2582,48 @@ static bool binder_proc_transaction(struct binder_transaction *t, return true; } +/** + * binder_get_node_refs_for_txn() - Get required refs on node for txn + * @node: struct binder_node for which to get refs + * @proc: returns @node->proc if valid + * @error: if no @proc then returns BR_DEAD_REPLY + * + * User-space normally keeps the node alive when creating a transaction + * since it has a reference to the target. The local strong ref keeps it + * alive if the sending process dies before the target process processes + * the transaction. If the source process is malicious or has a reference + * counting bug, relying on the local strong ref can fail. + * + * Since user-space can cause the local strong ref to go away, we also take + * a tmpref on the node to ensure it survives while we are constructing + * the transaction. We also need a tmpref on the proc while we are + * constructing the transaction, so we take that here as well. + * + * Return: The target_node with refs taken or NULL if no @node->proc is NULL. + * Also sets @proc if valid. If the @node->proc is NULL indicating that the + * target proc has died, @error is set to BR_DEAD_REPLY + */ +static struct binder_node *binder_get_node_refs_for_txn( + struct binder_node *node, + struct binder_proc **procp, + uint32_t *error) +{ + struct binder_node *target_node = NULL; + + binder_node_inner_lock(node); + if (node->proc) { + target_node = node; + binder_inc_node_nilocked(node, 1, 0, NULL); + binder_inc_node_tmpref_ilocked(node); + node->proc->tmp_ref++; + *procp = node->proc; + } else + *error = BR_DEAD_REPLY; + binder_node_inner_unlock(node); + + return target_node; +} + static void binder_transaction(struct binder_proc *proc, struct binder_thread *thread, struct binder_transaction_data *tr, int reply, @@ -2685,43 +2727,35 @@ static void binder_transaction(struct binder_proc *proc, ref = binder_get_ref_olocked(proc, tr->target.handle, true); if (ref) { - binder_inc_node(ref->node, 1, 0, NULL); - target_node = ref->node; - } - binder_proc_unlock(proc); - if (target_node == NULL) { + target_node = binder_get_node_refs_for_txn( + ref->node, &target_proc, + &return_error); + } else { binder_user_error("%d:%d got transaction to invalid handle\n", - proc->pid, thread->pid); + proc->pid, thread->pid); return_error = BR_FAILED_REPLY; - return_error_param = -EINVAL; - return_error_line = __LINE__; - goto err_invalid_target_handle; } + binder_proc_unlock(proc); } else { mutex_lock(&context->context_mgr_node_lock); target_node = context->binder_context_mgr_node; - if (target_node == NULL) { + if (target_node) + target_node = binder_get_node_refs_for_txn( + target_node, &target_proc, + &return_error); + else return_error = BR_DEAD_REPLY; - mutex_unlock(&context->context_mgr_node_lock); - return_error_line = __LINE__; - goto err_no_context_mgr_node; - } - binder_inc_node(target_node, 1, 0, NULL); mutex_unlock(&context->context_mgr_node_lock); } - e->to_node = target_node->debug_id; - binder_node_lock(target_node); - target_proc = target_node->proc; - if (target_proc == NULL) { - binder_node_unlock(target_node); - return_error = BR_DEAD_REPLY; + if (!target_node) { + /* + * return_error is set above + */ + return_error_param = -EINVAL; return_error_line = __LINE__; goto err_dead_binder; } - binder_inner_proc_lock(target_proc); - target_proc->tmp_ref++; - binder_inner_proc_unlock(target_proc); - binder_node_unlock(target_node); + e->to_node = target_node->debug_id; if (security_binder_transaction(proc->tsk, target_proc->tsk) < 0) { return_error = BR_FAILED_REPLY; @@ -3071,6 +3105,8 @@ static void binder_transaction(struct binder_proc *proc, if (target_thread) binder_thread_dec_tmpref(target_thread); binder_proc_dec_tmpref(target_proc); + if (target_node) + binder_dec_node_tmpref(target_node); /* * write barrier to synchronize with initialization * of log entry @@ -3089,6 +3125,8 @@ err_bad_parent: err_copy_data_failed: trace_binder_transaction_failed_buffer_release(t->buffer); binder_transaction_buffer_release(target_proc, t->buffer, offp); + if (target_node) + binder_dec_node_tmpref(target_node); target_node = NULL; t->buffer->transaction = NULL; binder_alloc_free_buf(&target_proc->alloc, t->buffer); @@ -3103,13 +3141,14 @@ err_bad_call_stack: err_empty_call_stack: err_dead_binder: err_invalid_target_handle: -err_no_context_mgr_node: if (target_thread) binder_thread_dec_tmpref(target_thread); if (target_proc) binder_proc_dec_tmpref(target_proc); - if (target_node) + if (target_node) { binder_dec_node(target_node, 1, 0); + binder_dec_node_tmpref(target_node); + } binder_debug(BINDER_DEBUG_FAILED_TRANSACTION, "%d:%d transaction failed %d/%d, size %lld-%lld line %d\n", -- 2.7.4

6 years, 9 months

1
0
0 0

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror March 2018