Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit aa2c274c authored by Todd Kjos's avatar Todd Kjos Committed by Greg Kroah-Hartman
Browse files

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: 80cd7956 ("binder: fix use-after-free due to ksys_close() during fdget()")
Cc: stable <stable@vger.kernel.org>
Reviewed-by: default avatarMartijn Coenen <maco@android.com>
Acked-by: default avatarChristian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: default avatarTodd Kjos <tkjos@google.com>
Link: https://lore.kernel.org/r/20210830195146.587206-1-tkjos@google.com


Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 93fa08e9

drivers/android/binder.c

+17 −6
Original line number Diff line number Diff line
@@ -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,