binder: make sure fd closes complete

commit 5fdb55c1ac9585eb23bb2541d5819224429e103d upstream.

During BC_FREE_BUFFER processing, the BINDER_TYPE_FDA object
cleanup may close 1 or more fds. The close operations are
completed using the task work mechanism -- which means the thread
needs to return to userspace or the file object may never be
dereferenced -- which can lead to hung processes.

Force the binder thread back to userspace if an fd is closed during
BC_FREE_BUFFER handling.

Fixes: 80cd795630d6 ("binder: fix use-after-free due to ksys_close() during fdget()")
Cc: stable <stable@vger.kernel.org>
Reviewed-by: Martijn Coenen <maco@android.com>
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Todd Kjos <tkjos@google.com>
Link: https://lore.kernel.org/r/20210830195146.587206-1-tkjos@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 89b590c9573fff59cacc38c2a5cf68a47fbc0a0c..4eaef780844ea0bb6d51d7ef76608b8986fdda7d 100644 (file)
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2239,6 +2239,7 @@ static void binder_deferred_fd_close(int fd)
 }
 
 static void binder_transaction_buffer_release(struct binder_proc *proc,
+                                             struct binder_thread *thread,
                                              struct binder_buffer *buffer,
                                              binder_size_t failed_at,
                                              bool is_failure)
@@ -2398,8 +2399,16 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
                                                &proc->alloc, &fd, buffer,
                                                offset, sizeof(fd));
                                WARN_ON(err);
-                               if (!err)
+                               if (!err) {
                                        binder_deferred_fd_close(fd);
+                                       /*
+                                        * Need to make sure the thread goes
+                                        * back to userspace to complete the
+                                        * deferred close
+                                        */
+                                       if (thread)
+                                               thread->looper_need_return = true;
+                               }
                        }
                } break;
                default:
@@ -3469,7 +3478,7 @@ static void binder_transaction(struct binder_proc *proc,
 err_copy_data_failed:
        binder_free_txn_fixups(t);
        trace_binder_transaction_failed_buffer_release(t->buffer);
-       binder_transaction_buffer_release(target_proc, t->buffer,
+       binder_transaction_buffer_release(target_proc, NULL, t->buffer,
                                          buffer_offset, true);
        if (target_node)
                binder_dec_node_tmpref(target_node);
@@ -3546,7 +3555,9 @@ static void binder_transaction(struct binder_proc *proc,
  * Cleanup buffer and free it.
  */
 static void
-binder_free_buf(struct binder_proc *proc, struct binder_buffer *buffer)
+binder_free_buf(struct binder_proc *proc,
+               struct binder_thread *thread,
+               struct binder_buffer *buffer)
 {
        binder_inner_proc_lock(proc);
        if (buffer->transaction) {
@@ -3574,7 +3585,7 @@ binder_free_buf(struct binder_proc *proc, struct binder_buffer *buffer)
                binder_node_inner_unlock(buf_node);
        }
        trace_binder_transaction_buffer_release(buffer);
-       binder_transaction_buffer_release(proc, buffer, 0, false);
+       binder_transaction_buffer_release(proc, thread, buffer, 0, false);
        binder_alloc_free_buf(&proc->alloc, buffer);
 }
 
@@ -3775,7 +3786,7 @@ static int binder_thread_write(struct binder_proc *proc,
                                     proc->pid, thread->pid, (u64)data_ptr,
                                     buffer->debug_id,
                                     buffer->transaction ? "active" : "finished");
-                       binder_free_buf(proc, buffer);
+                       binder_free_buf(proc, thread, buffer);
                        break;
                }
 
@@ -4463,7 +4474,7 @@ static int binder_thread_read(struct binder_proc *proc,
                        buffer->transaction = NULL;
                        binder_cleanup_transaction(t, "fd fixups failed",
                                                   BR_FAILED_REPLY);
-                       binder_free_buf(proc, buffer);
+                       binder_free_buf(proc, thread, buffer);
                        binder_debug(BINDER_DEBUG_FAILED_TRANSACTION,
                                     "%d:%d %stransaction %d fd fixups failed %d/%d, line %d\n",
                                     proc->pid, thread->pid,