UPSTREAM: 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>
Bug: 171501513
Change-Id: Ic9338c85cbe3b11ab6f2bda55dce9964bb48447a
(cherry picked from commit 0f966cba95c78029f491b433ea95ff38f414a761)
Signed-off-by: Todd Kjos <tkjos@google.com>
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index ba51fc709dea86012dda0d24f17cb8f2a1fb5131..9185a7f10852cfe8ee27b1085b278a04d5e220e2 100644 (file)
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -3305,6 +3305,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(

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 28fededae3d375475f674aa8be43a3d723e53714..5addcd56afb4b97e854ccdc772a0856912507653 100644 (file)
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -672,6 +672,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
@@ -682,6 +684,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);
@@ -777,6 +791,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++;
        }
@@ -1103,6 +1121,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

diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
index 3daa3e21126778cbdb6f119096cc4a6e1b15234f..fdb220eb55d14fc68cd8cb24d3beff2bc3cf75f0 100644 (file)
--- a/drivers/android/binder_alloc.h
+++ b/drivers/android/binder_alloc.h
@@ -32,6 +32,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
@@ -50,9 +51,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;
 

diff --git a/include/uapi/linux/android/binder.h b/include/uapi/linux/android/binder.h
index 0631c500702c10634b0e3a50cdd0c1041edfa0d8..0457ff02d616e9440bb2c51b8db0e3f9f7c2201e 100644 (file)
--- a/include/uapi/linux/android/binder.h
+++ b/include/uapi/linux/android/binder.h
@@ -294,6 +294,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 {