mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 02:30:34 +02:00 
			
		
		
		
	binder: don't detect sender/target during buffer cleanup
When freeing txn buffers, binder_transaction_buffer_release()
attempts to detect whether the current context is the target by
comparing current->group_leader to proc->tsk. This is an unreliable
test. Instead explicitly pass an 'is_failure' boolean.
Detecting the sender was being used as a way to tell if the
transaction failed to be sent.  When cleaning up after
failing to send a transaction, there is no need to close
the fds associated with a BINDER_TYPE_FDA object. Now
'is_failure' can be used to accurately detect this case.
Fixes: 44d8047f1d ("binder: use standard functions to allocate fds")
Cc: stable <stable@vger.kernel.org>
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Todd Kjos <tkjos@google.com>
Link: https://lore.kernel.org/r/20211015233811.3532235-1-tkjos@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
			
			
This commit is contained in:
		
							parent
							
								
									be24dd486d
								
							
						
					
					
						commit
						32e9f56a96
					
				
					 1 changed files with 7 additions and 7 deletions
				
			
		| 
						 | 
				
			
			@ -1870,7 +1870,7 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
 | 
			
		|||
		binder_dec_node(buffer->target_node, 1, 0);
 | 
			
		||||
 | 
			
		||||
	off_start_offset = ALIGN(buffer->data_size, sizeof(void *));
 | 
			
		||||
	off_end_offset = is_failure ? failed_at :
 | 
			
		||||
	off_end_offset = is_failure && failed_at ? failed_at :
 | 
			
		||||
				off_start_offset + buffer->offsets_size;
 | 
			
		||||
	for (buffer_offset = off_start_offset; buffer_offset < off_end_offset;
 | 
			
		||||
	     buffer_offset += sizeof(binder_size_t)) {
 | 
			
		||||
| 
						 | 
				
			
			@ -1956,9 +1956,8 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
 | 
			
		|||
			binder_size_t fd_buf_size;
 | 
			
		||||
			binder_size_t num_valid;
 | 
			
		||||
 | 
			
		||||
			if (proc->tsk != current->group_leader) {
 | 
			
		||||
			if (is_failure) {
 | 
			
		||||
				/*
 | 
			
		||||
				 * Nothing to do if running in sender context
 | 
			
		||||
				 * The fd fixups have not been applied so no
 | 
			
		||||
				 * fds need to be closed.
 | 
			
		||||
				 */
 | 
			
		||||
| 
						 | 
				
			
			@ -3185,6 +3184,7 @@ static void binder_transaction(struct binder_proc *proc,
 | 
			
		|||
 * binder_free_buf() - free the specified buffer
 | 
			
		||||
 * @proc:	binder proc that owns buffer
 | 
			
		||||
 * @buffer:	buffer to be freed
 | 
			
		||||
 * @is_failure:	failed to send transaction
 | 
			
		||||
 *
 | 
			
		||||
 * If buffer for an async transaction, enqueue the next async
 | 
			
		||||
 * transaction from the node.
 | 
			
		||||
| 
						 | 
				
			
			@ -3194,7 +3194,7 @@ static void binder_transaction(struct binder_proc *proc,
 | 
			
		|||
static void
 | 
			
		||||
binder_free_buf(struct binder_proc *proc,
 | 
			
		||||
		struct binder_thread *thread,
 | 
			
		||||
		struct binder_buffer *buffer)
 | 
			
		||||
		struct binder_buffer *buffer, bool is_failure)
 | 
			
		||||
{
 | 
			
		||||
	binder_inner_proc_lock(proc);
 | 
			
		||||
	if (buffer->transaction) {
 | 
			
		||||
| 
						 | 
				
			
			@ -3222,7 +3222,7 @@ binder_free_buf(struct binder_proc *proc,
 | 
			
		|||
		binder_node_inner_unlock(buf_node);
 | 
			
		||||
	}
 | 
			
		||||
	trace_binder_transaction_buffer_release(buffer);
 | 
			
		||||
	binder_transaction_buffer_release(proc, thread, buffer, 0, false);
 | 
			
		||||
	binder_transaction_buffer_release(proc, thread, buffer, 0, is_failure);
 | 
			
		||||
	binder_alloc_free_buf(&proc->alloc, buffer);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -3424,7 +3424,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, thread, buffer);
 | 
			
		||||
			binder_free_buf(proc, thread, buffer, false);
 | 
			
		||||
			break;
 | 
			
		||||
		}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -4117,7 +4117,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, thread, buffer);
 | 
			
		||||
			binder_free_buf(proc, thread, buffer, true);
 | 
			
		||||
			binder_debug(BINDER_DEBUG_FAILED_TRANSACTION,
 | 
			
		||||
				     "%d:%d %stransaction %d fd fixups failed %d/%d, line %d\n",
 | 
			
		||||
				     proc->pid, thread->pid,
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in a new issue