Skip to content

Commit

Permalink
UPSTREAM: binder: add flag to clear buffer on txn complete
Browse files Browse the repository at this point in the history
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 <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Cc: stable <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Bug: 171501513
Change-Id: Ic9338c85cbe3b11ab6f2bda55dce9964bb48447a
(cherry picked from commit 0f966cb)
Signed-off-by: Todd Kjos <[email protected]>
Signed-off-by: Sami Tolvanen <[email protected]>
Signed-off-by: Chenyang Zhong <[email protected]>
  • Loading branch information
toddkjos authored and PainKiller3 committed May 23, 2021
1 parent 3c0fee1 commit 60a6d02
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 1 deletion.
1 change: 1 addition & 0 deletions drivers/android/binder.c
Original file line number Diff line number Diff line change
Expand Up @@ -3284,6 +3284,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(
Expand Down
48 changes: 48 additions & 0 deletions drivers/android/binder_alloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,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
Expand All @@ -632,6 +634,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);
Expand Down Expand Up @@ -727,6 +741,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++;
}
Expand Down Expand Up @@ -1053,6 +1071,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
Expand Down
4 changes: 3 additions & 1 deletion drivers/android/binder_alloc.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,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
Expand All @@ -54,9 +55,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;

Expand Down
1 change: 1 addition & 0 deletions include/uapi/linux/android/binder.h
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,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 {
Expand Down

0 comments on commit 60a6d02

Please sign in to comment.