forked from mirrors/linux
		
	binder: add flag to clear buffer on txn complete
Add a per-transaction flag to indicate that the buffer must be cleared when the transaction is complete to prevent copies of sensitive data from being preserved in memory. Signed-off-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20201120233743.3617529-1-tkjos@google.com Cc: stable <stable@vger.kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
		
							parent
							
								
									d1b928ee1c
								
							
						
					
					
						commit
						0f966cba95
					
				
					 4 changed files with 53 additions and 1 deletions
				
			
		|  | @ -2756,6 +2756,7 @@ static void binder_transaction(struct binder_proc *proc, | |||
| 	t->buffer->debug_id = t->debug_id; | ||||
| 	t->buffer->transaction = t; | ||||
| 	t->buffer->target_node = target_node; | ||||
| 	t->buffer->clear_on_free = !!(t->flags & TF_CLEAR_BUF); | ||||
| 	trace_binder_transaction_alloc_buf(t->buffer); | ||||
| 
 | ||||
| 	if (binder_alloc_copy_user_to_buffer( | ||||
|  |  | |||
|  | @ -696,6 +696,8 @@ static void binder_free_buf_locked(struct binder_alloc *alloc, | |||
| 	binder_insert_free_buffer(alloc, buffer); | ||||
| } | ||||
| 
 | ||||
| static void binder_alloc_clear_buf(struct binder_alloc *alloc, | ||||
| 				   struct binder_buffer *buffer); | ||||
| /**
 | ||||
|  * binder_alloc_free_buf() - free a binder buffer | ||||
|  * @alloc:	binder_alloc for this proc | ||||
|  | @ -706,6 +708,18 @@ static void binder_free_buf_locked(struct binder_alloc *alloc, | |||
| void binder_alloc_free_buf(struct binder_alloc *alloc, | ||||
| 			    struct binder_buffer *buffer) | ||||
| { | ||||
| 	/*
 | ||||
| 	 * We could eliminate the call to binder_alloc_clear_buf() | ||||
| 	 * from binder_alloc_deferred_release() by moving this to | ||||
| 	 * binder_alloc_free_buf_locked(). However, that could | ||||
| 	 * increase contention for the alloc mutex if clear_on_free | ||||
| 	 * is used frequently for large buffers. The mutex is not | ||||
| 	 * needed for correctness here. | ||||
| 	 */ | ||||
| 	if (buffer->clear_on_free) { | ||||
| 		binder_alloc_clear_buf(alloc, buffer); | ||||
| 		buffer->clear_on_free = false; | ||||
| 	} | ||||
| 	mutex_lock(&alloc->mutex); | ||||
| 	binder_free_buf_locked(alloc, buffer); | ||||
| 	mutex_unlock(&alloc->mutex); | ||||
|  | @ -802,6 +816,10 @@ void binder_alloc_deferred_release(struct binder_alloc *alloc) | |||
| 		/* Transaction should already have been freed */ | ||||
| 		BUG_ON(buffer->transaction); | ||||
| 
 | ||||
| 		if (buffer->clear_on_free) { | ||||
| 			binder_alloc_clear_buf(alloc, buffer); | ||||
| 			buffer->clear_on_free = false; | ||||
| 		} | ||||
| 		binder_free_buf_locked(alloc, buffer); | ||||
| 		buffers++; | ||||
| 	} | ||||
|  | @ -1135,6 +1153,36 @@ static struct page *binder_alloc_get_page(struct binder_alloc *alloc, | |||
| 	return lru_page->page_ptr; | ||||
| } | ||||
| 
 | ||||
| /**
 | ||||
|  * binder_alloc_clear_buf() - zero out buffer | ||||
|  * @alloc: binder_alloc for this proc | ||||
|  * @buffer: binder buffer to be cleared | ||||
|  * | ||||
|  * memset the given buffer to 0 | ||||
|  */ | ||||
| static void binder_alloc_clear_buf(struct binder_alloc *alloc, | ||||
| 				   struct binder_buffer *buffer) | ||||
| { | ||||
| 	size_t bytes = binder_alloc_buffer_size(alloc, buffer); | ||||
| 	binder_size_t buffer_offset = 0; | ||||
| 
 | ||||
| 	while (bytes) { | ||||
| 		unsigned long size; | ||||
| 		struct page *page; | ||||
| 		pgoff_t pgoff; | ||||
| 		void *kptr; | ||||
| 
 | ||||
| 		page = binder_alloc_get_page(alloc, buffer, | ||||
| 					     buffer_offset, &pgoff); | ||||
| 		size = min_t(size_t, bytes, PAGE_SIZE - pgoff); | ||||
| 		kptr = kmap(page) + pgoff; | ||||
| 		memset(kptr, 0, size); | ||||
| 		kunmap(page); | ||||
| 		bytes -= size; | ||||
| 		buffer_offset += size; | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| /**
 | ||||
|  * binder_alloc_copy_user_to_buffer() - copy src user to tgt user | ||||
|  * @alloc: binder_alloc for this proc | ||||
|  |  | |||
|  | @ -23,6 +23,7 @@ struct binder_transaction; | |||
|  * @entry:              entry alloc->buffers | ||||
|  * @rb_node:            node for allocated_buffers/free_buffers rb trees | ||||
|  * @free:               %true if buffer is free | ||||
|  * @clear_on_free:      %true if buffer must be zeroed after use | ||||
|  * @allow_user_free:    %true if user is allowed to free buffer | ||||
|  * @async_transaction:  %true if buffer is in use for an async txn | ||||
|  * @debug_id:           unique ID for debugging | ||||
|  | @ -41,9 +42,10 @@ struct binder_buffer { | |||
| 	struct rb_node rb_node; /* free entry by size or allocated entry */ | ||||
| 				/* by address */ | ||||
| 	unsigned free:1; | ||||
| 	unsigned clear_on_free:1; | ||||
| 	unsigned allow_user_free:1; | ||||
| 	unsigned async_transaction:1; | ||||
| 	unsigned debug_id:29; | ||||
| 	unsigned debug_id:28; | ||||
| 
 | ||||
| 	struct binder_transaction *transaction; | ||||
| 
 | ||||
|  |  | |||
|  | @ -248,6 +248,7 @@ enum transaction_flags { | |||
| 	TF_ROOT_OBJECT	= 0x04,	/* contents are the component's root object */ | ||||
| 	TF_STATUS_CODE	= 0x08,	/* contents are a 32-bit status code */ | ||||
| 	TF_ACCEPT_FDS	= 0x10,	/* allow replies with file descriptors */ | ||||
| 	TF_CLEAR_BUF	= 0x20,	/* clear buffer on txn complete */ | ||||
| }; | ||||
| 
 | ||||
| struct binder_transaction_data { | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue
	
	 Todd Kjos
						Todd Kjos